RPP: further improve keyboard + annotations usage

1. Make it so you can hit enter on a selected event to create an
   annotation.
2. You can tab from the annotation's input text to the generate AI
   button and interact with it, without the input disappearing.

Fixed: 406539030
Change-Id: I20ffc67aa2493757ce1c16611ba5cefef537707c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6400621
Auto-Submit: Jack Franklin <[email protected]>
Commit-Queue: Alina Varkki <[email protected]>
Reviewed-by: Alina Varkki <[email protected]>

Author: jackfranklin

front_end/panels/timeline/TimelineFlameChartView.ts

@@ -403,7 +403,7 @@ export class TimelineFlameChartView extends Common.ObjectWrapper.eventMixin<Even
      * NOTE: ENTRY_SELECTED, ENTRY_INVOKED and ENTRY_HOVERED are not always super obvious:
      * ENTRY_SELECTED: is KEYBOARD ONLY selection of events (e.g. navigating through the flamechart with your arrow keys)
      * ENTRY_HOVERED: is MOUSE ONLY when an event is hovered over with the mouse.
-     * ENTRY_INVOKED: is when the user cilcks on an event, or hits the "enter" key whilst an event is selected.
+     * ENTRY_INVOKED: is when the user clicks on an event, or hits the "enter" key whilst an event is selected.
      */
     this.onMainEntrySelected = this.onEntrySelected.bind(this, this.mainDataProvider);
     this.onNetworkEntrySelected = this.onEntrySelected.bind(this, this.networkDataProvider);

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

@@ -6,7 +6,7 @@ import * as Common from '../../../core/common/common.js';
 import * as AiAssistanceModels from '../../../models/ai_assistance/ai_assistance.js';
 import * as Trace from '../../../models/trace/trace.js';
 import {mockAidaClient} from '../../../testing/AiAssistanceHelpers.js';
-import {cleanTextContent, dispatchClickEvent} from '../../../testing/DOMHelpers.js';
+import {cleanTextContent, dispatchClickEvent, doubleRaf} from '../../../testing/DOMHelpers.js';
 import {describeWithEnvironment, updateHostConfig} from '../../../testing/EnvironmentHelpers.js';
 import {
   makeInstantEvent,
@@ -668,28 +668,27 @@ describeWithEnvironment('Overlays', () => {
       );
     });
 
-    it('Does not show `generate ai label` button if the label is not empty',
-       async function() {
-         updateHostConfig({
-           aidaAvailability: {
-             enabled: false,
-             blockedByAge: true,
-             blockedByEnterprisePolicy: false,
-             blockedByGeo: false,
-             disallowLogging: true,
-             enterprisePolicyValue: 1,
-           },
-         });
+    it('Does not show `generate ai label` button if the label is not empty', async function() {
+      updateHostConfig({
+        aidaAvailability: {
+          enabled: false,
+          blockedByAge: true,
+          blockedByEnterprisePolicy: false,
+          blockedByGeo: false,
+          disallowLogging: true,
+          enterprisePolicyValue: 1,
+        },
+      });
 
-         const {elementsWrapper, inputField} =
-             await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'entry label');
-         assert.strictEqual(inputField?.innerText, 'entry label');
+      const {elementsWrapper, inputField} =
+          await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50, 'entry label');
+      assert.strictEqual(inputField?.innerText, 'entry label');
 
-         const aiLabelButtonWrapper =
-             elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;
-         // Button should not exist
-         assert.isNotOk(aiLabelButtonWrapper);
-       });
+      const aiLabelButtonWrapper =
+          elementsWrapper.querySelector<HTMLElement>('.ai-label-disabled-button-wrapper') as HTMLSpanElement;
+      // Button should not exist
+      assert.isNotOk(aiLabelButtonWrapper);
+    });
 
     it('Shows the `generate ai label` button if the label is empty', async function() {
       updateHostConfig({
@@ -905,7 +904,7 @@ describeWithEnvironment('Overlays', () => {
       assert.lengthOf(overlays.overlaysOfType('TIME_RANGE'), 2);
     });
 
-    it('Removes empty label if it is empty when navigated away from (removed focused from)', async function() {
+    it('removes empty label if it is empty when it loses focus', async function() {
       const {inputField, overlays, event} = await createAnnotationsLabelElement(this, 'web-dev.json.gz', 50);
 
       // Double click on the label box to make it editable and focus on it
@@ -916,7 +915,8 @@ describeWithEnvironment('Overlays', () => {
 
       // Change the content to not editable by changing the element blur like when clicking outside of it.
       // The label is empty since no initial value was passed into it and no characters were entered.
-      inputField.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
+      inputField.dispatchEvent(new FocusEvent('focusout', {bubbles: true}));
+      await doubleRaf();
 
       // Ensure that the entry overlay has been removed because it was saved empty
       assert.lengthOf(overlays.overlaysForEntry(event), 0);
@@ -1135,7 +1135,8 @@ describeWithEnvironment('Overlays', () => {
 
       // Make the content to editable by changing the element blur like when clicking outside of it.
       // When that happens, the content should be set to not editable.
-      inputField.dispatchEvent(new FocusEvent('blur', {bubbles: true}));
+      inputField.dispatchEvent(new FocusEvent('focusout', {bubbles: true}));
+      await doubleRaf();
       assert.isFalse(inputField.isContentEditable);
 
       // Double click on the label to make it editable again

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

@@ -496,14 +496,18 @@ export class EntryLabelOverlay extends HTMLElement {
   // Otherwise, show the fre dialog with a 'Got it' button that turns the setting on.
   async #handleAiButtonClick(): Promise<void> {
     if (this.#aiAnnotationsEnabledSetting.get()) {
-      // TODO(b/405354543): handle the loading state here.
       if (!this.#callTree || !this.#inputField) {
         // Shouldn't happen as we only show the Generate UI when we have this, but this satisfies TS.
         return;
       }
       try {
         // Trigger a re-render to display the loading component in the place of the button when the label is being generated.
         this.#isAILabelLoading = true;
+        // Trigger a re-render to put focus back on the input box, otherwise
+        // when the button changes to a loading spinner, it loses focus and the
+        // editing state is reset because the component loses focus.
+        this.#render();
+        this.#focusInputBox();
         void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);
 
         this.#label = await this.#performanceAgent.generateAIEntryLabel(this.#callTree);
@@ -512,7 +516,7 @@ export class EntryLabelOverlay extends HTMLElement {
 
         this.#isAILabelLoading = false;
         // Trigger a re-render to hide the AI Button and display the generated label.
-        void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);
+        this.#render();
       } catch {
         // TODO(b/405354265): handle the error state
       }
@@ -723,11 +727,41 @@ export class EntryLabelOverlay extends HTMLElement {
     // clang-format on
   }
 
+  #handleFocusOutEvent(): void {
+    /**
+     * Usually when the text box loses focus, we want to stop the edit mode and
+     * just display the annotation. However, if the user tabs from the text box
+     * to focus the GenerateAI button, we need to ensure that we do not exit
+     * edit mode. The only reliable method is to listen to the focusout event
+     * (which bubbles, unlike `blur`) on the parent.
+     * This means we get any updates on the focus state of anything inside this component.
+     * Once we get the event, we check to see if focus is still within this
+     * component (which means either the input, or the button, or the disclaimer popup).
+     * If it is, we do nothing, but if we have lost focus,  we can then exit editable mode.
+     *
+     * If you are thinking "why not `blur` on the span" it's because blur does
+     * not propagate; the span itself never blurs, but the elements inside it
+     * do as the span is not focusable.
+     *
+     * The reason we do it inside a rAF is because on the first run the values
+     * for `this.hasFocus()` are not accurate. I'm not quite sure why, but by
+     * letting the browser have a frame to update, it then accurately reports
+     * the up to date values for `this.hasFocus()`
+     */
+    requestAnimationFrame(() => {
+      if (!this.hasFocus()) {
+        this.setLabelEditabilityAndRemoveEmptyLabel(false);
+      }
+    });
+  }
+
   #render(): void {
     // clang-format off
     Lit.render(
         html`
-        <span class="label-parts-wrapper" role="region" aria-label=${i18nString(UIStrings.entryLabel)}>
+        <span class="label-parts-wrapper" role="region" aria-label=${i18nString(UIStrings.entryLabel)}
+        @focusout=${this.#handleFocusOutEvent}
+        >
           <span
             class="label-button-input-wrapper">
             <span
@@ -739,7 +773,6 @@ export class EntryLabelOverlay extends HTMLElement {
               @dblclick=${() => {
                 this.setLabelEditabilityAndRemoveEmptyLabel(true);
               }}
-              @blur=${() => this.setLabelEditabilityAndRemoveEmptyLabel(false)}
               @keydown=${this.#handleLabelInputKeyDown}
               @paste=${this.#handleLabelInputPaste}
               @keyup=${() => {

front_end/panels/timeline/overlays/components/entryLabelOverlay.css

@@ -55,6 +55,7 @@
 }
 
 .ai-label-button-wrapper:focus,
+.ai-label-button-wrapper:focus-within,
 .ai-label-button-wrapper:hover {
   width: fit-content;
   height: var(--sys-size-13);
@@ -105,7 +106,15 @@
   font-weight: var(--ref-typeface-weight-medium);
 }
 
-.input-field:focus {
+
+/* When the input field is focused we want to style it as a light background so
+ * it's clear that the user is in it and can edit the text.
+* However we also do this styling when the user's focus is on the GenerateAI
+* button (using the :focus-within on the parent). This is so if you open an
+* empty annotation, and then tab to the GenerateAI button, the text field
+* styling doesn't change. */
+.input-field:focus,
+.label-parts-wrapper:focus-within .input-field {
   background-color: var(--color-background);
   color: var(--color-background-inverted);
   outline: 2px solid var(--color-background-inverted);

front_end/ui/legacy/components/perf_ui/FlameChart.ts

@@ -1820,6 +1820,12 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
     if (event.key === 'Enter') {
       event.consume(true);
       this.dispatchEventToListeners(Events.ENTRY_INVOKED, this.selectedEntryIndex);
+
+      // Treat hitting enter on an entry just like we would clicking & create the annotation
+      this.dispatchEventToListeners(Events.ENTRY_LABEL_ANNOTATION_ADDED, {
+        entryIndex: this.selectedEntryIndex,
+        withLinkCreationButton: true,
+      });
       return true;
     }
     return false;