Skip to content

Commit

Permalink
Always open the details of a new selection (#129)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@cdamus
cdamus authored Nov 15, 2024
1 parent bc750cf commit f754e9d
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 26 deletions.
50 changes: 50 additions & 0 deletions ui/src/common/state.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
60 changes: 40 additions & 20 deletions ui/src/frontend/details_panel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -81,6 +82,7 @@ interface DragHandleAttrs {
resize: (height: number) => void;
tabs: Tab[];
currentTabKey?: string;
forceOpen?: boolean;
}

class DragHandle implements m.ClassComponent<DragHandleAttrs> {
Expand All @@ -94,6 +96,21 @@ class DragHandle implements m.ClassComponent<DragHandleAttrs> {
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<DragHandleAttrs>) {
this.resize = attrs.resize;
this.height = attrs.height;
Expand Down Expand Up @@ -145,6 +162,11 @@ class DragHandle implements m.ClassComponent<DragHandleAttrs> {
},
tab.name);
};

if (attrs.forceOpen && this.isClosed) {
this.toggleClosed();
}

return m(
'.handle',
m('.tabs', attrs.tabs.map(renderTab)),
Expand All @@ -164,18 +186,7 @@ class DragHandle implements m.ClassComponent<DragHandleAttrs> {
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,
Expand Down Expand Up @@ -244,6 +255,7 @@ addSelectionChangeObserver(handleSelectionChange);

export class DetailsPanel implements m.ClassComponent {
private detailsHeight = getDetailsHeight();
private previousSelection: Selection | null = null;

view() {
interface DetailsPanel {
Expand All @@ -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':
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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'}),
});
Expand All @@ -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',
Expand Down Expand Up @@ -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);
},
Expand Down
32 changes: 26 additions & 6 deletions ui/src/frontend/globals.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -699,7 +701,8 @@ class Globals {
return this._httpRpcEngineCustomizer;
}

set httpRpcEngineCustomizer(httpRpcEngineCustomizer: HttpRcpEngineCustomizer | undefined) {
set httpRpcEngineCustomizer(
httpRpcEngineCustomizer: HttpRcpEngineCustomizer | undefined) {
this._httpRpcEngineCustomizer = httpRpcEngineCustomizer;
}

Expand All @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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}));
Expand Down

0 comments on commit f754e9d

Please sign in to comment.