From f754e9de2fba0b91206b405c12958f65c2a784cd Mon Sep 17 00:00:00 2001 From: "Christian W. Damus" Date: Fri, 15 Nov 2024 15:24:36 -0500 Subject: [PATCH] Always open the details of a new selection (#129) Ensure that whenever the selection changes and the new selection exists, show it in the details panel even if the user had previously closed it. Add a global flag to allow the host application to disable this behaviour, for example on a user preference setting. Includes incidental fixing of lint problems in edited files. Signed-off-by: Christian W. Damus --- ui/src/common/state.ts | 50 ++++++++++++++++++++++++++ ui/src/frontend/details_panel.ts | 60 +++++++++++++++++++++----------- ui/src/frontend/globals.ts | 32 +++++++++++++---- 3 files changed, 116 insertions(+), 26 deletions(-) diff --git a/ui/src/common/state.ts b/ui/src/common/state.ts index 3f431a3919..b19201f95c 100644 --- a/ui/src/common/state.ts +++ b/ui/src/common/state.ts @@ -24,6 +24,7 @@ import {TopLevelScrollSelection} from '../tracks/scroll_jank/scroll_track'; import {Direction} from './event_set'; import {TPDuration, TPTime} from './time'; import {AddTrackLikeArgs} from './actions'; +import {assertExists} from '../base/logging'; /** * A plain js object, holding objects of type |Class| keyed by string id. @@ -986,3 +987,52 @@ export function getContainingTrackIds(state: State, trackId: string): null| return result; } + +/** + * Determine whether two selections are selecting the same + * object in the trace. + * + * @param {Selection|null} a a selection or none + * @param {Selection|null} b another selection or none + * @return {boolean} whether the selections are equal + */ +export function equalSelections( + a: Selection | null, + b: Selection | null): boolean { + if (a === b) { + return true; + } + if (!!a !== !!b) { + return false; + } + + // If both were null, the first check would have returned + const one = assertExists(a); + const other = assertExists(b); + + if (one.kind !== other.kind) { + return false; + } + + const keys = Object.keys(one) as (keyof Selection)[]; + if (Object.keys(other).length !== keys.length) { + return false; + } + + for (const key of keys) { + if (!eq(one[key], other[key])) { + return false; + } + } + + return true; +} + +function eq(a: any, b: any): boolean { + // Selection properties are only primitives or arrays of primitives + if (Array.isArray(a) && Array.isArray(b)) { + return a.length === b.length && + a.every((item, index) => eq(item, b[index])); + } + return a === b; +} diff --git a/ui/src/frontend/details_panel.ts b/ui/src/frontend/details_panel.ts index c3a3bbe62b..6d596331dd 100644 --- a/ui/src/frontend/details_panel.ts +++ b/ui/src/frontend/details_panel.ts @@ -19,7 +19,7 @@ import {isEmptyData} from '../common/aggregation_data'; import {LogExists, LogExistsKey} from '../common/logs'; import {pluginManager} from '../common/plugins'; import {addSelectionChangeObserver} from '../common/selection_observer'; -import {Selection} from '../common/state'; +import {equalSelections, Selection} from '../common/state'; import {DebugSliceDetailsTab} from '../tracks/debug/details_tab'; import {SCROLL_JANK_PLUGIN_ID} from '../tracks/scroll_jank'; import {TOP_LEVEL_SCROLL_KIND} from '../tracks/scroll_jank/scroll_track'; @@ -57,9 +57,10 @@ function getDetailsHeight() { } function getFullScreenHeight(dom?: Element) { - for(const selector of globals.frontendLocalState.detailsFullScreenSelectors) { + const selectors = globals.frontendLocalState.detailsFullScreenSelectors; + for (const selector of selectors) { const element = dom?.closest(selector) || document.querySelector(selector); - if(element != null) { + if (element != null) { return element.clientHeight; } } @@ -81,6 +82,7 @@ interface DragHandleAttrs { resize: (height: number) => void; tabs: Tab[]; currentTabKey?: string; + forceOpen?: boolean; } class DragHandle implements m.ClassComponent { @@ -94,6 +96,21 @@ class DragHandle implements m.ClassComponent { private fullscreenHeight = getDetailsHeight(); private dom?: Element = undefined; + private toggleClosed() { + if (this.height === DRAG_HANDLE_HEIGHT_PX) { + this.isClosed = false; + if (this.previousHeight === 0) { + this.previousHeight = getDetailsHeight(); + } + this.resize(this.previousHeight); + } else { + this.isFullscreen = false; + this.isClosed = true; + this.previousHeight = this.height; + this.resize(DRAG_HANDLE_HEIGHT_PX); + } + } + oncreate({dom, attrs}: m.CVnodeDOM) { this.resize = attrs.resize; this.height = attrs.height; @@ -145,6 +162,11 @@ class DragHandle implements m.ClassComponent { }, tab.name); }; + + if (attrs.forceOpen && this.isClosed) { + this.toggleClosed(); + } + return m( '.handle', m('.tabs', attrs.tabs.map(renderTab)), @@ -164,18 +186,7 @@ class DragHandle implements m.ClassComponent { m('i.material-icons', { onclick: () => { - if (this.height === DRAG_HANDLE_HEIGHT_PX) { - this.isClosed = false; - if (this.previousHeight === 0) { - this.previousHeight = getDetailsHeight(); - } - this.resize(this.previousHeight); - } else { - this.isFullscreen = false; - this.isClosed = true; - this.previousHeight = this.height; - this.resize(DRAG_HANDLE_HEIGHT_PX); - } + this.toggleClosed(); globals.rafScheduler.scheduleFullRedraw(); }, title, @@ -244,6 +255,7 @@ addSelectionChangeObserver(handleSelectionChange); export class DetailsPanel implements m.ClassComponent { private detailsHeight = getDetailsHeight(); + private previousSelection: Selection | null = null; view() { interface DetailsPanel { @@ -265,6 +277,9 @@ export class DetailsPanel implements m.ClassComponent { } const curSelection = globals.state.currentSelection; + const shouldOpenDetails = globals.alwaysOpenDetailsOnSelectionChange && + !equalSelections(curSelection, this.previousSelection); + this.previousSelection = curSelection; if (curSelection) { switch (curSelection.kind) { case 'NOTE': @@ -281,7 +296,7 @@ export class DetailsPanel implements m.ClassComponent { break; case 'SLICE': detailsPanels.push({ - key: 'current_selection', + key: CURRENT_SELECTION_TAG, name: 'Current Selection', vnode: m(SliceDetailsPanel, { key: 'slice', @@ -290,7 +305,7 @@ export class DetailsPanel implements m.ClassComponent { break; case 'COUNTER': detailsPanels.push({ - key: 'current_selection', + key: CURRENT_SELECTION_TAG, name: 'Current Selection', vnode: m(CounterDetailsPanel, { key: 'counter', @@ -300,14 +315,14 @@ export class DetailsPanel implements m.ClassComponent { case 'PERF_SAMPLES': case 'HEAP_PROFILE': detailsPanels.push({ - key: 'current_selection', + key: CURRENT_SELECTION_TAG, name: 'Current Selection', vnode: m(FlamegraphDetailsPanel, {key: 'flamegraph'}), }); break; case 'CPU_PROFILE_SAMPLE': detailsPanels.push({ - key: 'current_selection', + key: CURRENT_SELECTION_TAG, name: 'Current Selection', vnode: m(CpuProfileDetailsPanel, { key: 'cpu_profile_sample', @@ -316,7 +331,7 @@ export class DetailsPanel implements m.ClassComponent { break; case 'CHROME_SLICE': detailsPanels.push({ - key: 'current_selection', + key: CURRENT_SELECTION_TAG, name: 'Current Selection', vnode: m(ChromeSliceDetailsPanel, {key: 'chrome_slice'}), }); @@ -325,6 +340,10 @@ export class DetailsPanel implements m.ClassComponent { break; } } + + const openDetails = shouldOpenDetails && detailsPanels.length > 0 && + detailsPanels[detailsPanels.length - 1].key === CURRENT_SELECTION_TAG; + if (hasLogs()) { detailsPanels.push({ key: 'android_logs', @@ -403,6 +422,7 @@ export class DetailsPanel implements m.ClassComponent { }, }, m(DragHandle, { + forceOpen: openDetails, resize: (height: number) => { this.detailsHeight = Math.max(height, DRAG_HANDLE_HEIGHT_PX); }, diff --git a/ui/src/frontend/globals.ts b/ui/src/frontend/globals.ts index 105fd62619..5e4271ac4f 100644 --- a/ui/src/frontend/globals.ts +++ b/ui/src/frontend/globals.ts @@ -294,10 +294,12 @@ class Globals { private _trackFilteringEnabled = false; private _engineReadyObservers: ((engine: EngineConfig) => void)[] = []; private _timelineSubsetRange?: TimespanProvider = undefined; + private _alwaysOpenDetailsOnSelectionChange = true; // Init from session storage since correct value may be required very early on - private _customContentSecurityPolicy = window.sessionStorage.getItem(CUSTOM_CONTENT_SECURITY_POLICY); + private _customContentSecurityPolicy = + window.sessionStorage.getItem(CUSTOM_CONTENT_SECURITY_POLICY); private _currentSearchResults: CurrentSearchResults = { sliceIds: new Float64Array(0), @@ -699,7 +701,8 @@ class Globals { return this._httpRpcEngineCustomizer; } - set httpRpcEngineCustomizer(httpRpcEngineCustomizer: HttpRcpEngineCustomizer | undefined) { + set httpRpcEngineCustomizer( + httpRpcEngineCustomizer: HttpRcpEngineCustomizer | undefined) { this._httpRpcEngineCustomizer = httpRpcEngineCustomizer; } @@ -715,8 +718,10 @@ class Globals { return this._promptToLoadFromTraceProcessorShell; } - set promptToLoadFromTraceProcessorShell(promptToLoadFromTraceProcessorShell: boolean) { - this._promptToLoadFromTraceProcessorShell = promptToLoadFromTraceProcessorShell; + set promptToLoadFromTraceProcessorShell( + promptToLoadFromTraceProcessorShell: boolean) { + this._promptToLoadFromTraceProcessorShell = + promptToLoadFromTraceProcessorShell; } get trackFilteringEnabled(): boolean { @@ -741,9 +746,11 @@ class Globals { } } - /** Register an engine ready observer. + /** + * Register an engine ready observer. * - * returns a cleanup function, to be called to unregister. + * @param {Function} observer an engine-ready call-back function to register + * @return {Function} a cleanup function, to be called to unregister */ addEngineReadyObserver(observer: (engine: EngineConfig) => void): ()=>void { this._engineReadyObservers.push(observer); @@ -773,6 +780,19 @@ class Globals { this._timelineSubsetRange = timeSpan; } + /** + * Whether the details pane is always opened to show the new selection + * on selection change (if the selection is not cleared), even when the + * user had previously closed it. Initially this is `true`. + */ + get alwaysOpenDetailsOnSelectionChange() { + return this._alwaysOpenDetailsOnSelectionChange; + }; + + set alwaysOpenDetailsOnSelectionChange(always: boolean) { + this._alwaysOpenDetailsOnSelectionChange = always; + } + makeSelection(action: DeferredAction<{}>, tabToOpen = 'current_selection') { // A new selection should cancel the current search selection. globals.dispatch(Actions.setSearchIndex({index: -1}));