Skip to content

Commit 77d35bb

Browse files
jackfranklinDevtools-frontend LUCI CQ
authored andcommitted
RPP: fix incorrect annotations adorner count
We weren't using the de-duplicated set of annotations when counting to update the badge. The component already does the de-duplicating, so we can use its list of annotations to get the correct number to display. Fixed: 393291324 Change-Id: Ie2d478de8424ca293909844359892302e011bbf6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6218245 Auto-Submit: Jack Franklin <[email protected]> Commit-Queue: Andres Olivares <[email protected]> Reviewed-by: Andres Olivares <[email protected]>
1 parent dfca663 commit 77d35bb

File tree

3 files changed

+71
-6
lines changed

3 files changed

+71
-6
lines changed

front_end/panels/timeline/components/Sidebar.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import type * as Trace from '../../../models/trace/trace.js';
5+
import * as Trace from '../../../models/trace/trace.js';
66
import {raf, renderElementIntoDOM} from '../../../testing/DOMHelpers.js';
77
import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
88
import {TraceLoader} from '../../../testing/TraceLoader.js';
@@ -66,4 +66,57 @@ describeWithEnvironment('Sidebar', () => {
6666
tabs.filter(tab => tab.classList.contains('selected')).map(elem => elem.getAttribute('aria-label'));
6767
assert.deepEqual(selectedTabLabels, ['Annotations']);
6868
});
69+
70+
it('shows the count for the active annotations', async function() {
71+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
72+
const events = parsedTrace.Renderer.allTraceEntries;
73+
const annotation1: Trace.Types.File.Annotation = {
74+
type: 'ENTRY_LABEL',
75+
entry: events[0],
76+
label: 'Entry Label 1',
77+
};
78+
79+
const annotation2: Trace.Types.File.Annotation = {
80+
type: 'ENTRY_LABEL',
81+
entry: events[1],
82+
label: 'Entry Label 2',
83+
};
84+
85+
const sidebar = await renderSidebar(parsedTrace, metadata, null);
86+
sidebar.setAnnotations([annotation1, annotation2], new Map());
87+
const tabbedPane = sidebar.element.querySelector('.tabbed-pane')?.shadowRoot;
88+
assert.isOk(tabbedPane);
89+
const annotationsTab = tabbedPane.querySelector('#tab-annotations');
90+
assert.isOk(annotationsTab);
91+
const countBadge = annotationsTab.querySelector<HTMLElement>('.badge');
92+
assert.strictEqual(countBadge?.innerText, '2');
93+
});
94+
95+
it('de-duplicates annotations that are pending to not show an incorrect count', async function() {
96+
const {parsedTrace, metadata} = await TraceLoader.traceEngine(this, 'web-dev-with-commit.json.gz');
97+
const events = parsedTrace.Renderer.allTraceEntries;
98+
99+
// Create Empty Entry Label Annotation (considered not started)
100+
const entryLabelAnnotation: Trace.Types.File.Annotation = {
101+
type: 'ENTRY_LABEL',
102+
entry: events[0],
103+
label: '',
104+
};
105+
106+
// Create Entries link that only has 'to' entry (considered not started)
107+
const entriesLink: Trace.Types.File.Annotation = {
108+
type: 'ENTRIES_LINK',
109+
entryFrom: events[0],
110+
state: Trace.Types.File.EntriesLinkState.CREATION_NOT_STARTED,
111+
};
112+
113+
const sidebar = await renderSidebar(parsedTrace, metadata, null);
114+
sidebar.setAnnotations([entryLabelAnnotation, entriesLink], new Map());
115+
const tabbedPane = sidebar.element.querySelector('.tabbed-pane')?.shadowRoot;
116+
assert.isOk(tabbedPane);
117+
const annotationsTab = tabbedPane.querySelector('#tab-annotations');
118+
assert.isOk(annotationsTab);
119+
const countBadge = annotationsTab.querySelector<HTMLElement>('.badge');
120+
assert.strictEqual(countBadge?.innerText, '1');
121+
});
69122
});

front_end/panels/timeline/components/Sidebar.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ export class SidebarWidget extends UI.Widget.VBox {
5151
#insightsView = new InsightsView();
5252
#annotationsView = new AnnotationsView();
5353

54-
#annotationCount = 0;
55-
5654
/**
5755
* Track if the user has opened the sidebar before. We do this so that the
5856
* very first time they record/import a trace after the sidebar ships, we can
@@ -102,13 +100,13 @@ export class SidebarWidget extends UI.Widget.VBox {
102100
updatedAnnotations: Trace.Types.File.Annotation[],
103101
annotationEntryToColorMap: Map<Trace.Types.Events.Event, string>): void {
104102
this.#annotationsView.setAnnotations(updatedAnnotations, annotationEntryToColorMap);
105-
this.#annotationCount = updatedAnnotations.length;
106103
this.#updateAnnotationsCountBadge();
107104
}
108105

109106
#updateAnnotationsCountBadge(): void {
110-
if (this.#annotationCount) {
111-
this.#tabbedPane.setBadge('annotations', this.#annotationCount.toString());
107+
const annotations = this.#annotationsView.deduplicatedAnnotations();
108+
if (annotations.length) {
109+
this.#tabbedPane.setBadge('annotations', annotations.length.toString());
112110
}
113111
}
114112

@@ -174,4 +172,14 @@ class AnnotationsView extends UI.Widget.VBox {
174172
this.#component.annotationEntryToColorMap = annotationEntryToColorMap;
175173
this.#component.annotations = annotations;
176174
}
175+
176+
/**
177+
* The component "de-duplicates" annotations to ensure implementation details
178+
* about how we create pending annotations don't leak into the UI. We expose
179+
* these here because we use this count to show the number of annotations in
180+
* the small adorner in the sidebar tab.
181+
*/
182+
deduplicatedAnnotations(): readonly Trace.Types.File.Annotation[] {
183+
return this.#component.deduplicatedAnnotations();
184+
}
177185
}

front_end/panels/timeline/components/SidebarAnnotationsTab.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ export class SidebarAnnotationsTab extends HTMLElement {
110110
this.#annotationsHiddenSetting = Common.Settings.Settings.instance().moduleSetting('annotations-hidden');
111111
}
112112

113+
deduplicatedAnnotations(): readonly Trace.Types.File.Annotation[] {
114+
return this.#annotations;
115+
}
116+
113117
set annotations(annotations: Trace.Types.File.Annotation[]) {
114118
this.#annotations = this.#processAnnotationsList(annotations);
115119
void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);

0 commit comments

Comments
 (0)