Skip to content

Commit 0456e4b

Browse files
AlinaVarkkiDevtools-frontend LUCI CQ
authored andcommitted
[AI][RPP] Only show the FRE dialog when the generate label setting is off and show the dialog otherwise
Make the fre dialog appear only when the setting is off. The setting can be turned on from the dialog of the AI settings panel. Screencast: http://screencast/cast/NDk3NDIxNzM3NTI1MjQ4MHwyZGUzOGFjNC1lMg Also refactored tests for labels to remove a bunch of duplicate code. Bug: 393063467 Change-Id: I7f7aaf854d4e164335a6cebd73bc8ffdad4b336e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6348860 Commit-Queue: Alina Varkki <[email protected]> Auto-Submit: Alina Varkki <[email protected]> Reviewed-by: Jack Franklin <[email protected]>
1 parent b59c0ce commit 0456e4b

File tree

2 files changed

+113
-169
lines changed

2 files changed

+113
-169
lines changed

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

Lines changed: 87 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +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 * as Common from '../../../core/common/common.js';
56
import * as Trace from '../../../models/trace/trace.js';
67
import {dispatchClickEvent} from '../../../testing/DOMHelpers.js';
78
import {describeWithEnvironment, updateHostConfig} from '../../../testing/EnvironmentHelpers.js';
@@ -315,6 +316,46 @@ describeWithEnvironment('Overlays', () => {
315316
return {overlays, container, charts};
316317
}
317318

319+
async function createAnnotationsLabelElement(
320+
context: Mocha.Suite|Mocha.Context|null, file: string, entryIndex: number, label?: string): Promise<{
321+
elementsWrapper: HTMLElement,
322+
inputField: HTMLElement,
323+
overlays: Overlays.Overlays.Overlays,
324+
event: Trace.Types.Events.Event,
325+
}> {
326+
updateHostConfig({
327+
devToolsAiGeneratedTimelineLabels: {
328+
enabled: true,
329+
}
330+
});
331+
332+
const {parsedTrace} = await TraceLoader.traceEngine(context, file);
333+
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
334+
const event = charts.mainProvider.eventByIndex?.(entryIndex);
335+
assert.isOk(event);
336+
337+
// Create an entry label overlay
338+
Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
339+
type: 'ENTRY_LABEL',
340+
entry: event,
341+
label: label ?? '',
342+
});
343+
await overlays.update();
344+
345+
// Ensure that the overlay was created.
346+
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
347+
assert.isOk(overlayDOM);
348+
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
349+
assert.isOk(component?.shadowRoot);
350+
component.connectedCallback();
351+
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
352+
assert.isOk(elementsWrapper);
353+
const inputField = elementsWrapper.querySelector<HTMLElement>('.input-field');
354+
assert.isOk(inputField);
355+
356+
return {elementsWrapper, inputField, overlays, event};
357+
}
358+
318359
it('can render an entry selected overlay', async function() {
319360
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
320361
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
@@ -435,53 +476,47 @@ describeWithEnvironment('Overlays', () => {
435476
});
436477
});
437478

438-
// TODO: update to check if the fre is completed and make the dialog visible dependant on that
439-
it('should show FRE dialog on the ai suggestion button click', async function() {
440-
updateHostConfig({
441-
devToolsAiGeneratedTimelineLabels: {
442-
enabled: true,
443-
}
444-
});
479+
it('should show FRE dialog on the ai suggestion button click if the `ai-annotations-enabled` setting is off',
480+
async function() {
481+
Common.Settings.moduleSetting('ai-annotations-enabled').set(false);
482+
const {elementsWrapper, inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
445483

446-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
447-
const {overlays, container} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
448-
const charts = createCharts(parsedTrace);
449-
const event = charts.mainProvider.eventByIndex?.(50);
450-
assert.isOk(event);
484+
// Double click on the label box to make it editable and focus on it
485+
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
451486

452-
// Since ENTRY_LABEL is AnnotationOverlay, create it through ModificationsManager
453-
Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
454-
type: 'ENTRY_LABEL',
455-
label: '',
456-
entry: event,
457-
});
487+
const aiLabelButtonWrapper =
488+
elementsWrapper.querySelector<HTMLElement>('.ai-label-button-wrapper') as HTMLSpanElement;
489+
assert.isOk(aiLabelButtonWrapper);
490+
const aiButton = aiLabelButtonWrapper.querySelector<HTMLElement>('.ai-label-button') as HTMLSpanElement;
491+
assert.isOk(aiButton);
458492

459-
await overlays.update();
460-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
461-
assert.isOk(overlayDOM);
462-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
463-
assert.isOk(component?.shadowRoot);
464-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
465-
assert.isOk(elementsWrapper);
466-
const inputField = elementsWrapper.querySelector<HTMLElement>('.input-field');
467-
assert.isOk(inputField);
493+
// This dialog should not be visible unless the `generate annotation` button is clicked
494+
assert.isFalse(showFreDialogStub.called, 'Expected FreDialog to be not shown but it\'s shown');
495+
aiButton.dispatchEvent(new FocusEvent('click', {bubbles: true}));
468496

469-
// Double click on the label box to make it editable and focus on it
470-
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
497+
// This dialog should be visible
498+
assert.isTrue(showFreDialogStub.called, 'Expected FreDialog to be shown but it\'s not shown');
499+
});
471500

472-
const aiLabelButtonWrapper =
473-
elementsWrapper.querySelector<HTMLElement>('.ai-label-button-wrapper') as HTMLSpanElement;
474-
assert.isOk(aiLabelButtonWrapper);
475-
const aiButton = aiLabelButtonWrapper.querySelector<HTMLElement>('.ai-label-button') as HTMLSpanElement;
476-
assert.isOk(aiButton);
501+
it('should not show FRE dialog on the ai suggestion button click if the `ai-annotations-enabled` setting is on',
502+
async function() {
503+
Common.Settings.moduleSetting('ai-annotations-enabled').set(true);
504+
const {elementsWrapper, inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
477505

478-
// This dialog should not be visible unless the `generate annotation` button is clicked
479-
assert.isFalse(showFreDialogStub.called, 'Expected FreDialog to be not shown but it\'s shown');
480-
aiButton.dispatchEvent(new FocusEvent('click', {bubbles: true}));
506+
// Double click on the label box to make it editable and focus on it
507+
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
481508

482-
// This dialog should be visible
483-
assert.isTrue(showFreDialogStub.called, 'Expected FreDialog to be shown but it\'s not shown');
484-
});
509+
const aiLabelButtonWrapper =
510+
elementsWrapper.querySelector<HTMLElement>('.ai-label-button-wrapper') as HTMLSpanElement;
511+
512+
assert.isOk(aiLabelButtonWrapper);
513+
const aiButton = aiLabelButtonWrapper.querySelector<HTMLElement>('.ai-label-button') as HTMLSpanElement;
514+
assert.isOk(aiButton);
515+
516+
aiButton.dispatchEvent(new FocusEvent('click', {bubbles: true}));
517+
// This dialog should not be visible on the `generate label` button click since the setting is already on
518+
assert.isFalse(showFreDialogStub.called, 'Expected FreDialog to be shown but it\'s not shown');
519+
});
485520

486521
it('toggles overlays container display', async function() {
487522
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
@@ -518,59 +553,13 @@ describeWithEnvironment('Overlays', () => {
518553
assert.lengthOf(container.children, 1);
519554
});
520555

521-
it('can render the label for entry label overlay', async function() {
522-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
523-
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
524-
const event = charts.mainProvider.eventByIndex?.(50);
525-
assert.isOk(event);
526-
527-
overlays.add({
528-
type: 'ENTRY_LABEL',
529-
entry: event,
530-
label: 'entry label',
531-
});
532-
await overlays.update();
533-
534-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
535-
assert.isOk(overlayDOM);
536-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
537-
assert.isOk(component?.shadowRoot);
538-
539-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
540-
assert.isOk(elementsWrapper);
541-
542-
const inputField = elementsWrapper.querySelector<HTMLElement>('.input-field');
543-
assert.isOk(inputField);
544-
556+
it('can render provided label for entry label overlay', async function() {
557+
const {inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'entry label');
545558
assert.strictEqual(inputField?.innerText, 'entry label');
546559
});
547560

548561
it('Inputting `Enter`into label overlay makes it non-editable', async function() {
549-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
550-
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
551-
const event = charts.mainProvider.eventByIndex?.(50);
552-
assert.isOk(event);
553-
554-
// Create an entry label overlay
555-
overlays.add({
556-
type: 'ENTRY_LABEL',
557-
entry: event,
558-
label: 'label',
559-
});
560-
await overlays.update();
561-
562-
// Ensure that the overlay was created.
563-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
564-
assert.isOk(overlayDOM);
565-
566-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
567-
assert.isOk(component?.shadowRoot);
568-
component.connectedCallback();
569-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
570-
assert.isOk(elementsWrapper);
571-
572-
const inputField = elementsWrapper.querySelector<HTMLElement>('.input-field');
573-
assert.isOk(inputField);
562+
const {inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'label');
574563

575564
// Double click on the label box to make it editable and focus on it
576565
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
@@ -687,31 +676,7 @@ describeWithEnvironment('Overlays', () => {
687676
});
688677

689678
it('Removes empty label if it is empty when navigated away from (removed focused from)', async function() {
690-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
691-
const {overlays, container, charts} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
692-
const event = charts.mainProvider.eventByIndex?.(50);
693-
assert.isOk(event);
694-
695-
// Create an entry label overlay
696-
Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
697-
type: 'ENTRY_LABEL',
698-
entry: event,
699-
label: '',
700-
});
701-
await overlays.update();
702-
703-
// Ensure that the overlay was created.
704-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
705-
assert.isOk(overlayDOM);
706-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
707-
assert.isOk(component?.shadowRoot);
708-
709-
component.connectedCallback();
710-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
711-
assert.isOk(elementsWrapper);
712-
713-
const inputField = elementsWrapper.querySelector<HTMLElement>('.input-field');
714-
assert.isOk(inputField);
679+
const {inputField, overlays, event} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
715680

716681
// Double click on the label box to make it editable and focus on it
717682
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
@@ -927,70 +892,25 @@ describeWithEnvironment('Overlays', () => {
927892
assert.lengthOf(container.children, 0);
928893
});
929894

930-
it('the label entry field is editable when created', async function() {
931-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
932-
const {overlays, container} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
933-
const charts = createCharts(parsedTrace);
934-
const event = charts.mainProvider.eventByIndex?.(50);
935-
assert.isOk(event);
936-
937-
// Since ENTRY_LABEL is AnnotationOverlay, create it through ModificationsManager
938-
Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
939-
type: 'ENTRY_LABEL',
940-
label: '',
941-
entry: event,
942-
});
943-
944-
await overlays.update();
945-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
946-
assert.isOk(overlayDOM);
947-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
948-
assert.isOk(component?.shadowRoot);
949-
950-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
951-
const inputField = elementsWrapper?.querySelector<HTMLElement>('.input-field') as HTMLSpanElement;
952-
assert.isOk(inputField);
895+
it('the label entry field is editable when created without initial label', async function() {
896+
const {inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
953897
// The label input box should be editable after it is created and before anything else happened
954898
assert.isTrue(inputField.isContentEditable);
955899
});
956900

957901
it('the label entry field is in focus after being double clicked on', async function() {
958-
const {parsedTrace} = await TraceLoader.traceEngine(this, 'web-dev.json.gz');
959-
const {overlays, container} = setupChartWithDimensionsAndAnnotationOverlayListeners(parsedTrace);
960-
const charts = createCharts(parsedTrace);
961-
const event = charts.mainProvider.eventByIndex?.(50);
962-
assert.isOk(event);
963-
964-
// Since ENTRY_LABEL is AnnotationOverlay, create it through ModificationsManager
965-
Timeline.ModificationsManager.ModificationsManager.activeManager()?.createAnnotation({
966-
type: 'ENTRY_LABEL',
967-
label: '',
968-
entry: event,
969-
});
970-
971-
await overlays.update();
972-
const overlayDOM = container.querySelector<HTMLElement>('.overlay-type-ENTRY_LABEL');
973-
assert.isOk(overlayDOM);
974-
const component = overlayDOM?.querySelector('devtools-entry-label-overlay');
975-
assert.isOk(component?.shadowRoot);
976-
977-
const elementsWrapper = component.shadowRoot.querySelector<HTMLElement>('.label-parts-wrapper');
978-
assert.isOk(elementsWrapper);
979-
const labelBox = elementsWrapper.querySelector<HTMLElement>('.input-field') as HTMLSpanElement;
980-
981-
assert.isOk(labelBox);
982-
902+
const {inputField} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
983903
// The label input box should be editable after it is created and before anything else happened
984-
assert.isTrue(labelBox.isContentEditable);
904+
assert.isTrue(inputField.isContentEditable);
985905

986906
// Make the content to editable by changing the element blur like when clicking outside of it.
987907
// When that happens, the content should be set to not editable.
988-
labelBox.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
989-
assert.isFalse(labelBox.isContentEditable);
908+
inputField.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
909+
assert.isFalse(inputField.isContentEditable);
990910

991911
// Double click on the label to make it editable again
992-
labelBox.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
993-
assert.isTrue(labelBox.isContentEditable);
912+
inputField.dispatchEvent(new FocusEvent('dblclick', {bubbles: true}));
913+
assert.isTrue(inputField.isContentEditable);
994914
});
995915
});
996916

front_end/panels/timeline/overlays/components/EntryLabelOverlay.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import '../../../../ui/components/icon_button/icon_button.js';
66

7+
import * as Common from '../../../../core/common/common.js';
78
import * as i18n from '../../../../core/i18n/i18n.js';
89
import * as Platform from '../../../../core/platform/platform.js';
910
import * as Root from '../../../../core/root/root.js';
@@ -379,8 +380,28 @@ export class EntryLabelOverlay extends HTMLElement {
379380
}
380381
}
381382

383+
// Generate the AI label suggestion if:
384+
// 1. the user has already already seen the fre dialog and confirmed the feature usage
385+
// or
386+
// 2. turned on the `generate AI labels` setting through the AI settings panel
387+
//
388+
// Otherwise, show the fre dialog with a 'Got it' button that turns the setting on.
382389
async #handleAiButtonClick(): Promise<void> {
383-
await PanelCommon.FreDialog.show({
390+
// Creates or gets the setting if it exists.
391+
const onboardingCompleteSetting =
392+
Common.Settings.Settings.instance().createSetting('ai-annotations-enabled', false);
393+
394+
if (onboardingCompleteSetting.get()) {
395+
// TODO: Actually generate the ai label
396+
if (this.#inputField) {
397+
this.#label = 'ai generated label';
398+
this.dispatchEvent(new EntryLabelChangeEvent(this.#label));
399+
this.#inputField.innerText = this.#label;
400+
}
401+
return;
402+
}
403+
404+
const userConsented = await PanelCommon.FreDialog.show({
384405
header: {iconName: 'pen-spark', text: lockedString(UIStringsNotTranslate.freDisclaimerHeader)},
385406
reminderItems: [
386407
{
@@ -411,12 +432,15 @@ export class EntryLabelOverlay extends HTMLElement {
411432
// TODO: Update this href to be the correct link.
412433
learnMoreHref: Platform.DevToolsPath.EmptyUrlString
413434
});
435+
436+
if (userConsented) {
437+
onboardingCompleteSetting.set(true);
438+
}
414439
}
415440

416441
#renderAiButton(): Lit.TemplateResult {
417442
// clang-format off
418443
return html`
419-
<!-- TODO: On button click generate a label -->
420444
<!-- 'preventDefault' on the AI label button to prevent the label removal on blur -->
421445
<span
422446
class="ai-label-button-wrapper"

0 commit comments

Comments
 (0)