Skip to content

Commit 1cc967a

Browse files
committed
Make network markers in the network panel sticky on click
When clicking a marker in the Network panel, its tooltip is now persisted, matching the Marker Chart behavior. This enables interaction with tooltip content like copying text or clicking the filter button. The existing behavior of displaying the tooltip on hover is kept as well. Because of that, it should now be easier to compare two markers. - Add click coordinate tracking to NetworkChartRow state for sticky tooltip positioning - Show filter button only in sticky (clicked) tooltips - Toggle selection off when re-clicking the same row - Dismiss sticky tooltip on Escape key - Add 7 new tests covering the sticky tooltip related behavior
1 parent 2eba5df commit 1cc967a

4 files changed

Lines changed: 231 additions & 14 deletions

File tree

src/components/network-chart/NetworkChartRow.tsx

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -308,18 +308,28 @@ type State = {
308308
pageX: CssPixels;
309309
pageY: CssPixels;
310310
hovered: boolean | null;
311+
clickPageX: CssPixels | null;
312+
clickPageY: CssPixels | null;
311313
};
312314

313315
export class NetworkChartRow extends React.PureComponent<
314316
NetworkChartRowProps,
315317
State
316318
> {
317-
override state = {
319+
override state: State = {
318320
pageX: 0,
319321
pageY: 0,
320322
hovered: false,
323+
clickPageX: null,
324+
clickPageY: null,
321325
};
322326

327+
override componentDidUpdate(prevProps: NetworkChartRowProps) {
328+
if (prevProps.isSelected && !this.props.isSelected) {
329+
this.setState({ clickPageX: null, clickPageY: null });
330+
}
331+
}
332+
323333
_hoverIn = (event: React.MouseEvent<HTMLDivElement>) => {
324334
const pageX = event.pageX;
325335
const pageY = event.pageY;
@@ -348,6 +358,7 @@ export class NetworkChartRow extends React.PureComponent<
348358
_onMouseDown = (e: React.MouseEvent<HTMLDivElement>) => {
349359
const { markerIndex, onLeftClick, onRightClick } = this.props;
350360
if (e.button === 0) {
361+
this.setState({ clickPageX: e.pageX, clickPageY: e.pageY });
351362
if (onLeftClick) {
352363
onLeftClick(markerIndex);
353364
}
@@ -467,6 +478,17 @@ export class NetworkChartRow extends React.PureComponent<
467478
}
468479
);
469480

481+
const clickX = this.state.clickPageX;
482+
const clickY = this.state.clickPageY;
483+
484+
const isSticky = isSelected && clickX !== null && clickY !== null;
485+
const showTooltip =
486+
shouldDisplayTooltips() && (this.state.hovered || isSticky);
487+
488+
// When sticky, use the click coordinates; otherwise use the current mouse position.
489+
const tooltipX = isSticky ? clickX : this.state.pageX;
490+
const tooltipY = isSticky ? clickY : this.state.pageY;
491+
470492
return (
471493
<div
472494
// The className below is responsible for the blue hover effect
@@ -488,19 +510,19 @@ export class NetworkChartRow extends React.PureComponent<
488510
width={width}
489511
timeRange={timeRange}
490512
/>
491-
{shouldDisplayTooltips() && this.state.hovered ? (
492-
// This magic value "5" avoids the tooltip of being too close of the
493-
// row, especially when we mouseEnter the row from the top edge.
494-
<Tooltip mouseX={this.state.pageX} mouseY={this.state.pageY + 5}>
513+
{showTooltip ? (
514+
<Tooltip
515+
mouseX={tooltipX}
516+
mouseY={tooltipY}
517+
className={isSticky ? 'clickable' : undefined}
518+
>
495519
<TooltipMarker
496520
className="tooltipNetwork"
497521
markerIndex={markerIndex}
498522
marker={marker}
499523
threadsKey={this.props.threadsKey}
500524
restrictHeightWidth={true}
501-
// Network Chart doesn't have sticky tooltips yet. But we should convert it
502-
// to false once we implement sticky tooltips for the network chart.
503-
hideFilterButton={true}
525+
hideFilterButton={!isSticky}
504526
/>
505527
</Tooltip>
506528
) : null}

src/components/network-chart/index.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,14 @@ class NetworkChartImpl extends React.PureComponent<Props> {
143143
};
144144

145145
_onKeyDown = (event: React.KeyboardEvent<HTMLElement>) => {
146+
if (event.key === 'Escape') {
147+
const { threadsKey, changeSelectedNetworkMarker } = this.props;
148+
event.stopPropagation();
149+
event.preventDefault();
150+
changeSelectedNetworkMarker(threadsKey, null, { source: 'keyboard' });
151+
return;
152+
}
153+
146154
const hasModifier = event.ctrlKey || event.altKey;
147155
const isNavigationKey =
148156
event.key.startsWith('Arrow') ||
@@ -232,15 +240,21 @@ class NetworkChartImpl extends React.PureComponent<Props> {
232240
};
233241

234242
_onLeftClick = (selectedNetworkMarkerIndex: MarkerIndex) => {
235-
this._onSelectionChange(selectedNetworkMarkerIndex, { source: 'pointer' });
243+
if (this.props.selectedNetworkMarkerIndex === selectedNetworkMarkerIndex) {
244+
this._onSelectionChange(null, { source: 'pointer' });
245+
} else {
246+
this._onSelectionChange(selectedNetworkMarkerIndex, {
247+
source: 'pointer',
248+
});
249+
}
236250
};
237251

238252
_selectWithKeyboard(selectedNetworkMarkerIndex: MarkerIndex) {
239253
this._onSelectionChange(selectedNetworkMarkerIndex, { source: 'keyboard' });
240254
}
241255

242256
_onSelectionChange = (
243-
selectedNetworkMarkerIndex: MarkerIndex,
257+
selectedNetworkMarkerIndex: MarkerIndex | null,
244258
context: SelectionContext
245259
) => {
246260
const { threadsKey, changeSelectedNetworkMarker } = this.props;

src/components/tooltip/Marker.tsx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ import {
2222
getProcessIdToNameMap,
2323
getThreadSelectorsFromThreadsKey,
2424
} from 'firefox-profiler/selectors';
25-
import { changeMarkersSearchString } from 'firefox-profiler/actions/profile-view';
25+
import {
26+
changeMarkersSearchString,
27+
changeNetworkSearchString,
28+
} from 'firefox-profiler/actions/profile-view';
2629

2730
import {
2831
TooltipNetworkMarkerPhases,
@@ -112,6 +115,7 @@ type StateProps = {
112115

113116
type DispatchProps = {
114117
readonly changeMarkersSearchString: typeof changeMarkersSearchString;
118+
readonly changeNetworkSearchString: typeof changeNetworkSearchString;
115119
};
116120

117121
type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
@@ -493,10 +497,18 @@ class MarkerTooltipContents extends React.PureComponent<Props> {
493497
}
494498

495499
_onFilterButtonClick = () => {
496-
const { markerIndex, getMarkerSearchTerm, changeMarkersSearchString } =
497-
this.props;
500+
const {
501+
marker,
502+
markerIndex,
503+
getMarkerSearchTerm,
504+
changeMarkersSearchString,
505+
changeNetworkSearchString,
506+
} = this.props;
498507
const searchTerm = getMarkerSearchTerm(markerIndex);
499508
changeMarkersSearchString(searchTerm);
509+
if (marker.data && marker.data.type === 'Network') {
510+
changeNetworkSearchString(searchTerm);
511+
}
500512
};
501513

502514
/**
@@ -718,7 +730,7 @@ const ConnectedMarkerTooltipContents = explicitConnect<
718730
categories: getCategories(state),
719731
};
720732
},
721-
mapDispatchToProps: { changeMarkersSearchString },
733+
mapDispatchToProps: { changeMarkersSearchString, changeNetworkSearchString },
722734
component: MarkerTooltipContents,
723735
});
724736

src/test/components/NetworkChart.test.tsx

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,175 @@ describe('Network Chart/tooltip behavior', () => {
683683
});
684684
});
685685

686+
describe('Network Chart/sticky tooltip behavior', () => {
687+
beforeEach(addRootOverlayElement);
688+
afterEach(removeRootOverlayElement);
689+
690+
function setupForStickyTooltip(uris: string[] = ['https://mozilla.org/1']) {
691+
const markers: TestDefinedMarker[] = [];
692+
uris.forEach((uri, i) => {
693+
markers.push(
694+
...getNetworkMarkers({
695+
uri,
696+
id: i,
697+
startTime: 10 + i * 10,
698+
endTime: 19 + i * 10,
699+
})
700+
);
701+
});
702+
703+
const result = setupWithPayload(markers);
704+
const { container } = result;
705+
706+
function rowItems(): HTMLElement[] {
707+
return Array.from(
708+
container.querySelectorAll('.networkChartRowItem')
709+
) as HTMLElement[];
710+
}
711+
712+
return { ...result, rowItems };
713+
}
714+
715+
it('persists tooltip when clicking a row (sticky)', () => {
716+
const { rowItem, getByTestId, getAllByTestId } = setupForStickyTooltip();
717+
const row = rowItem();
718+
719+
// Hover to show tooltip
720+
fireEvent(row, getMouseEvent('mouseover', { pageX: 25, pageY: 25 }));
721+
expect(getByTestId('tooltip')).toBeInTheDocument();
722+
723+
// Click to make sticky
724+
fireFullClick(row, { pageX: 25, pageY: 25 });
725+
726+
// Mouse out — tooltip should still be present
727+
fireEvent(row, getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
728+
expect(getAllByTestId('tooltip').length).toBeGreaterThanOrEqual(1);
729+
730+
// Verify the tooltip has the clickable class
731+
const tooltips = getAllByTestId('tooltip');
732+
const hasClickable = tooltips.some((t) =>
733+
t.classList.contains('clickable')
734+
);
735+
expect(hasClickable).toBe(true);
736+
});
737+
738+
it('dismisses sticky tooltip when clicking the same row again', () => {
739+
const { rowItem, getByTestId, queryByTestId } = setupForStickyTooltip();
740+
const row = rowItem();
741+
742+
// Click to make sticky
743+
fireFullClick(row, { pageX: 25, pageY: 25 });
744+
fireEvent(row, getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
745+
expect(getByTestId('tooltip')).toBeInTheDocument();
746+
747+
// Click again to dismiss
748+
fireFullClick(row, { pageX: 25, pageY: 25 });
749+
fireEvent(row, getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
750+
expect(queryByTestId('tooltip')).not.toBeInTheDocument();
751+
});
752+
753+
it('moves sticky tooltip when clicking a different row', () => {
754+
const { rowItems, queryAllByTestId } = setupForStickyTooltip([
755+
'https://mozilla.org/1',
756+
'https://mozilla.org/2',
757+
]);
758+
const rows = rowItems();
759+
expect(rows.length).toBe(2);
760+
761+
// Click first row to make sticky
762+
fireFullClick(rows[0], { pageX: 25, pageY: 25 });
763+
fireEvent(rows[0], getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
764+
765+
let tooltips = queryAllByTestId('tooltip');
766+
expect(tooltips.length).toBe(1);
767+
expect(tooltips[0]).toHaveClass('clickable');
768+
769+
// Click second row — first row tooltip should go, second should appear
770+
fireFullClick(rows[1], { pageX: 25, pageY: 50 });
771+
fireEvent(rows[1], getMouseEvent('mouseout', { pageX: 25, pageY: 50 }));
772+
773+
tooltips = queryAllByTestId('tooltip');
774+
expect(tooltips.length).toBe(1);
775+
expect(tooltips[0]).toHaveClass('clickable');
776+
});
777+
778+
it('dismisses sticky tooltip on Escape key', () => {
779+
const { rowItem, container, getByTestId, queryByTestId } =
780+
setupForStickyTooltip();
781+
const row = rowItem();
782+
783+
// Click to make sticky
784+
fireFullClick(row, { pageX: 25, pageY: 25 });
785+
fireEvent(row, getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
786+
expect(getByTestId('tooltip')).toBeInTheDocument();
787+
788+
// Press Escape
789+
const treeViewBody = ensureExists(
790+
container.querySelector('.treeViewBody'),
791+
`Couldn't find the tree view body`
792+
);
793+
fireEvent.keyDown(treeViewBody, { key: 'Escape' });
794+
expect(queryByTestId('tooltip')).not.toBeInTheDocument();
795+
});
796+
797+
it('shows filter button only in sticky tooltip', () => {
798+
const { rowItem } = setupForStickyTooltip();
799+
const row = rowItem();
800+
801+
// Hover-only tooltip should hide filter button
802+
fireEvent(row, getMouseEvent('mouseover', { pageX: 25, pageY: 25 }));
803+
expect(
804+
document.querySelector('.tooltipTitleFilterButton')
805+
).not.toBeInTheDocument();
806+
807+
// Click to make sticky — filter button should appear
808+
fireFullClick(row, { pageX: 25, pageY: 25 });
809+
expect(
810+
document.querySelector('.tooltipTitleFilterButton')
811+
).toBeInTheDocument();
812+
});
813+
814+
it('filters network panel when clicking filter button in sticky tooltip', () => {
815+
const { rowItems } = setupForStickyTooltip([
816+
'https://mozilla.org/1',
817+
'https://example.com/2',
818+
]);
819+
const rows = rowItems();
820+
821+
// Click first row to make sticky
822+
fireFullClick(rows[0], { pageX: 25, pageY: 25 });
823+
824+
// Click the filter button
825+
const filterButton = ensureExists(
826+
document.querySelector('.tooltipTitleFilterButton'),
827+
`Couldn't find the filter button`
828+
) as HTMLElement;
829+
fireFullClick(filterButton);
830+
831+
// Network search string should be set, filtering down the rows
832+
const rowsAfter = rowItems();
833+
expect(rowsAfter.length).toBeLessThan(rows.length);
834+
});
835+
836+
it('hover tooltip on another row works alongside sticky tooltip', () => {
837+
const { rowItems, queryAllByTestId } = setupForStickyTooltip([
838+
'https://mozilla.org/1',
839+
'https://mozilla.org/2',
840+
]);
841+
const rows = rowItems();
842+
843+
// Click row 1 to make sticky
844+
fireFullClick(rows[0], { pageX: 25, pageY: 25 });
845+
fireEvent(rows[0], getMouseEvent('mouseout', { pageX: 25, pageY: 25 }));
846+
847+
// Hover row 2 — both tooltips should be visible
848+
fireEvent(rows[1], getMouseEvent('mouseover', { pageX: 25, pageY: 50 }));
849+
850+
const tooltips = queryAllByTestId('tooltip');
851+
expect(tooltips.length).toBe(2);
852+
});
853+
});
854+
686855
describe('calltree/ProfileCallTreeView navigation keys', () => {
687856
beforeEach(addRootOverlayElement);
688857
afterEach(removeRootOverlayElement);

0 commit comments

Comments
 (0)