Skip to content

Commit c98c103

Browse files
authored
Persist selected marker in URL and show sticky tooltip on load (#5847)
Add a `marker` URL parameter that persists the selected marker index per thread, following the same pattern as transforms. When loading a shared profile URL with a selected marker, the marker chart scrolls to it and displays a sticky tooltip. - Move selectedMarker from view state to URL state as a per-thread map - Encode/decode the marker parameter in URL query strings - Scroll the viewport to bring the selected marker into view on load - Show a sticky tooltip anchored to the marker's canvas position
1 parent d36e006 commit c98c103

File tree

11 files changed

+597
-13
lines changed

11 files changed

+597
-13
lines changed

src/app-logic/url-handling.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import type {
4040
NativeSymbolInfo,
4141
Transform,
4242
IndexIntoFrameTable,
43+
MarkerIndex,
44+
SelectedMarkersPerThread,
4345
} from 'firefox-profiler/types';
4446
import {
4547
decodeUintArrayFromUrlComponent,
@@ -49,7 +51,7 @@ import {
4951
import { tabSlugs } from '../app-logic/tabs-handling';
5052
import { StringTable } from 'firefox-profiler/utils/string-table';
5153

52-
export const CURRENT_URL_VERSION = 13;
54+
export const CURRENT_URL_VERSION = 14;
5355

5456
/**
5557
* This static piece of state might look like an anti-pattern, but it's a relatively
@@ -184,6 +186,7 @@ type CallTreeQuery = BaseQuery & {
184186

185187
type MarkersQuery = BaseQuery & {
186188
markerSearch: string; // "DOMEvent"
189+
marker?: MarkerIndex; // Selected marker index for the current thread, e.g. 42
187190
};
188191

189192
type NetworkQuery = BaseQuery & {
@@ -218,6 +221,7 @@ type Query = BaseQuery & {
218221

219222
// Markers specific
220223
markerSearch?: string;
224+
marker?: MarkerIndex;
221225

222226
// Network specific
223227
networkSearch?: string;
@@ -367,11 +371,17 @@ export function getQueryStringFromUrlState(urlState: UrlState): string {
367371
query = baseQuery as MarkersQueryShape;
368372
query.markerSearch =
369373
urlState.profileSpecific.markersSearchString || undefined;
374+
query.marker =
375+
selectedThreadsKey !== null &&
376+
urlState.profileSpecific.selectedMarkers[selectedThreadsKey] !== null
377+
? urlState.profileSpecific.selectedMarkers[selectedThreadsKey]
378+
: undefined;
370379
break;
371380
case 'network-chart':
372381
query = baseQuery as NetworkQueryShape;
373382
query.networkSearch =
374383
urlState.profileSpecific.networkSearchString || undefined;
384+
// TODO: Add support for query.marker
375385
break;
376386
case 'js-tracer': {
377387
query = baseQuery as JsTracerQueryShape;
@@ -499,6 +509,19 @@ export function stateFromLocation(
499509
transforms[selectedThreadsKey] = parseTransforms(query.transforms);
500510
}
501511

512+
// Parse the selected marker for the current thread
513+
const selectedMarkers: SelectedMarkersPerThread = {};
514+
if (
515+
selectedThreadsKey !== null &&
516+
query.marker !== undefined &&
517+
query.marker !== null
518+
) {
519+
const markerIndex = Number(query.marker);
520+
if (Number.isInteger(markerIndex) && markerIndex >= 0) {
521+
selectedMarkers[selectedThreadsKey] = markerIndex;
522+
}
523+
}
524+
502525
// tabID is used for the tab selector that we have in our full view.
503526
let tabID = null;
504527
if (query.tabID && Number.isInteger(Number(query.tabID))) {
@@ -587,6 +610,7 @@ export function stateFromLocation(
587610
legacyHiddenThreads: query.hiddenThreads
588611
? query.hiddenThreads.split('-').map((index) => Number(index))
589612
: null,
613+
selectedMarkers,
590614
},
591615
};
592616
}
@@ -1196,6 +1220,10 @@ const _upgraders: {
11961220
[13]: (_) => {
11971221
// just added the focus-self transform
11981222
},
1223+
[14]: (_) => {
1224+
// Added marker parameter for persisting highlighted markers in URLs.
1225+
// This is backward compatible as the marker parameter is optional.
1226+
},
11991227
};
12001228

12011229
for (let destVersion = 1; destVersion <= CURRENT_URL_VERSION; destVersion++) {

src/components/marker-chart/Canvas.tsx

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,49 @@ function getDefaultMarkerColors(isHighlighted: boolean) {
147147
class MarkerChartCanvasImpl extends React.PureComponent<Props> {
148148
_textMeasurement: TextMeasurement | null = null;
149149

150+
override componentDidUpdate(prevProps: Props) {
151+
const viewportDidMount =
152+
!prevProps.viewport.isSizeSet && this.props.viewport.isSizeSet;
153+
const viewportResized =
154+
this.props.viewport.isSizeSet &&
155+
this.props.viewport.containerHeight !==
156+
prevProps.viewport.containerHeight;
157+
const selectedMarkerChanged =
158+
this.props.selectedMarkerIndex !== prevProps.selectedMarkerIndex;
159+
160+
if (viewportDidMount || viewportResized || selectedMarkerChanged) {
161+
this._scrollSelectionIntoView();
162+
}
163+
}
164+
165+
_scrollSelectionIntoView = () => {
166+
const { selectedMarkerIndex, markerTimingAndBuckets, rowHeight, viewport } =
167+
this.props;
168+
169+
if (selectedMarkerIndex === null) {
170+
return;
171+
}
172+
173+
const markerIndexToTimingRow = this._getMarkerIndexToTimingRow(
174+
markerTimingAndBuckets
175+
);
176+
const rowIndex = markerIndexToTimingRow[selectedMarkerIndex];
177+
178+
if (rowIndex === undefined) {
179+
return;
180+
}
181+
182+
const y: CssPixels = rowIndex * rowHeight;
183+
const { viewportTop, viewportBottom } = viewport;
184+
185+
if (y < viewportTop || y + rowHeight > viewportBottom) {
186+
// Scroll the marker to the vertical center of the viewport.
187+
const viewportHeight = viewportBottom - viewportTop;
188+
const targetViewportTop = y + rowHeight / 2 - viewportHeight / 2;
189+
viewport.moveViewport(0, viewportTop - targetViewportTop);
190+
}
191+
};
192+
150193
/**
151194
* Get the fill, stroke, and text colors for a marker based on its schema and data.
152195
* If the marker schema has a colorField, use that field's value.
@@ -957,8 +1000,100 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
9571000
);
9581001
};
9591002

1003+
/**
1004+
* Compute canvas-relative tooltip offset for the selected marker.
1005+
* Returns null if the marker can't be located in the timing data or
1006+
* is outside the visible viewport.
1007+
*/
1008+
_getSelectedItemTooltipOffset(): {
1009+
offsetX: CssPixels;
1010+
offsetY: CssPixels;
1011+
} | null {
1012+
const { selectedMarkerIndex } = this.props;
1013+
if (selectedMarkerIndex === null) {
1014+
return null;
1015+
}
1016+
1017+
const {
1018+
rangeStart,
1019+
rangeEnd,
1020+
markerTimingAndBuckets,
1021+
rowHeight,
1022+
marginLeft,
1023+
marginRight,
1024+
viewport: {
1025+
containerWidth,
1026+
containerHeight,
1027+
viewportLeft,
1028+
viewportRight,
1029+
viewportTop,
1030+
},
1031+
} = this.props;
1032+
1033+
// Step 1: Find which row this marker is displayed in
1034+
const markerIndexToTimingRow = this._getMarkerIndexToTimingRow(
1035+
markerTimingAndBuckets
1036+
);
1037+
const rowIndex = markerIndexToTimingRow[selectedMarkerIndex];
1038+
1039+
// Step 2: Get the timing data for all markers in this row
1040+
const markerTiming = markerTimingAndBuckets[rowIndex];
1041+
if (!markerTiming || typeof markerTiming === 'string') {
1042+
// Row is empty or is a bucket label (string), not actual marker data
1043+
return null;
1044+
}
1045+
1046+
// Step 3: Find the position of our specific marker within this row's data
1047+
let markerTimingIndex = -1;
1048+
for (let i = 0; i < markerTiming.length; i++) {
1049+
if (markerTiming.index[i] === selectedMarkerIndex) {
1050+
markerTimingIndex = i;
1051+
break;
1052+
}
1053+
}
1054+
1055+
if (markerTimingIndex === -1) {
1056+
return null;
1057+
}
1058+
1059+
// Step 4: Calculate horizontal (X) position
1060+
const startTimestamp = markerTiming.start[markerTimingIndex];
1061+
const endTimestamp = markerTiming.end[markerTimingIndex];
1062+
1063+
const markerContainerWidth = containerWidth - marginLeft - marginRight;
1064+
const rangeLength: Milliseconds = rangeEnd - rangeStart;
1065+
const viewportLength: UnitIntervalOfProfileRange =
1066+
viewportRight - viewportLeft;
1067+
const startTime: UnitIntervalOfProfileRange =
1068+
(startTimestamp - rangeStart) / rangeLength;
1069+
const endTime: UnitIntervalOfProfileRange =
1070+
(endTimestamp - rangeStart) / rangeLength;
1071+
1072+
const x: CssPixels =
1073+
((startTime - viewportLeft) * markerContainerWidth) / viewportLength +
1074+
marginLeft;
1075+
const w: CssPixels =
1076+
((endTime - startTime) * markerContainerWidth) / viewportLength;
1077+
1078+
// For instant markers (start === end), use the center point.
1079+
// For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller).
1080+
const isInstantMarker = startTimestamp === endTimestamp;
1081+
const offsetX = isInstantMarker ? x : x + Math.min(w / 3, 30);
1082+
1083+
// Step 5: Calculate vertical (Y) position
1084+
// + 5 offsets the tooltip slightly below the row's top edge.
1085+
const offsetY: CssPixels = rowIndex * rowHeight - viewportTop + 5;
1086+
1087+
if (offsetY < 0 || offsetY > containerHeight) {
1088+
return null;
1089+
}
1090+
1091+
return { offsetX, offsetY };
1092+
}
1093+
9601094
override render() {
9611095
const { containerWidth, containerHeight, isDragging } = this.props.viewport;
1096+
const { selectedMarkerIndex } = this.props;
9621097

9631098
return (
9641099
<ChartCanvas
@@ -976,6 +1111,8 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
9761111
onMouseMove={this.onMouseMove}
9771112
onMouseLeave={this.onMouseLeave}
9781113
stickyTooltips={true}
1114+
selectedItem={selectedMarkerIndex}
1115+
selectedItemTooltipOffset={this._getSelectedItemTooltipOffset()}
9791116
/>
9801117
);
9811118
}

src/components/shared/chart/Canvas.tsx

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ type Props<Item> = {
3333
readonly onMouseLeave?: (e: { nativeEvent: MouseEvent }) => unknown;
3434
// Defaults to false. Set to true if the chart should persist the tooltips on click.
3535
readonly stickyTooltips?: boolean;
36+
// Selected item coming from URL or other non-click mechanism.
37+
readonly selectedItem?: Item | null;
38+
// Pre-computed canvas-relative offset for the tooltip of the selected item
39+
// (null when the item is not currently visible in the viewport).
40+
readonly selectedItemTooltipOffset?: {
41+
offsetX: CssPixels;
42+
offsetY: CssPixels;
43+
} | null;
3644
};
3745

3846
// The naming of the X and Y coordinates here correspond to the ones
@@ -359,6 +367,28 @@ export class ChartCanvas<Item> extends React.Component<
359367
this._canvas = canvas;
360368
};
361369

370+
_syncSelectedItemFromProp = () => {
371+
const { selectedItem, selectedItemTooltipOffset } = this.props;
372+
373+
if (selectedItem === undefined || selectedItem === null) {
374+
this.setState({ selectedItem: null });
375+
return;
376+
}
377+
378+
if (!selectedItemTooltipOffset || !this._canvas) {
379+
// Keep selectedItem set, so the tooltip stays at
380+
// its last known position rather than disappearing.
381+
this.setState({ selectedItem });
382+
return;
383+
}
384+
385+
const { offsetX, offsetY } = selectedItemTooltipOffset;
386+
const canvasRect = this._canvas.getBoundingClientRect();
387+
const pageX = canvasRect.left + window.scrollX + offsetX;
388+
const pageY = canvasRect.top + window.scrollY + offsetY;
389+
this.setState({ selectedItem, pageX, pageY });
390+
};
391+
362392
override UNSAFE_componentWillReceiveProps() {
363393
// It is possible that the data backing the chart has been
364394
// changed, for instance after symbolication. Clear the
@@ -377,6 +407,15 @@ export class ChartCanvas<Item> extends React.Component<
377407

378408
override componentDidMount() {
379409
window.addEventListener('profiler-theme-change', this._onThemeChange);
410+
411+
// If the viewport hasn't been laid out yet,
412+
// componentDidUpdate will pick it up once it becomes non-zero.
413+
if (
414+
this.props.selectedItem !== undefined &&
415+
this.props.containerWidth !== 0
416+
) {
417+
this._syncSelectedItemFromProp();
418+
}
380419
}
381420

382421
override componentWillUnmount() {
@@ -389,6 +428,37 @@ export class ChartCanvas<Item> extends React.Component<
389428

390429
override componentDidUpdate(prevProps: Props<Item>, prevState: State<Item>) {
391430
if (prevProps !== this.props) {
431+
if (this.props.selectedItem !== undefined) {
432+
const selectedItemChanged =
433+
this.props.selectedItem !== prevProps.selectedItem;
434+
const canvasJustGotSize =
435+
prevProps.containerWidth === 0 && this.props.containerWidth !== 0;
436+
const offsetChanged =
437+
this.props.selectedItemTooltipOffset !==
438+
prevProps.selectedItemTooltipOffset;
439+
440+
// The item was already set internally (eg, via click handler),
441+
// so skip the prop-driven sync which would overwrite that position.
442+
const alreadySetInternally =
443+
selectedItemChanged &&
444+
hoveredItemsAreEqual(
445+
this.state.selectedItem,
446+
this.props.selectedItem
447+
);
448+
449+
const offsetSyncRelevant =
450+
offsetChanged &&
451+
!selectedItemChanged &&
452+
this.state.selectedItem !== null;
453+
454+
if (
455+
!alreadySetInternally &&
456+
(selectedItemChanged || canvasJustGotSize || offsetSyncRelevant)
457+
) {
458+
this._syncSelectedItemFromProp();
459+
}
460+
}
461+
392462
if (
393463
this.state.selectedItem !== null &&
394464
prevState.selectedItem === this.state.selectedItem

src/reducers/profile-view.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ export const defaultThreadViewOptions: ThreadViewOptions = {
137137
selectedInvertedCallNodePath: [],
138138
expandedNonInvertedCallNodePaths: new PathSet(),
139139
expandedInvertedCallNodePaths: new PathSet(),
140-
selectedMarker: null,
141140
selectedNetworkMarker: null,
142141
lastSeenTransformCount: 0,
143142
};
@@ -328,10 +327,6 @@ const viewOptionsPerThread: Reducer<ThreadViewOptionsPerThreads> = (
328327
: { expandedNonInvertedCallNodePaths: expandedCallNodePaths }
329328
);
330329
}
331-
case 'CHANGE_SELECTED_MARKER': {
332-
const { threadsKey, selectedMarker } = action;
333-
return _updateThreadViewOptions(state, threadsKey, { selectedMarker });
334-
}
335330
case 'CHANGE_SELECTED_NETWORK_MARKER': {
336331
const { threadsKey, selectedNetworkMarker } = action;
337332
return _updateThreadViewOptions(state, threadsKey, {

0 commit comments

Comments
 (0)