diff --git a/res/css/global.css b/res/css/global.css index 1fcabb3a70..61ec860915 100644 --- a/res/css/global.css +++ b/res/css/global.css @@ -15,6 +15,7 @@ --z-copy-table-warning: 4; --z-tree-view: 0; --z-tree-view-inner: 2; + --z-tree-view-header-divider: 1; --z-overflow-edge-indicator: 3; --z-chart-viewport-expanded: 1; --z-timeline-selection: 1; diff --git a/src/actions/profile-view.ts b/src/actions/profile-view.ts index 51ff3dcdb4..ed06ca604c 100644 --- a/src/actions/profile-view.ts +++ b/src/actions/profile-view.ts @@ -82,6 +82,7 @@ import { import { changeStoredProfileNameInDb } from 'firefox-profiler/app-logic/uploaded-profiles-db'; import type { TabSlug } from '../app-logic/tabs-handling'; import type { CallNodeInfo } from '../profile-logic/call-node-info'; +import type { SingleColumnSortState } from '../components/shared/TreeView'; import { intersectSets } from 'firefox-profiler/utils/set'; /** @@ -1615,6 +1616,13 @@ export function changeMarkersSearchString(searchString: string): Action { }; } +export function changeMarkerTableSort(sort: SingleColumnSortState[]): Action { + return { + type: 'CHANGE_MARKER_TABLE_SORT', + sort, + }; +} + export function changeNetworkSearchString(searchString: string): Action { return { type: 'CHANGE_NETWORK_SEARCH_STRING', diff --git a/src/app-logic/url-handling.ts b/src/app-logic/url-handling.ts index 02b2907421..a8568da7a0 100644 --- a/src/app-logic/url-handling.ts +++ b/src/app-logic/url-handling.ts @@ -52,8 +52,9 @@ import { tabSlugs } from '../app-logic/tabs-handling'; import { StringTable } from 'firefox-profiler/utils/string-table'; import type { ProfileUpgradeInfo } from 'firefox-profiler/profile-logic/processed-profile-versioning'; import type { ProfileAndProfileUpgradeInfo } from 'firefox-profiler/actions/receive-profile'; +import type { SingleColumnSortState } from '../components/shared/TreeView'; -export const CURRENT_URL_VERSION = 16; +export const CURRENT_URL_VERSION = 17; /** * This static piece of state might look like an anti-pattern, but it's a relatively @@ -190,6 +191,7 @@ type CallTreeQuery = BaseQuery & { type MarkersQuery = BaseQuery & { markerSearch: string; // "DOMEvent" marker?: MarkerIndex; // Selected marker index for the current thread, e.g. 42 + markerSort?: string; // "duration:desc,start:asc" — primary first }; type NetworkQuery = BaseQuery & { @@ -228,6 +230,7 @@ type Query = BaseQuery & { // Markers specific markerSearch?: string; marker?: MarkerIndex; + markerSort?: string; // Network specific networkSearch?: string; @@ -394,6 +397,9 @@ export function getQueryStringFromUrlState(urlState: UrlState): string { urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null ? urlState.profileSpecific.selectedMarkers[selectedThreadsKey] : undefined; + query.markerSort = convertMarkerTableSortToString( + urlState.profileSpecific.markerTableSort + ); break; case 'network-chart': query = baseQuery as NetworkQueryShape; @@ -632,10 +638,59 @@ export function stateFromLocation( ? query.hiddenThreads.split('-').map((index) => Number(index)) : null, selectedMarkers, + markerTableSort: convertMarkerTableSortFromString(query.markerSort), }, }; } +// MarkerTable sort URL encoding. The internal ColumnSortState stores the +// primary-sorted column last (newest click wins as primary); the URL puts the +// primary first for human readability. +const VALID_MARKER_SORT_COLUMNS = new Set(['start', 'duration', 'name']); + +function convertMarkerTableSortToString( + sort: SingleColumnSortState[] +): string | undefined { + if (sort.length === 0) { + return undefined; + } + // Omit when it matches the marker table's own default. + if (sort.length === 1 && sort[0].column === 'start' && sort[0].ascending) { + return undefined; + } + return sort + .slice() + .reverse() + .map((s) => `${s.column}-${s.ascending ? 'asc' : 'desc'}`) + .join('~'); +} + +function convertMarkerTableSortFromString( + raw: string | null | void +): SingleColumnSortState[] { + if (!raw) { + return []; + } + const parsed: SingleColumnSortState[] = []; + for (const part of raw.split('~')) { + const dashIndex = part.lastIndexOf('-'); + if (dashIndex === -1) { + return []; + } + const column = part.slice(0, dashIndex); + const dir = part.slice(dashIndex + 1); + if ( + !VALID_MARKER_SORT_COLUMNS.has(column) || + (dir !== 'asc' && dir !== 'desc') + ) { + return []; + } + parsed.push({ column, ascending: dir === 'asc' }); + } + // URL is primary-first; internal storage is primary-last. + return parsed.reverse(); +} + function convertGlobalTrackOrderFromString( rawString: string | null | void ): TrackIndex[] { @@ -1443,6 +1498,11 @@ const _upgraders: { .join('~'); } }, + [17]: (_processedLocation: ProcessedLocationBeforeUpgrade) => { + // Adds the optional `markerSort` query parameter for the marker table. + // No migration is necessary: older URLs simply omit it and the default + // (sort by start ascending) is used. + }, }; /** diff --git a/src/components/calltree/CallTree.tsx b/src/components/calltree/CallTree.tsx index a8e4742c61..401247eee5 100644 --- a/src/components/calltree/CallTree.tsx +++ b/src/components/calltree/CallTree.tsx @@ -111,7 +111,7 @@ class CallTreeImpl extends PureComponent { { propName: 'totalPercent', titleL10nId: '', - initialWidth: 50, + initialWidth: 55, hideDividerAfter: true, }, { @@ -120,20 +120,20 @@ class CallTreeImpl extends PureComponent { minWidth: 30, initialWidth: 70, resizable: true, - headerWidthAdjustment: 50, + headerWidthAdjustment: 55 /* totalPercent initialWidth */, }, { propName: 'self', titleL10nId: 'CallTree--tracing-ms-self', - minWidth: 30, - initialWidth: 70, + minWidth: 40, + initialWidth: 80, resizable: true, }, { propName: 'icon', titleL10nId: '', component: Icon as any, - initialWidth: 10, + initialWidth: 20, }, ]; case 'samples': @@ -141,7 +141,7 @@ class CallTreeImpl extends PureComponent { { propName: 'totalPercent', titleL10nId: '', - initialWidth: 50, + initialWidth: 55, hideDividerAfter: true, }, { @@ -150,20 +150,20 @@ class CallTreeImpl extends PureComponent { minWidth: 30, initialWidth: 70, resizable: true, - headerWidthAdjustment: 50, + headerWidthAdjustment: 55 /* totalPercent initialWidth */, }, { propName: 'self', titleL10nId: 'CallTree--samples-self', - minWidth: 30, - initialWidth: 70, + minWidth: 40, + initialWidth: 80, resizable: true, }, { propName: 'icon', titleL10nId: '', component: Icon as any, - initialWidth: 10, + initialWidth: 20, }, ]; case 'bytes': @@ -171,7 +171,7 @@ class CallTreeImpl extends PureComponent { { propName: 'totalPercent', titleL10nId: '', - initialWidth: 50, + initialWidth: 55, hideDividerAfter: true, }, { @@ -180,20 +180,20 @@ class CallTreeImpl extends PureComponent { minWidth: 30, initialWidth: 140, resizable: true, - headerWidthAdjustment: 50, + headerWidthAdjustment: 55 /* totalPercent initialWidth */, }, { propName: 'self', titleL10nId: 'CallTree--bytes-self', - minWidth: 30, - initialWidth: 90, + minWidth: 40, + initialWidth: 100, resizable: true, }, { propName: 'icon', titleL10nId: '', component: Icon as any, - initialWidth: 10, + initialWidth: 20, }, ]; default: diff --git a/src/components/marker-table/index.tsx b/src/components/marker-table/index.tsx index f5057da0b2..66354ac0a3 100644 --- a/src/components/marker-table/index.tsx +++ b/src/components/marker-table/index.tsx @@ -6,7 +6,7 @@ import { PureComponent } from 'react'; import memoize from 'memoize-immutable'; import explicitConnect from '../../utils/connect'; -import { TreeView } from '../shared/TreeView'; +import { ColumnSortState, TreeView } from '../shared/TreeView'; import { MarkerTableEmptyReasons } from './MarkerTableEmptyReasons'; import { getZeroAt, @@ -15,11 +15,15 @@ import { getCurrentTableViewOptions, } from '../../selectors/profile'; import { selectedThreadSelectors } from '../../selectors/per-thread'; -import { getSelectedThreadsKey } from '../../selectors/url-state'; +import { + getSelectedThreadsKey, + getMarkerTableSort, +} from '../../selectors/url-state'; import { changeSelectedMarker, changeRightClickedMarker, changeTableViewOptions, + changeMarkerTableSort, } from '../../actions/profile-view'; import { MarkerSettings } from '../shared/MarkerSettings'; import { formatSeconds, formatTimestamp } from '../../utils/format-numbers'; @@ -37,12 +41,21 @@ import type { TableViewOptions, SelectionContext, } from 'firefox-profiler/types'; +import type { + SingleColumnSortState, + Tree, + SortableColumn, +} from '../shared/TreeView'; import type { ConnectedProps } from '../../utils/connect'; // Limit how many characters in the description get sent to the DOM. const MAX_DESCRIPTION_CHARACTERS = 500; +const DEFAULT_MARKER_TABLE_SORT: SingleColumnSortState[] = [ + { column: 'start', ascending: true }, +]; + type MarkerDisplayData = { start: string; duration: string | null; @@ -50,7 +63,7 @@ type MarkerDisplayData = { details: string; }; -class MarkerTree { +class MarkerTree implements Tree { _getMarker: (param: MarkerIndex) => Marker; _markerIndexes: MarkerIndex[]; _zeroAt: Milliseconds; @@ -73,6 +86,16 @@ class MarkerTree { this._getMarkerLabel = getMarkerLabel; } + static _sortableColumns: SortableColumn[] = [ + { name: 'start', prefersDescending: false }, + { name: 'duration', prefersDescending: true }, + { name: 'name', prefersDescending: false }, + ]; + + getSortableColumns(): SortableColumn[] { + return MarkerTree._sortableColumns; + } + copyTable = ( format: 'plain' | 'markdown', onExceeedMaxCopyRows: (rows: number, maxRows: number) => void @@ -167,12 +190,49 @@ class MarkerTree { copy(text); }; - getRoots(): MarkerIndex[] { + getRoots(sort: ColumnSortState | null = null): MarkerIndex[] { + if (sort !== null) { + return sort.sortItemsHelper( + this._markerIndexes, + (first: MarkerIndex, second: MarkerIndex, column: string) => { + const firstValue = this._getSortValueForColumn(first, column); + const secondValue = this._getSortValueForColumn(second, column); + if (typeof firstValue === 'string') { + return firstValue.localeCompare(secondValue as string); + } + return (firstValue as number) - (secondValue as number); + } + ); + } return this._markerIndexes; } - getChildren(markerIndex: MarkerIndex): MarkerIndex[] { - return markerIndex === -1 ? this.getRoots() : []; + getChildren( + markerIndex: MarkerIndex, + sort: ColumnSortState | null = null + ): MarkerIndex[] { + return markerIndex === -1 ? this.getRoots(sort) : []; + } + + _getSortValueForColumn( + markerIndex: MarkerIndex, + column: string + ): string | number { + const marker = this._getMarker(markerIndex); + switch (column) { + case 'start': + return marker.start; + case 'duration': { + if (marker.incomplete || marker.end === null) { + return -Infinity; + } + return marker.end - marker.start; + } + case 'name': + return marker.name; + default: + throw new Error('Invalid column ' + column); + } } hasChildren(_markerIndex: MarkerIndex): boolean { @@ -180,7 +240,7 @@ class MarkerTree { } getAllDescendants() { - return new Set(); + return new Set(); } getParent(): MarkerIndex { @@ -205,10 +265,11 @@ class MarkerTree { } let duration = null; + const markerEnd = marker.end; if (marker.incomplete) { duration = 'unknown'; - } else if (marker.end !== null) { - duration = formatTimestamp(marker.end - marker.start); + } else if (markerEnd !== null) { + duration = formatTimestamp(markerEnd - marker.start); } displayData = { @@ -238,12 +299,14 @@ type StateProps = { readonly markerSchemaByName: MarkerSchemaByName; readonly getMarkerLabel: (param: MarkerIndex) => string; readonly tableViewOptions: TableViewOptions; + readonly sort: SingleColumnSortState[]; }; type DispatchProps = { readonly changeSelectedMarker: typeof changeSelectedMarker; readonly changeRightClickedMarker: typeof changeRightClickedMarker; readonly onTableViewOptionsChange: (param: TableViewOptions) => any; + readonly changeMarkerTableSort: typeof changeMarkerTableSort; }; type Props = ConnectedProps<{}, StateProps, DispatchProps>; @@ -253,22 +316,22 @@ class MarkerTableImpl extends PureComponent { { propName: 'start', titleL10nId: 'MarkerTable--start', - minWidth: 30, - initialWidth: 90, + minWidth: 35, + initialWidth: 95, resizable: true, }, { propName: 'duration', titleL10nId: 'MarkerTable--duration', - minWidth: 30, - initialWidth: 80, + minWidth: 40, + initialWidth: 90, resizable: true, }, { propName: 'name', titleL10nId: 'MarkerTable--name', - minWidth: 30, - initialWidth: 150, + minWidth: 40, + initialWidth: 160, resizable: true, }, ]; @@ -280,6 +343,11 @@ class MarkerTableImpl extends PureComponent { this._treeView = treeView; }; + _getSortedColumns = memoize( + (sort: SingleColumnSortState[]) => + new ColumnSortState(sort.length > 0 ? sort : DEFAULT_MARKER_TABLE_SORT) + ); + getMarkerTree = memoize( ( getMarker: any, @@ -331,6 +399,10 @@ class MarkerTableImpl extends PureComponent { changeSelectedMarker(threadsKey, selectedMarker, context); }; + _onColumnSortChange = (sortedColumns: ColumnSortState) => { + this.props.changeMarkerTableSort(sortedColumns.sortedColumns); + }; + _onRightClickSelection = (selectedMarker: MarkerIndex) => { const { threadsKey, changeRightClickedMarker } = this.props; changeRightClickedMarker(threadsKey, selectedMarker); @@ -366,7 +438,7 @@ class MarkerTableImpl extends PureComponent { ) : ( { indentWidth={10} viewOptions={this.props.tableViewOptions} onViewOptionsChange={this.props.onTableViewOptionsChange} + sortedColumns={this._getSortedColumns(this.props.sort)} + onColumnSortChange={this._onColumnSortChange} /> )} @@ -401,12 +475,14 @@ export const MarkerTable = explicitConnect<{}, StateProps, DispatchProps>({ markerSchemaByName: getMarkerSchemaByName(state), getMarkerLabel: selectedThreadSelectors.getMarkerTableLabelGetter(state), tableViewOptions: getCurrentTableViewOptions(state), + sort: getMarkerTableSort(state), }), mapDispatchToProps: { changeSelectedMarker, changeRightClickedMarker, onTableViewOptionsChange: (tableViewOptions) => changeTableViewOptions('marker-table', tableViewOptions), + changeMarkerTableSort, }, component: MarkerTableImpl, }); diff --git a/src/components/shared/TreeView.css b/src/components/shared/TreeView.css index 8bcb1f74a0..42e4c0ed6a 100644 --- a/src/components/shared/TreeView.css +++ b/src/components/shared/TreeView.css @@ -58,8 +58,7 @@ } .treeViewHeader { - height: 15px; - padding: 4px 0; + height: 23px; border-bottom: 1px solid var(--base-border-color); background: var(--raised-background-color); color: var(--raised-foreground-color); @@ -113,38 +112,87 @@ } .treeViewHeaderColumn { - position: relative; line-height: 15px; white-space: nowrap; } +.treeViewHeaderColumn, +.treeViewSortButton, +.treeViewRowColumn, +.treeViewRowScrolledColumns { + box-sizing: border-box; + padding-right: 5px; + padding-left: 5px; +} + .treeViewFixedColumn { overflow: hidden; text-overflow: ellipsis; } -.treeViewColumnDivider { +.treeViewSortButton { + border: 0; + background: none; + color: inherit; + font: inherit; + text-align: inherit; +} + +.treeViewSortButton:active:hover { + background-color: var(--clickable-ghost-active-background-color); +} + +.treeViewSortButton.sortInactive::after { + content: ' ▲'; + opacity: 0; +} + +.treeViewSortButton.sortAscending::after { + content: ' ▲'; +} + +.treeViewSortButton.sortDescending::after { + content: ' ▼'; +} + +.treeViewHeaderColumnDivider, +.treeViewRowColumnDivider { display: flex; - width: 20px; + width: 1px; flex: none; align-items: stretch; justify-content: center; + padding-right: 5px; + padding-left: 5px; margin-right: -5px; margin-left: -5px; } -.treeViewColumnDivider.isResizable, +.treeViewHeaderColumn, +.treeViewHeaderColumnDivider { + padding-top: 4px; + padding-bottom: 4px; +} + +.treeViewHeaderColumnDivider.isResizable { + position: relative; + z-index: var(--z-tree-view-header-divider); +} + +.treeViewHeaderColumnDivider.isResizable, .treeView.isResizingColumns { cursor: col-resize; } -.treeViewColumnDivider::before { +.treeViewRowColumnDivider::before, +.treeViewHeaderColumnDivider::before { border-right: 1px solid var(--base-border-color); content: ''; } -.treeViewColumnDivider.isResizable::before { +.treeViewHeaderColumnDivider.isResizable::before { width: 1px; + flex-shrink: 0; border-left: 1px solid var(--base-border-color); } diff --git a/src/components/shared/TreeView.tsx b/src/components/shared/TreeView.tsx index c1505e1472..4831f47403 100644 --- a/src/components/shared/TreeView.tsx +++ b/src/components/shared/TreeView.tsx @@ -51,6 +51,60 @@ export type Column> = { }>; }; +export type SingleColumnSortState = { + column: string; + ascending: boolean; +}; + +export class ColumnSortState { + sortedColumns: SingleColumnSortState[]; + + constructor(sortedColumns: SingleColumnSortState[]) { + this.sortedColumns = sortedColumns; + } + + withToggledSortForColumn( + column: string, + prefersDescending: boolean + ): ColumnSortState { + const current = this.current(); + const sortedColumns = this.sortedColumns.filter((c) => c.column !== column); + + sortedColumns.push({ + column, + ascending: + current && current.column === column + ? !current.ascending + : !prefersDescending, + }); + return new ColumnSortState(sortedColumns); + } + + current(): SingleColumnSortState | null { + return this.sortedColumns.length > 0 + ? this.sortedColumns[this.sortedColumns.length - 1] + : null; + } + + /** + * Sort `items` by all columns in `sortedColumns`, with the last column being + * the primary key. `compareColumn(a, b, column)` must return the sign of + * (a - b) for that column — i.e. ascending order. Array.prototype.sort is + * stable, so earlier-listed columns act as tiebreakers. + */ + sortItemsHelper( + items: T[], + compareColumn: (a: T, b: T, column: string) => number + ): T[] { + const sorted = items.slice(); + for (const { column, ascending } of this.sortedColumns) { + const sign = ascending ? 1 : -1; + sorted.sort((a, b) => sign * compareColumn(a, b, column)); + } + return sorted; + } +} + export type MaybeResizableColumn> = Column & { /** defaults to initialWidth */ @@ -73,6 +127,9 @@ type TreeViewHeaderProps> = { // passes the column index and the start x coordinate readonly onColumnWidthChangeStart: (param: number, x: CssPixels) => void; readonly onColumnWidthReset: (param: number) => void; + readonly onToggleSortForColumn: (column: string) => void; + readonly currentSortedColumn: SingleColumnSortState | null; + readonly sortableColumns?: Set; }; class TreeViewHeader< @@ -91,8 +148,22 @@ class TreeViewHeader< ); }; + _onToggleSortForColumn = (e: React.MouseEvent) => { + const { onToggleSortForColumn } = this.props; + const target = e.currentTarget; + if (target instanceof HTMLElement && target.dataset.column) { + onToggleSortForColumn(target.dataset.column); + } + }; + override render() { - const { fixedColumns, mainColumn, viewOptions } = this.props; + const { + fixedColumns, + mainColumn, + viewOptions, + currentSortedColumn, + sortableColumns, + } = this.props; const columnWidths = viewOptions.fixedColumnWidths; if (fixedColumns.length === 0 && !mainColumn.titleL10nId) { // If there is nothing to display in the header, do not render it. @@ -102,18 +173,54 @@ class TreeViewHeader<
{fixedColumns.map((col, i) => { const width = columnWidths[i] + (col.headerWidthAdjustment || 0); + const isSortable = sortableColumns?.has(col.propName) ?? false; + let sortClass = ''; + let ariaSort: 'ascending' | 'descending' | 'none' | undefined; + if (isSortable) { + if ( + currentSortedColumn && + currentSortedColumn.column === col.propName + ) { + sortClass = currentSortedColumn.ascending + ? 'sortAscending' + : 'sortDescending'; + ariaSort = currentSortedColumn.ascending + ? 'ascending' + : 'descending'; + } else { + sortClass = 'sortInactive'; + ariaSort = 'none'; + } + } + const cellClassName = `treeViewHeaderColumn treeViewFixedColumn ${col.propName}`; return ( - - - + {isSortable ? ( + + + + ) : ( + + + + )} {col.hideDividerAfter !== true ? ( {col.hideDividerAfter !== true ? ( - + ) : null} ); @@ -380,14 +487,9 @@ class TreeViewRowScrolledColumns< ) : null } {displayData.badge ? ( {appendageColumn ? ( {reactStringWithHighlightedSubstrings( displayData[appendageColumn.propName], @@ -429,14 +531,21 @@ class TreeViewRowScrolledColumns< } } -interface Tree> { +export type SortableColumn = { + name: string; + prefersDescending: boolean; +}; + +export interface Tree> { getDepth(nodeIndex: NodeIndex): number; - getRoots(): NodeIndex[]; + getRoots(sort: ColumnSortState | null): NodeIndex[]; getDisplayData(nodeIndex: NodeIndex): DisplayData; getParent(nodeIndex: NodeIndex): NodeIndex; - getChildren(nodeIndex: NodeIndex): NodeIndex[]; + getChildren(nodeIndex: NodeIndex, sort: ColumnSortState | null): NodeIndex[]; hasChildren(nodeIndex: NodeIndex): boolean; getAllDescendants(nodeIndex: NodeIndex): Set; + + getSortableColumns(): SortableColumn[]; // constant } type TreeViewProps> = { @@ -465,6 +574,8 @@ type TreeViewProps> = { readonly onKeyDown?: (param: React.KeyboardEvent) => void; readonly viewOptions: TableViewOptions; readonly onViewOptionsChange?: (param: TableViewOptions) => void; + readonly sortedColumns?: ColumnSortState; + readonly onColumnSortChange?: (sortedColumns: ColumnSortState) => void; }; type TreeViewState = { @@ -472,6 +583,8 @@ type TreeViewState = { readonly isResizingColumns: boolean; }; +const EMPTY_SORT_STATE = new ColumnSortState([]); + export class TreeView< DisplayData extends Record, > extends React.PureComponent, TreeViewState> { @@ -487,7 +600,7 @@ export class TreeView< initialWidth: CssPixels; } | null = null; - override state = { + override state: TreeViewState = { // This contains the current widths, while or after the user resizes them. fixedColumnWidths: null, @@ -495,6 +608,10 @@ export class TreeView< isResizingColumns: false, }; + _getSortedColumns(): ColumnSortState { + return this.props.sortedColumns ?? EMPTY_SORT_STATE; + } + // This is incremented when a column changed its size. We use this to force a // rerender of the VirtualList component. _columnSizeChangedCounter: number = 0; @@ -524,6 +641,11 @@ export class TreeView< fixedColumns.map((c) => c.initialWidth) ); + _getSortableColumnNames = memoize( + (tree: Tree): Set => + new Set(tree.getSortableColumns().map((c) => c.name)) + ); + // This returns the column widths from several possible sources, in this order: // * the current state (this means the user changed them recently, or is // currently changing them) @@ -614,7 +736,11 @@ export class TreeView< }; _computeAllVisibleRowsMemoized = memoize( - (tree: Tree, expandedNodes: Set) => { + ( + tree: Tree, + expandedNodes: Set, + sortedColumns: ColumnSortState + ) => { function _addVisibleRowsFromNode( tree: Tree, expandedNodes: Set, @@ -625,13 +751,13 @@ export class TreeView< if (!expandedNodes.has(nodeId)) { return; } - const children = tree.getChildren(nodeId); + const children = tree.getChildren(nodeId, sortedColumns); for (let i = 0; i < children.length; i++) { _addVisibleRowsFromNode(tree, expandedNodes, arr, children[i]); } } - const roots = tree.getRoots(); + const roots = tree.getRoots(sortedColumns); const allRows: NodeIndex[] = []; for (let i = 0; i < roots.length; i++) { _addVisibleRowsFromNode(tree, expandedNodes, allRows, roots[i]); @@ -723,7 +849,11 @@ export class TreeView< _getAllVisibleRows(): NodeIndex[] { const { tree } = this.props; - return this._computeAllVisibleRowsMemoized(tree, this._getExpandedNodes()); + return this._computeAllVisibleRowsMemoized( + tree, + this._getExpandedNodes(), + this._getSortedColumns() + ); } _getSpecialItems(): [NodeIndex | void, NodeIndex | void] { @@ -911,7 +1041,10 @@ export class TreeView< // Do KEY_DOWN only if the next element is a child if (this.props.tree.hasChildren(selected)) { this._selectWithKeyboard( - this.props.tree.getChildren(selected)[0] + this.props.tree.getChildren( + selected, + this._getSortedColumns() + )[0] ); } } @@ -936,6 +1069,25 @@ export class TreeView< } }; + _onToggleSortForColumn = (column: string) => { + const { onColumnSortChange } = this.props; + if (!onColumnSortChange) { + return; + } + const sortableColumn = this.props.tree + .getSortableColumns() + .find((c) => c.name === column); + if (sortableColumn === undefined) { + return; + } + onColumnSortChange( + this._getSortedColumns().withToggledSortForColumn( + column, + sortableColumn.prefersDescending + ) + ); + }; + /* This method is used by users of this component. */ /* eslint-disable-next-line react/no-unused-class-component-methods */ focus() { @@ -956,6 +1108,7 @@ export class TreeView< selectedNodeId, } = this.props; const { isResizingColumns } = this.state; + const sortableColumns = this._getSortableColumnNames(this.props.tree); return (
= (state = '', action) => { } }; +const markerTableSort: Reducer = ( + state = [], + action +) => { + switch (action.type) { + case 'CHANGE_MARKER_TABLE_SORT': + return action.sort; + default: + return state; + } +}; + const networkSearchString: Reducer = (state = '', action) => { switch (action.type) { case 'CHANGE_NETWORK_SEARCH_STRING': @@ -793,6 +806,7 @@ const profileSpecific = combineReducers({ showJsTracerSummary, tabFilter, selectedMarkers, + markerTableSort, // The timeline tracks used to be hidden and sorted by thread indexes, rather than // track indexes. The only way to migrate this information to tracks-based data is to // first retrieve the profile, so they can't be upgraded by the normal url upgrading diff --git a/src/selectors/url-state.ts b/src/selectors/url-state.ts index bf2bc8fe65..80c9733a3c 100644 --- a/src/selectors/url-state.ts +++ b/src/selectors/url-state.ts @@ -36,6 +36,7 @@ import type { import type { TabSlug } from '../app-logic/tabs-handling'; import type { MarkerRegExps } from '../profile-logic/marker-data'; +import type { SingleColumnSortState } from '../components/shared/TreeView'; import urlStateReducer from '../reducers/url-state'; import { formatMetaInfoString } from '../profile-logic/profile-metainfo'; @@ -117,6 +118,8 @@ export const getCurrentSearchString: Selector = (state) => getProfileSpecificState(state).callTreeSearchString; export const getMarkersSearchString: Selector = (state) => getProfileSpecificState(state).markersSearchString; +export const getMarkerTableSort: Selector = (state) => + getProfileSpecificState(state).markerTableSort; export const getNetworkSearchString: Selector = (state) => getProfileSpecificState(state).networkSearchString; export const getSelectedTab: Selector = (state) => diff --git a/src/test/components/MarkerTable.test.tsx b/src/test/components/MarkerTable.test.tsx index 9f9f16b6df..7de8a582dc 100644 --- a/src/test/components/MarkerTable.test.tsx +++ b/src/test/components/MarkerTable.test.tsx @@ -476,11 +476,13 @@ describe('MarkerTable', function () { ); let dividerForFirstColumn = ensureExists( - document.querySelector('.treeViewColumnDivider') + document.querySelector('.treeViewHeaderColumnDivider') ); - let firstColumn = screen.getByText('Start'); - expect(firstColumn).toHaveStyle({ width: '90px' }); - fireEvent.mouseDown(dividerForFirstColumn, { clientX: 90 }); + let firstColumn = ensureExists( + document.querySelector('.treeViewHeaderColumn.start') + ); + expect(firstColumn).toHaveStyle({ width: '95px' }); + fireEvent.mouseDown(dividerForFirstColumn, { clientX: 95 }); const body = ensureExists(document.body); @@ -505,15 +507,17 @@ describe('MarkerTable', function () { ); // Make sure the first column kept its width - firstColumn = screen.getByText('Start'); + firstColumn = ensureExists( + document.querySelector('.treeViewHeaderColumn.start') + ); expect(firstColumn).toHaveStyle({ width: '80px' }); // Now double click to reset the style. dividerForFirstColumn = ensureExists( - document.querySelector('.treeViewColumnDivider') + document.querySelector('.treeViewHeaderColumnDivider') ); fireEvent.dblClick(dividerForFirstColumn, { detail: 2 }); - expect(firstColumn).toHaveStyle({ width: '90px' }); + expect(firstColumn).toHaveStyle({ width: '95px' }); }); }); diff --git a/src/test/components/__snapshots__/MarkerTable.test.tsx.snap b/src/test/components/__snapshots__/MarkerTable.test.tsx.snap index 9e9856f6e5..0558e43a1b 100644 --- a/src/test/components/__snapshots__/MarkerTable.test.tsx.snap +++ b/src/test/components/__snapshots__/MarkerTable.test.tsx.snap @@ -205,34 +205,43 @@ exports[`MarkerTable renders some basic markers and updates when needed 1`] = `
- Start - + - Duration - + - Name - + 0s 0s UserTiming
0.002s NotifyDidPaint
0.108s unknown IPCOut
0.153s 584.00ns setTimeout
0.153s 584.00ns Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed eget magna sed magna vehicula congue id id nulla. Ut convallis, neque consequat aliquam egestas, dui urna interdum quam, id semper magna erat et nisi. Vivamus molestie quis ligula eget aliquam. Sed facilisis, turpis sed facilisis posuere, risus odio convallis velit, vitae vehicula justo risus at ipsum. Proin non porttitor neque. Vivamus fringilla ex nec iaculis cursus. Vestibulum suscipit mauris sem, vitae gravida ipsum fermentum id. Quisque pulvinar blandit ullamcorper. Donec id justo at metus scelerisque pulvinar. Proin suscipit suscipit nisi, quis tempus ipsum vulputate quis. Pellentesque sodales rutrum eros, eget pulvinar ante condimentum a. Donec accumsan, ante ut facilisis cursus, nibh quam congue eros, vitae placerat tortor magna vel lacus. Etiam odio diam, venenatis eu sollicitudin non, ultrices ut urna. Aliquam vehicula diam eu eros eleifend, ac vulputate purus faucibus.
0.158s LogMessages
0.162s 1ms FileIO
@@ -534,7 +543,7 @@ exports[`MarkerTable renders some basic markers and updates when needed 1`] = ` class="treeRowToggleButton collapsed leaf" /> foobar @@ -555,7 +564,7 @@ exports[`MarkerTable renders some basic markers and updates when needed 1`] = ` class="treeRowToggleButton collapsed leaf" />