Skip to content

Commit

Permalink
Refactor Item Properties View to use table components
Browse files Browse the repository at this point in the history
The current implentation of the item properties view uses
<p> tags with no styling to display the properties.
This looks ok but can be significantly improved by showing
the properties in a table.

 This commit makes the following changes:
- Reuses filter tree component for item properties view.
- Added option to hide filler columns/cells
- Ability to render a table cell value as an element
- Added tooltips for item properties entries

Signed-off-by: Neel Gondalia <[email protected]>
  • Loading branch information
ngondalia committed May 6, 2024
1 parent dd3880e commit 3d2ee18
Show file tree
Hide file tree
Showing 14 changed files with 231 additions and 70 deletions.
29 changes: 29 additions & 0 deletions packages/base/src/signals/item-properties-signal-payload.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/***************************************************************************************
* Copyright (c) 2024 BlackBerry Limited and contributors.
*
* Licensed under the MIT license. See LICENSE file in the project root for details.
***************************************************************************************/

export class ItemPropertiesSignalPayload {
private outputDescriptorId: string | undefined;
private experimentUUID: string | undefined;
private properties: { [key: string]: string };

constructor(props: { [key: string]: string }, expUUID?: string, descriptorId?: string) {
this.properties = props;
this.experimentUUID = expUUID;
this.outputDescriptorId = descriptorId;
}

public getOutputDescriptorId(): string | undefined {
return this.outputDescriptorId;
}

public getExperimentUUID(): string | undefined {
return this.experimentUUID;
}

public getProperties(): { [key: string]: string } {
return this.properties;
}
}
7 changes: 4 additions & 3 deletions packages/base/src/signals/signal-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { TimeRangeUpdatePayload } from './time-range-data-signal-payloads';
import { ContextMenuContributedSignalPayload } from './context-menu-contributed-signal-payload';
import { ContextMenuItemClickedSignalPayload } from './context-menu-item-clicked-signal-payload';
import { RowSelectionsChangedSignalPayload } from './row-selections-changed-signal-payload';
import { ItemPropertiesSignalPayload } from './item-properties-signal-payload';

export declare interface SignalManager {
fireTraceOpenedSignal(trace: Trace): void;
Expand All @@ -19,7 +20,7 @@ export declare interface SignalManager {
fireExperimentUpdatedSignal(experiment: Experiment): void;
fireOpenedTracesChangedSignal(payload: OpenedTracesUpdatedSignalPayload): void;
fireOutputAddedSignal(payload: OutputAddedSignalPayload): void;
fireItemPropertiesSignalUpdated(properties?: { [key: string]: string }): void;
fireItemPropertiesSignalUpdated(payload: ItemPropertiesSignalPayload): void;
fireThemeChangedSignal(theme: string): void;
// TODO - Refactor or remove this signal. Similar signal to fireRequestSelectionRangeChange
fireSelectionChangedSignal(payload: { [key: string]: string }): void;
Expand Down Expand Up @@ -119,8 +120,8 @@ export class SignalManager extends EventEmitter implements SignalManager {
fireOutputAddedSignal(payload: OutputAddedSignalPayload): void {
this.emit(Signals.OUTPUT_ADDED, payload);
}
fireItemPropertiesSignalUpdated(properties?: { [key: string]: string }): void {
this.emit(Signals.ITEM_PROPERTIES_UPDATED, properties);
fireItemPropertiesSignalUpdated(payload: ItemPropertiesSignalPayload): void {
this.emit(Signals.ITEM_PROPERTIES_UPDATED, payload);
}
fireThemeChangedSignal(theme: string): void {
this.emit(Signals.THEME_CHANGED, theme);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { SearchFilterRenderer, CellRenderer, LoadingRenderer } from './table-ren
import { OutputDescriptor, ResponseStatus } from 'tsp-typescript-client';
import { PaginationBarComponent } from './utils/pagination-bar-component';
import { OptionCheckBoxState, OptionState, OptionType } from './drop-down-component';
import { ItemPropertiesSignalPayload } from 'traceviewer-base/lib/signals/item-properties-signal-payload';

type TableOuputState = AbstractOutputState & {
tableColumns: ColDef[];
Expand Down Expand Up @@ -301,7 +302,9 @@ export class TableOutputComponent extends AbstractOutputComponent<TableOutputPro
// Notfiy selection changed
this.handleRowSelectionChange(itemPropsObj);
// Notfiy properties changed
signalManager().fireItemPropertiesSignalUpdated(itemPropsObj);
signalManager().fireItemPropertiesSignalUpdated(
new ItemPropertiesSignalPayload(itemPropsObj, this.props.traceId, this.props.outputDescriptor.id)
);
}

private fetchItemProperties(columns: Column[] | null, data: any) {
Expand Down Expand Up @@ -437,7 +440,11 @@ export class TableOutputComponent extends AbstractOutputComponent<TableOutputPro
// Notfiy selection changed
this.handleRowSelectionChange(itemPropsObj);
// Notify properties changed
signalManager().fireItemPropertiesSignalUpdated(itemPropsObj);
if (itemPropsObj) {
signalManager().fireItemPropertiesSignalUpdated(
new ItemPropertiesSignalPayload(itemPropsObj, this.props.traceId, this.props.outputDescriptor.id)
);
}
}
}

Expand Down Expand Up @@ -839,7 +846,15 @@ export class TableOutputComponent extends AbstractOutputComponent<TableOutputPro
// Notify selection changed
this.handleRowSelectionChange(itemPropsObj);
// Notify properties changed
signalManager().fireItemPropertiesSignalUpdated(itemPropsObj);
if (itemPropsObj) {
signalManager().fireItemPropertiesSignalUpdated(
new ItemPropertiesSignalPayload(
itemPropsObj,
this.props.traceId,
this.props.outputDescriptor.id
)
);
}
isFound = true;
rowNode.setSelected(true);
}
Expand Down Expand Up @@ -875,7 +890,15 @@ export class TableOutputComponent extends AbstractOutputComponent<TableOutputPro
// Notfiy selection changed
this.handleRowSelectionChange(itemPropsObj);
// Notify properties changed
signalManager().fireItemPropertiesSignalUpdated(itemPropsObj);
if (itemPropsObj) {
signalManager().fireItemPropertiesSignalUpdated(
new ItemPropertiesSignalPayload(
itemPropsObj,
this.props.traceId,
this.props.outputDescriptor.id
)
);
}
this.selectRows();
}
this.gridMatched = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
} from 'traceviewer-base/lib/signals/context-menu-contributed-signal-payload';
import { ContextMenuItemClickedSignalPayload } from 'traceviewer-base/lib/signals/context-menu-item-clicked-signal-payload';
import { RowSelectionsChangedSignalPayload } from 'traceviewer-base/lib/signals/row-selections-changed-signal-payload';
import { ItemPropertiesSignalPayload } from 'traceviewer-base/lib/signals/item-properties-signal-payload';

type TimegraphOutputProps = AbstractOutputProps & {
addWidgetResizeHandler: (handler: () => void) => void;
Expand Down Expand Up @@ -929,7 +930,11 @@ export class TimegraphOutputComponent extends AbstractTreeOutputComponent<Timegr
if (element && this.props.viewRange) {
tooltipObject = await this.fetchTooltip(element);
}
signalManager().fireItemPropertiesSignalUpdated(tooltipObject);
if (tooltipObject) {
signalManager().fireItemPropertiesSignalUpdated(
new ItemPropertiesSignalPayload(tooltipObject, this.props.traceId, this.props.outputDescriptor.id)
);
}
}

private getTimegraphRowIds() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ interface TableBodyProps {
onClose: (id: number) => void;
onToggleCheck: (id: number) => void;
onContextMenu: (event: React.MouseEvent<HTMLDivElement>, id: number) => void;
hideFillers?: boolean;
}

export class TableBody extends React.Component<TableBodyProps> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,19 @@ export class TableCell extends React.Component<TableCellProps> {
}
render(): React.ReactNode {
const { node, index } = this.props;
const content = node.labels[index];

let content;
if (node.elementIndex && node.elementIndex === index && node.getElement) {
content = node.getElement();
} else {
content = node.labels[index];
}

const title = node.showTooltip ? node.labels[index] : undefined;

return (
<td key={this.props.index + '-td-' + this.props.node.id}>
<span>
<span title={title}>
{this.props.children}
{content}
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ interface TableHeaderProps {
sortableColumns: string[];
sortConfig: SortConfig[];
onSort: (sortColumn: string) => void;
hideFillers?: boolean;
}

export class TableHeader extends React.Component<TableHeaderProps> {
Expand All @@ -18,8 +19,17 @@ export class TableHeader extends React.Component<TableHeaderProps> {

constructor(props: TableHeaderProps) {
super(props);
this.gridTemplateColumns = this.props.columns.map(() => 'max-content');
this.gridTemplateColumns.push('minmax(0px, 1fr)');
this.gridTemplateColumns = this.props.columns.map((_header, index) => {
if (index === this.props.columns.length - 1) {
if (this.props.hideFillers) {
return 'auto';
}
}
return 'max-content';
});
if (!this.props.hideFillers) {
this.gridTemplateColumns.push('minmax(0px, 1fr)');
}
}

handleSortChange = (sortColumn: string, ev: React.MouseEvent<HTMLTableCellElement, MouseEvent>): void => {
Expand Down Expand Up @@ -106,7 +116,9 @@ export class TableHeader extends React.Component<TableHeaderProps> {
</span>
</th>
));
header.push(<th key={'th-filler'} className="filler" />);
if (!this.props.hideFillers) {
header.push(<th key={'th-filler'} className="filler" />);
}
return header;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface TableRowProps {
onRowClick: (id: number) => void;
onMultipleRowClick?: (id: number, isShiftClicked?: boolean) => void;
onContextMenu: (event: React.MouseEvent<HTMLDivElement>, id: number) => void;
hideFillers?: boolean;
}

export class TableRow extends React.Component<TableRowProps> {
Expand Down Expand Up @@ -78,7 +79,9 @@ export class TableRow extends React.Component<TableRowProps> {
{index === 0 ? this.renderCloseButton() : undefined}
</TableCell>
));
row.push(<td key={node.id + '-filler'} className="filler" />);
if (!this.props.hideFillers) {
row.push(<td key={node.id + '-filler'} className="filler" />);
}
return row;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface TableProps {
showHeader: boolean;
headers: ColumnHeader[];
className: string;
hideFillers?: boolean;
}

export class Table extends React.Component<TableProps> {
Expand Down Expand Up @@ -59,10 +60,20 @@ export class Table extends React.Component<TableProps> {
};

render(): JSX.Element {
const gridTemplateColumns = this.props.headers
.map(() => 'max-content')
.join(' ')
.concat(' minmax(0px, 1fr)');
let gridTemplateColumns = this.props.headers
.map((_header, index) => {
if (index === this.props.headers.length - 1) {
if (this.props.hideFillers) {
return 'auto';
}
}
return 'max-content';
})
.join(' ');
if (!this.props.hideFillers) {
gridTemplateColumns = gridTemplateColumns.concat(' minmax(0px, 1fr)');
}

return (
<div>
<table style={{ gridTemplateColumns: gridTemplateColumns }} className={this.props.className}>
Expand All @@ -72,6 +83,7 @@ export class Table extends React.Component<TableProps> {
sortableColumns={this.sortableColumns}
onSort={this.onSortChange}
sortConfig={this.props.sortConfig}
hideFillers={this.props.hideFillers}
/>
)}
<TableBody {...this.props} nodes={sortNodes(this.props.nodes, this.props.sortConfig)} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ export interface TreeNode {
labels: string[];
children: Array<TreeNode>;
isRoot: boolean;
showTooltip?: boolean;
elementIndex?: number;
getElement?: () => JSX.Element;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface FilterTreeProps {
showHeader: boolean;
headers: ColumnHeader[];
className: string;
hideFillers?: boolean;
}

interface FilterTreeState {
Expand Down Expand Up @@ -288,6 +289,7 @@ export class FilterTree extends React.Component<FilterTreeProps, FilterTreeState
showHeader={this.props.showHeader}
headers={this.props.headers}
className={this.props.className}
hideFillers={this.props.hideFillers}
/>
);

Expand Down
Loading

0 comments on commit 3d2ee18

Please sign in to comment.