Skip to content

Commit 8510cd9

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: fix 1/3P data table having first row selected by default
Turns out we have some logic to auto select rows. I don't know if we need it, but it was a deliberate change for a11y reasons, so I don't want to remove that functionality. So I've added a flag such that we can disable it for this table. Bug: 383567961 Change-Id: I7ab73e5f95e33cdf825476ad69a5897397f52772 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6198201 Auto-Submit: Jack Franklin <[email protected]> Reviewed-by: Adriana Ixba <[email protected]> Commit-Queue: Adriana Ixba <[email protected]>
1 parent 3f36b4b commit 8510cd9

File tree

3 files changed

+27
-3
lines changed

3 files changed

+27
-3
lines changed

front_end/panels/timeline/ThirdPartyTreeView.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,27 @@
55
import type * as Trace from '../../models/trace/trace.js';
66
import {describeWithEnvironment} from '../../testing/EnvironmentHelpers.js';
77
import {TraceLoader} from '../../testing/TraceLoader.js';
8+
import * as UI from '../../ui/legacy/legacy.js';
89

910
import * as Timeline from './timeline.js';
1011
import * as Utils from './utils/utils.js';
1112

1213
describeWithEnvironment('TimelineTreeView', function() {
1314
describe('Third party tree', function() {
15+
it('does not select the first row by default', async function() {
16+
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
17+
const treeView = new Timeline.ThirdPartyTreeView.ThirdPartyTreeViewWidget();
18+
const mapper = new Utils.EntityMapper.EntityMapper(parsedTrace);
19+
const events = [...mapper.mappings().eventsByEntity.values()].flat();
20+
treeView.setModelWithEvents(events, parsedTrace, mapper);
21+
const sel: Timeline.TimelineSelection.TimeRangeSelection = {
22+
bounds: parsedTrace.Meta.traceBounds,
23+
};
24+
const box = new UI.Widget.VBox();
25+
treeView.show(box.element);
26+
treeView.updateContents(sel);
27+
assert.isNull(treeView.dataGrid.selectedNode);
28+
});
1429
it('includes 1p and extension badges', async function() {
1530
// This trace creates 2 nodes in the tree. One representing the first party entity, and one for
1631
// a chrome extension.

front_end/panels/timeline/ThirdPartyTreeView.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ export class ThirdPartyTreeViewWidget extends TimelineTreeView.TimelineTreeView
3737
entityByEvent: Map<Trace.Types.Events.Event, Trace.Extras.ThirdParties.Entity>,
3838
}|null = null;
3939

40+
// By default the TimelineTreeView will auto-select the first row
41+
// when the grid is refreshed but for the ThirdParty view we only
42+
// want to do this when the user hovers.
43+
protected override autoSelectFirstChildOnRefresh = false;
44+
4045
constructor() {
4146
super();
4247
this.element.setAttribute('jslog', `${VisualLogging.pane('third-party-tree').track({hover: true})}`);

front_end/panels/timeline/TimelineTreeView.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,12 @@ export class TimelineTreeView extends
194194
#lastHighlightedEvent: HTMLElement|null = null;
195195
eventToTreeNode: WeakMap<Trace.Types.Events.Event, Trace.Extras.TraceTree.Node> = new WeakMap();
196196

197+
/**
198+
* Determines if the first child in the data grid will be selected
199+
* by default when refreshTree() gets called.
200+
*/
201+
protected autoSelectFirstChildOnRefresh = true;
202+
197203
constructor() {
198204
super();
199205
this.#selectedEvents = null;
@@ -279,8 +285,6 @@ export class TimelineTreeView extends
279285
this.splitWidget.hideSidebar();
280286
this.splitWidget.show(this.element);
281287
this.splitWidget.addEventListener(UI.SplitWidget.Events.SHOW_MODE_CHANGED, this.onShowModeChanged, this);
282-
283-
this.lastSelectedNodeInternal;
284288
}
285289

286290
lastSelectedNode(): Trace.Extras.TraceTree.Node|null|undefined {
@@ -424,7 +428,7 @@ export class TimelineTreeView extends
424428
this.searchableView.refreshSearch();
425429
}
426430
const rootNode = this.dataGrid.rootNode();
427-
if (rootNode.children.length > 0) {
431+
if (this.autoSelectFirstChildOnRefresh && rootNode.children.length > 0) {
428432
rootNode.children[0].select(/* supressSelectedEvent */ true);
429433
}
430434
}

0 commit comments

Comments
 (0)