Skip to content

Commit ebd6e33

Browse files
Adam RaineDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Ensure entry re-highlights after clearing selection
If the user clicks an entry with an ENTRY_OUTLINE overlay, it will be replaced with a ENTRY_SELECTED overlay. However, if the selection is cleared (e.g. clicking somewhere in blank space) the ENTRY_OUTLINE will not immediately be restored. This CL resolves the issue by keeping the original ENTRY_OUTLINE rendered, and using CSS to ensure that the ENTRY_SELECTED overlay is always rendered on top of it. Bug: None Change-Id: Ia5263089b0e00433e2c500643646bcff019fd5ef Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6084661 Commit-Queue: Adam Raine <[email protected]> Reviewed-by: Jack Franklin <[email protected]>
1 parent 6b0be08 commit ebd6e33

File tree

3 files changed

+12
-9
lines changed

3 files changed

+12
-9
lines changed

front_end/panels/timeline/overlays/OverlaysImpl.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ describeWithEnvironment('Overlays', () => {
329329
assert.isOk(overlayDOM);
330330
});
331331

332-
it('does not render an ENTRY_OUTLINE if the entry is also the ENTRY_SELECTED entry', async function() {
332+
it('renders an ENTRY_OUTLINE even if the entry is also the ENTRY_SELECTED entry', async function() {
333333
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
334334
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
335335
const event = charts.mainProvider.eventByIndex?.(50);
@@ -351,9 +351,9 @@ describeWithEnvironment('Overlays', () => {
351351
entry: event,
352352
});
353353
await overlays.update();
354-
const outlineNowHidden =
355-
container.querySelector<HTMLElement>('.overlay-type-ENTRY_OUTLINE')?.style.display === 'none';
356-
assert.isTrue(outlineNowHidden, 'The ENTRY_OUTLINE should be hidden');
354+
const outlineStillVisible =
355+
container.querySelector<HTMLElement>('.overlay-type-ENTRY_OUTLINE')?.style.display === 'block';
356+
assert.isTrue(outlineStillVisible, 'The ENTRY_OUTLINE should be visible');
357357
});
358358

359359
it('only ever renders a single selected overlay', async function() {

front_end/panels/timeline/overlays/OverlaysImpl.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -783,11 +783,7 @@ export class Overlays extends EventTarget {
783783
break;
784784
}
785785
case 'ENTRY_OUTLINE': {
786-
const selectedOverlay = this.overlaysOfType<EntrySelected>('ENTRY_SELECTED')?.at(0);
787-
// Check if this entry has also been selected by the user. If it has,
788-
// do not show the outline, but only show the selected outline.
789-
const outlinedEntryIsSelected = Boolean(selectedOverlay && selectedOverlay.entry === overlay.entry);
790-
if (!outlinedEntryIsSelected && this.entryIsVisibleOnChart(overlay.entry)) {
786+
if (this.entryIsVisibleOnChart(overlay.entry)) {
791787
this.#setOverlayElementVisibility(element, true);
792788
this.#positionEntryBorderOutlineType(overlay.entry, element);
793789
} else {

front_end/panels/timeline/timelineFlameChartView.css

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,16 @@
5050
}
5151
}
5252

53+
.overlay-type-ENTRY_SELECTED {
54+
/* Ensure ENTRY_SELECTED overlays are always displayed on top of ENTRY_OUTLINE overlays */
55+
z-index: 1;
56+
}
57+
5358
.overlay-type-ENTRY_OUTLINE {
5459
background-color: transparent;
5560
border-width: 1px;
61+
/* Ensure ENTRY_SELECTED overlays are always displayed on top of ENTRY_OUTLINE overlays */
62+
z-index: 0;
5663

5764
&.outline-reason-ERROR {
5865
border-color: var(--sys-color-error-bright);

0 commit comments

Comments
 (0)