Skip to content

Commit a0f11d6

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
UI Eng Vision: migrate Timeline.RelatedInsightChips
Fixed: 407941548 Change-Id: Ifad450e2fd55694481948f59706929a2c425e485 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6760831 Commit-Queue: Jack Franklin <jacktfranklin@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org>
1 parent 0c4a9c0 commit a0f11d6

File tree

8 files changed

+330
-266
lines changed

8 files changed

+330
-266
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,6 +1987,7 @@ grd_files_unbundled_sources = [
19871987
"front_end/panels/timeline/overlays/components/timeRangeOverlay.css.js",
19881988
"front_end/panels/timeline/overlays/components/timespanBreakdownOverlay.css.js",
19891989
"front_end/panels/timeline/thirdPartyTreeView.css.js",
1990+
"front_end/panels/timeline/timelineDetailsView.css.js",
19901991
"front_end/panels/timeline/timelineFlameChartView.css.js",
19911992
"front_end/panels/timeline/timelineFlamechartPopover.css.js",
19921993
"front_end/panels/timeline/timelineHistoryManager.css.js",

front_end/panels/timeline/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ devtools_pre_built("easter-egg") {
4141
generate_css("css_files") {
4242
sources = [
4343
"thirdPartyTreeView.css",
44+
"timelineDetailsView.css",
4445
"timelineFlameChartView.css",
4546
"timelineFlamechartPopover.css",
4647
"timelineHistoryManager.css",

front_end/panels/timeline/TimelineDetailsView.test.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
// found in the LICENSE file.
44

55
import * as Trace from '../../models/trace/trace.js';
6-
import {doubleRaf} from '../../testing/DOMHelpers.js';
6+
import {doubleRaf, raf, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
77
import {describeWithEnvironment} from '../../testing/EnvironmentHelpers.js';
88
import {TraceLoader} from '../../testing/TraceLoader.js';
99

10+
import * as Components from './components/components.js';
1011
import * as Timeline from './timeline.js';
1112

1213
class MockViewDelegate implements Timeline.TimelinePanel.TimelineModeViewDelegate {
@@ -28,6 +29,8 @@ describeWithEnvironment('TimelineDetailsView', function() {
2829
const {parsedTrace, insights} = await TraceLoader.traceEngine(this, 'lcp-web-font.json.gz');
2930
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
3031

32+
renderElementIntoDOM(detailsView);
33+
3134
const networkRequests = parsedTrace.NetworkRequests.byTime;
3235
const cssRequest = networkRequests.find(request => {
3336
return request.args.data.url === 'https://chromedevtools.github.io/performance-stories/lcp-web-font/app.css';
@@ -37,30 +40,32 @@ describeWithEnvironment('TimelineDetailsView', function() {
3740
}
3841
const selection = Timeline.TimelineSelection.selectionFromEvent(cssRequest);
3942

43+
// Set up a related insight to test the rendering of the chips
44+
const relatedInsights: Components.RelatedInsightChips.EventToRelatedInsightsMap = new Map([
45+
[cssRequest, [{insightLabel: 'Test insight', activateInsight: () => {}, messages: []}]],
46+
]);
47+
4048
await detailsView.setModel({
4149
parsedTrace,
4250
selectedEvents: null,
4351
traceInsightsSets: insights,
44-
eventToRelatedInsightsMap: null,
52+
eventToRelatedInsightsMap: relatedInsights,
4553
entityMapper: null
4654
});
4755
await detailsView.setSelection(selection);
56+
await raf();
4857

4958
const detailsContentElement = detailsView.getDetailsContentElementForTest();
50-
assert.deepEqual(
51-
Array.from(detailsContentElement.children).map(n => n.localName),
52-
['devtools-performance-network-request-details']);
53-
54-
const content = detailsContentElement.firstElementChild?.shadowRoot;
55-
assert(content);
56-
assert.lengthOf(content.querySelectorAll('div.network-request-details-row'), 9);
57-
assert.lengthOf(content.querySelectorAll('div.network-request-details-item'), 2);
58-
assert.lengthOf(content.querySelectorAll('devtools-related-insight-chips'), 1);
59+
assert.instanceOf(
60+
detailsContentElement.querySelector('devtools-performance-network-request-details'),
61+
Components.NetworkRequestDetails.NetworkRequestDetails);
62+
assert.instanceOf(detailsContentElement.querySelector('.insight-label'), HTMLElement);
5963
});
6064

6165
it('displays the details for a frame correctly', async function() {
6266
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
6367
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
68+
renderElementIntoDOM(detailsView);
6469
await detailsView.setModel({
6570
parsedTrace,
6671
selectedEvents: null,
@@ -87,6 +92,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
8792
it('renders the layout shift component for a single layout shift', async function() {
8893
const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
8994
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
95+
renderElementIntoDOM(detailsView);
9096
await detailsView.setModel({
9197
parsedTrace,
9298
selectedEvents: null,
@@ -109,6 +115,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
109115
it('renders the layout shift component for a selected cluster', async function() {
110116
const {parsedTrace} = await TraceLoader.traceEngine(this, 'shift-attribution.json.gz');
111117
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
118+
renderElementIntoDOM(detailsView);
112119
await detailsView.setModel({
113120
parsedTrace,
114121
selectedEvents: null,
@@ -131,6 +138,7 @@ describeWithEnvironment('TimelineDetailsView', function() {
131138
it('updates the range details when the user has a range selected', async function() {
132139
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
133140
const detailsView = new Timeline.TimelineDetailsView.TimelineDetailsPane(mockViewDelegate);
141+
renderElementIntoDOM(detailsView);
134142
await detailsView.setModel({
135143
parsedTrace,
136144
// We have to set selected events for the range selection UI to be drawn
@@ -146,9 +154,11 @@ describeWithEnvironment('TimelineDetailsView', function() {
146154
bounds.max,
147155
);
148156
await detailsView.setSelection(selection);
157+
await raf();
158+
149159
const detailsContentElement = detailsView.getDetailsContentElementForTest();
150160
const component = detailsContentElement.querySelector<HTMLElement>('devtools-performance-timeline-summary');
151161
const range = component?.shadowRoot?.querySelector<HTMLElement>('.summary-range');
152-
assert.strictEqual(range?.innerText, 'Range: 0 ms – 5.39 s');
162+
assert.strictEqual(range?.innerText, 'Range: 0 ms – 5.39 s');
153163
});
154164
});

front_end/panels/timeline/TimelineDetailsView.ts

Lines changed: 87 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ import * as Trace from '../../models/trace/trace.js';
1111
import * as TraceBounds from '../../services/trace_bounds/trace_bounds.js';
1212
import * as Components from '../../ui/legacy/components/utils/utils.js';
1313
import * as UI from '../../ui/legacy/legacy.js';
14+
import {html, nothing, render} from '../../ui/lit/lit.js';
1415
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
1516

1617
import * as TimelineComponents from './components/components.js';
1718
import {EventsTimelineTreeView} from './EventsTimelineTreeView.js';
1819
import {Tracker} from './FreshRecording.js';
1920
import {targetForEvent} from './TargetForEvent.js';
2021
import {ThirdPartyTreeViewWidget} from './ThirdPartyTreeView.js';
22+
import detailsViewStyles from './timelineDetailsView.css.js';
2123
import {TimelineLayersView} from './TimelineLayersView.js';
2224
import {TimelinePaintProfilerView} from './TimelinePaintProfilerView.js';
2325
import type {TimelineModeViewDelegate} from './TimelinePanel.js';
@@ -76,7 +78,7 @@ export class TimelineDetailsPane extends
7678
private readonly detailsLinkifier: Components.Linkifier.Linkifier;
7779
private tabbedPane: UI.TabbedPane.TabbedPane;
7880
private readonly defaultDetailsWidget: UI.Widget.VBox;
79-
private defaultDetailsContentWidget: UI.Widget.VBox;
81+
#summaryContent = new SummaryView();
8082
private rangeDetailViews: Map<string, TimelineTreeView>;
8183
#selectedEvents?: Trace.Types.Events.Event[]|null;
8284
private lazyPaintProfilerView?: TimelinePaintProfilerView|null;
@@ -87,18 +89,17 @@ export class TimelineDetailsPane extends
8789
private lazySelectorStatsView: TimelineSelectorStatsView|null;
8890
#parsedTrace: Trace.Handlers.Types.ParsedTrace|null = null;
8991
#traceInsightsSets: Trace.Insights.Types.TraceInsightSets|null = null;
90-
9192
#eventToRelatedInsightsMap: TimelineComponents.RelatedInsightChips.EventToRelatedInsightsMap|null = null;
9293
#filmStrip: Trace.Extras.FilmStrip.Data|null = null;
9394
#networkRequestDetails: TimelineComponents.NetworkRequestDetails.NetworkRequestDetails;
9495
#layoutShiftDetails: TimelineComponents.LayoutShiftDetails.LayoutShiftDetails;
9596
#onTraceBoundsChangeBound = this.#onTraceBoundsChange.bind(this);
96-
#relatedInsightChips = new TimelineComponents.RelatedInsightChips.RelatedInsightChips();
9797
#thirdPartyTree = new ThirdPartyTreeViewWidget();
9898
#entityMapper: Utils.EntityMapper.EntityMapper|null = null;
9999

100100
constructor(delegate: TimelineModeViewDelegate) {
101101
super();
102+
this.registerRequiredCSS(detailsViewStyles);
102103
this.element.classList.add('timeline-details');
103104

104105
this.detailsLinkifier = new Components.Linkifier.Linkifier();
@@ -112,7 +113,8 @@ export class TimelineDetailsPane extends
112113
this.defaultDetailsWidget = new UI.Widget.VBox();
113114
this.defaultDetailsWidget.element.classList.add('timeline-details-view');
114115
this.defaultDetailsWidget.element.setAttribute('jslog', `${VisualLogging.pane('details').track({resize: true})}`);
115-
this.defaultDetailsContentWidget = this.#createContentWidget();
116+
this.#summaryContent.contentElement.classList.add('timeline-details-view-body');
117+
this.#summaryContent.show(this.defaultDetailsWidget.contentElement);
116118
this.appendTab(Tab.Details, i18nString(UIStrings.summary), this.defaultDetailsWidget);
117119
this.setPreferredTab(Tab.Details);
118120

@@ -237,13 +239,6 @@ export class TimelineDetailsPane extends
237239
}
238240
}
239241

240-
#createContentWidget(): UI.Widget.VBox {
241-
const defaultDetailsContentWidget = new UI.Widget.VBox();
242-
defaultDetailsContentWidget.element.classList.add('timeline-details-view-body');
243-
defaultDetailsContentWidget.show(this.defaultDetailsWidget.element);
244-
return defaultDetailsContentWidget;
245-
}
246-
247242
private selectorStatsView(): TimelineSelectorStatsView {
248243
if (this.lazySelectorStatsView) {
249244
return this.lazySelectorStatsView;
@@ -256,7 +251,7 @@ export class TimelineDetailsPane extends
256251
}
257252

258253
getDetailsContentElementForTest(): HTMLElement {
259-
return this.defaultDetailsContentWidget.element;
254+
return this.#summaryContent.contentElement;
260255
}
261256

262257
revealEventInTreeView(event: Trace.Types.Events.Event|null): void {
@@ -304,35 +299,30 @@ export class TimelineDetailsPane extends
304299
this.#selectedEvents = data.selectedEvents;
305300
this.#traceInsightsSets = data.traceInsightsSets;
306301
this.#eventToRelatedInsightsMap = data.eventToRelatedInsightsMap;
307-
if (data.eventToRelatedInsightsMap) {
308-
this.#relatedInsightChips.eventToRelatedInsightsMap = data.eventToRelatedInsightsMap;
309-
}
302+
this.#summaryContent.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
310303
this.tabbedPane.closeTabs([Tab.PaintProfiler, Tab.LayerViewer], false);
311304
for (const view of this.rangeDetailViews.values()) {
312305
view.setModelWithEvents(data.selectedEvents, data.parsedTrace, data.entityMapper);
313306
}
314307
// Set the 3p tree model.
315308
this.#thirdPartyTree.setModelWithEvents(data.selectedEvents, data.parsedTrace, data.entityMapper);
309+
this.#summaryContent.requestUpdate();
316310
this.lazyPaintProfilerView = null;
317311
this.lazyLayersView = null;
318312
await this.setSelection(null);
319313
}
320314

321-
private setSummaryContent(node: Node): void {
315+
private async setSummaryContent(node: Node): Promise<void> {
322316
const allTabs = this.tabbedPane.otherTabs(Tab.Details);
323317
for (let i = 0; i < allTabs.length; ++i) {
324318
if (!this.rangeDetailViews.has(allTabs[i])) {
325319
this.tabbedPane.closeTab(allTabs[i]);
326320
}
327321
}
328322

329-
// Append relatedChips inside of the node being shown.
330-
const chipParent = (node instanceof Element && node.shadowRoot || node);
331-
chipParent.appendChild(this.#relatedInsightChips);
332-
333-
this.defaultDetailsContentWidget.detach();
334-
this.defaultDetailsContentWidget = this.#createContentWidget();
335-
this.defaultDetailsContentWidget.contentElement.append(node);
323+
this.#summaryContent.node = node;
324+
this.#summaryContent.requestUpdate();
325+
await this.#summaryContent.updateComplete;
336326
}
337327

338328
private updateContents(): void {
@@ -374,7 +364,7 @@ export class TimelineDetailsPane extends
374364
*/
375365
private scheduleUpdateContentsFromWindow(forceImmediateUpdate = false): void {
376366
if (!this.#parsedTrace) {
377-
this.setSummaryContent(UI.Fragment.html`<div/>`);
367+
void this.setSummaryContent(UI.Fragment.html`<div/>`);
378368
return;
379369
}
380370
if (forceImmediateUpdate) {
@@ -423,7 +413,7 @@ export class TimelineDetailsPane extends
423413

424414
#setSelectionForTimelineFrame(frame: Trace.Types.Events.LegacyTimelineFrame): void {
425415
const matchedFilmStripFrame = this.#getFilmStripFrame(frame);
426-
this.setSummaryContent(
416+
void this.setSummaryContent(
427417
TimelineUIUtils.generateDetailsContentForFrame(frame, this.#filmStrip, matchedFilmStripFrame));
428418
const target = SDK.TargetManager.TargetManager.instance().rootTarget();
429419
if (frame.layerTree && target) {
@@ -442,31 +432,27 @@ export class TimelineDetailsPane extends
442432
}
443433
const maybeTarget = targetForEvent(this.#parsedTrace, networkRequest);
444434
await this.#networkRequestDetails.setData(this.#parsedTrace, networkRequest, maybeTarget, this.#entityMapper);
445-
this.#relatedInsightChips.activeEvent = networkRequest;
446-
if (this.#eventToRelatedInsightsMap) {
447-
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
448-
}
449435

450-
this.setSummaryContent(this.#networkRequestDetails);
436+
this.#summaryContent.selectedEvent = networkRequest;
437+
this.#summaryContent.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
438+
await this.setSummaryContent(this.#networkRequestDetails);
451439
}
452440

453441
async #setSelectionForTraceEvent(event: Trace.Types.Events.Event): Promise<void> {
454442
if (!this.#parsedTrace) {
455443
return;
456444
}
457445

458-
this.#relatedInsightChips.activeEvent = event;
459-
if (this.#eventToRelatedInsightsMap) {
460-
this.#relatedInsightChips.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
461-
}
446+
this.#summaryContent.selectedEvent = event;
447+
this.#summaryContent.eventToRelatedInsightsMap = this.#eventToRelatedInsightsMap;
448+
this.#summaryContent.requestUpdate();
462449

463450
// Special case: if the user selects a layout shift or a layout shift cluster,
464451
// render the new layout shift details component.
465452
if (Trace.Types.Events.isSyntheticLayoutShift(event) || Trace.Types.Events.isSyntheticLayoutShiftCluster(event)) {
466453
const isFreshRecording = Boolean(this.#parsedTrace && Tracker.instance().recordingIsFresh(this.#parsedTrace));
467454
this.#layoutShiftDetails.setData(event, this.#traceInsightsSets, this.#parsedTrace, isFreshRecording);
468-
this.setSummaryContent(this.#layoutShiftDetails);
469-
return;
455+
return await this.setSummaryContent(this.#layoutShiftDetails);
470456
}
471457

472458
// Otherwise, build the generic trace event details UI.
@@ -482,8 +468,8 @@ export class TimelineDetailsPane extends
482468
}
483469
this.detailsLinkifier.reset();
484470
this.selection = selection;
485-
this.#relatedInsightChips.activeEvent = null;
486471
if (!this.selection) {
472+
this.#summaryContent.selectedEvent = null;
487473
// Update instantly using forceImmediateUpdate, since we are only
488474
// making a single call and don't need to debounce.
489475
this.scheduleUpdateContentsFromWindow(/* forceImmediateUpdate */ true);
@@ -563,7 +549,7 @@ export class TimelineDetailsPane extends
563549
}
564550

565551
private appendDetailsTabsForTraceEventAndShowDetails(event: Trace.Types.Events.Event, content: Node): void {
566-
this.setSummaryContent(content);
552+
void this.setSummaryContent(content);
567553
if (Trace.Types.Events.isPaint(event) || Trace.Types.Events.isRasterTask(event)) {
568554
this.showEventInPaintProfiler(event);
569555
}
@@ -605,9 +591,16 @@ export class TimelineDetailsPane extends
605591
const summaryDetailElem = TimelineUIUtils.generateSummaryDetails(
606592
aggregatedStats, startOffset, endOffset, this.#selectedEvents, this.#thirdPartyTree);
607593

608-
this.#thirdPartyTree.updateContents(this.selection || selectionFromRangeMilliSeconds(startTime, endTime));
609-
610-
this.setSummaryContent(summaryDetailElem);
594+
// This is a bit of a hack as we are midway through migrating this to
595+
// the new UI Eng vision.
596+
// The 3P tree view will only bother to update its DOM if it has a
597+
// parentElement, so we trigger the rendering of the summary content
598+
// (so the 3P Tree View is attached to the DOM) and then we tell it to
599+
// update.
600+
// This will be fixed once we migrate this component fully to the new vision (b/407751379)
601+
void this.setSummaryContent(summaryDetailElem).then(() => {
602+
this.#thirdPartyTree.updateContents(this.selection || selectionFromRangeMilliSeconds(startTime, endTime));
603+
});
611604

612605
// Find all recalculate style events data from range
613606
const isSelectorStatsEnabled =
@@ -636,3 +629,56 @@ export enum Tab {
636629
SelectorStats = 'selector-stats',
637630
/* eslint-enable @typescript-eslint/naming-convention */
638631
}
632+
633+
/**
634+
* This code renders the contents of the summary view.
635+
* To assist with the migration to the UI Eng vision, its primary job is to
636+
* render a given Node (which is usually a DocumentFragmetn). These are what get
637+
* build via TimelineUIUtils depending on what sort of event or range is
638+
* selected. In time once this is migrated we can remove the "render this node"
639+
* functionality.
640+
*/
641+
642+
interface SummaryViewInput {
643+
// A helper as we migrate to the new eng vision: this is arbitrary DOM that we want to render within the summary.
644+
node: Node|null;
645+
selectedEvent: Trace.Types.Events.Event|null;
646+
eventToRelatedInsightsMap: TimelineComponents.RelatedInsightChips.EventToRelatedInsightsMap|null;
647+
}
648+
649+
type View = (input: SummaryViewInput, output: object, target: HTMLElement) => void;
650+
const SUMMARY_DEFAULT_VIEW: View = (input, _output, target) => {
651+
render(
652+
html`
653+
<style>${detailsViewStyles}</style>
654+
${input.node ?? nothing}
655+
<devtools-widget .widgetConfig=${
656+
UI.Widget.widgetConfig(TimelineComponents.RelatedInsightChips.RelatedInsightChips, {
657+
activeEvent: input.selectedEvent,
658+
eventToInsightsMap: input.eventToRelatedInsightsMap,
659+
})}></devtools-widget>
660+
`,
661+
target, {host: input});
662+
};
663+
664+
class SummaryView extends UI.Widget.VBox {
665+
#view: View;
666+
node: Node|null = null;
667+
selectedEvent: Trace.Types.Events.Event|null = null;
668+
eventToRelatedInsightsMap: TimelineComponents.RelatedInsightChips.EventToRelatedInsightsMap|null = null;
669+
670+
constructor(element?: HTMLElement, view = SUMMARY_DEFAULT_VIEW) {
671+
super(false, false, element);
672+
this.#view = view;
673+
}
674+
675+
override performUpdate(): void {
676+
this.#view(
677+
{
678+
node: this.node,
679+
selectedEvent: this.selectedEvent,
680+
eventToRelatedInsightsMap: this.eventToRelatedInsightsMap,
681+
},
682+
{}, this.contentElement);
683+
}
684+
}

0 commit comments

Comments
 (0)