[styles] make color-mix keyboard interactable

This change allows selecting color-mix swatches with the keyboard by
using the left/right arrows when a style rules is selected. When a
color-mix is focused, alt+down activates its popover.

To get the focusing right (focus only the icon and not the entire
function call), the color-mix text is moved out of the icon's DOM
structure.

Bug: 396311936
Change-Id: Ibd3b095961beaf97a6538579a605b2955ec076ce
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6286012
Reviewed-by: Ergün Erdoğmuş <[email protected]>
Commit-Queue: Philip Pfaffe <[email protected]>

Author: pfaffe

front_end/panels/elements/StylePropertyTreeElement.test.ts

@@ -190,7 +190,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
 
         const colorMixSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-mix-swatch');
         assert.exists(colorMixSwatch);
-        assert.isTrue(colorMixSwatch.textContent?.includes('var(--space)'));
       });
 
       it('should show color mix swatch when color-mix is used with an known variable in interpolation method even if it is not a valid method',
@@ -201,7 +200,6 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
 
            const colorMixSwatch = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-mix-swatch');
            assert.exists(colorMixSwatch);
-           assert.isTrue(colorMixSwatch.textContent?.includes('var(--garbage-space)'));
          });
 
       it('should not show color mix swatch when color-mix is used with an unknown variable in interpolation method',
@@ -225,7 +223,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
         renderElementIntoDOM(colorMixSwatch);
 
         assert.isTrue(addPopoverSpy.calledOnce);
-        assert.strictEqual(addPopoverSpy.args[0][0], colorMixSwatch.icon);
+        assert.strictEqual(addPopoverSpy.args[0][0], colorMixSwatch);
         assert.strictEqual(addPopoverSpy.args[0][1].contents()?.textContent, '#ff8000');
       });
 
@@ -239,7 +237,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
         renderElementIntoDOM(colorMixSwatch);
 
         assert.isTrue(addPopoverSpy.calledOnce);
-        assert.strictEqual(addPopoverSpy.args[0][0], colorMixSwatch.icon);
+        assert.strictEqual(addPopoverSpy.args[0][0], colorMixSwatch);
         assert.strictEqual(addPopoverSpy.args[0][1].contents()?.textContent, 'color(srgb 1 0.24 0.17)');
       });
 
@@ -248,12 +246,14 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
             getTreeElement('color', 'color-mix(in srgb, color-mix(in oklch, red, green), blue)');
         stylePropertyTreeElement.updateTitle();
 
-        const outerColorMix = stylePropertyTreeElement.valueElement?.querySelector('devtools-color-mix-swatch');
+        assert.exists(stylePropertyTreeElement.valueElement);
+
+        const [outerColorMix, innerColorMix] =
+            Array.from(stylePropertyTreeElement.valueElement.querySelectorAll('devtools-color-mix-swatch'));
         assert.exists(outerColorMix);
+        assert.exists(innerColorMix);
         const handler = sinon.fake();
         outerColorMix.addEventListener(InlineEditor.ColorMixSwatch.Events.COLOR_CHANGED, handler);
-        const innerColorMix = outerColorMix.querySelector('devtools-color-mix-swatch');
-        assert.exists(innerColorMix);
         assert.strictEqual(outerColorMix.getText(), 'color-mix(in srgb, color-mix(in oklch, red, green), blue)');
         assert.strictEqual(innerColorMix.getText(), 'color-mix(in oklch, red, green)');
         innerColorMix.setFirstColor('blue');
@@ -262,7 +262,7 @@ describeWithMockConnection('StylePropertyTreeElement', () => {
 
         // setFirstColor does not actually update the rendered color swatches or the textContent, which is why the first
         // color is still red here.
-        const colorSwatch = innerColorMix.querySelector('devtools-color-swatch');
+        const colorSwatch = stylePropertyTreeElement.valueElement.querySelector('devtools-color-swatch');
         assert.isOk(colorSwatch);
         const newColor = colorSwatch.getColor()?.as(Common.Color.Format.HEX);
         assert.isOk(newColor);

front_end/panels/elements/StylePropertyTreeElement.ts

@@ -588,33 +588,29 @@ export class ColorMixRenderer extends rendererBase(SDK.CSSPropertyParserMatchers
     const space = match.space.map(space => context.matchedResult.getComputedText(space)).join(' ');
     const color1Text = match.color1.map(color => context.matchedResult.getComputedText(color)).join(' ');
     const color2Text = match.color2.map(color => context.matchedResult.getComputedText(color)).join(' ');
-    swatch.appendChild(contentChild);
+    swatch.tabIndex = -1;
     swatch.setColorMixText(`color-mix(${space}, ${color1Text}, ${color2Text})`);
-    swatch.setRegisterPopoverCallback(swatch => {
-      if (swatch.icon) {
-        this.#pane.addPopover(swatch.icon, {
-          contents: () => {
-            const color = swatch.mixedColor();
-            if (!color) {
-              return undefined;
-            }
-            const span = document.createElement('span');
-            span.style.padding = '11px 7px';
-            const rgb = color.as(Common.Color.Format.HEX);
-            const text = rgb.isGamutClipped() ? color.asString() : rgb.asString();
-            if (!text) {
-              return undefined;
-            }
-            span.appendChild(document.createTextNode(text));
-            return span;
-          },
-          jslogContext: 'elements.css-color-mix',
-        });
-      }
+    this.#pane.addPopover(swatch, {
+      contents: () => {
+        const color = swatch.mixedColor();
+        if (!color) {
+          return undefined;
+        }
+        const span = document.createElement('span');
+        span.style.padding = '11px 7px';
+        const rgb = color.as(Common.Color.Format.HEX);
+        const text = rgb.isGamutClipped() ? color.asString() : rgb.asString();
+        if (!text) {
+          return undefined;
+        }
+        span.appendChild(document.createTextNode(text));
+        return span;
+      },
+      jslogContext: 'elements.css-color-mix',
     });
 
     context.addControl('color', swatch);
-    return [swatch];
+    return [swatch, contentChild];
   }
 
   matcher(): SDK.CSSPropertyParserMatchers.ColorMixMatcher {

front_end/panels/elements/StylesSidebarPane.ts

@@ -396,7 +396,7 @@ export class StylesSidebarPane extends Common.ObjectWrapper.eventMixin<EventType
     this.#genericPopoverHelper.setTimeout(500, 200);
   }
 
-  addPopover(element: Node, popover: {contents: () => HTMLElement | undefined, jslogContext?: string}): void {
+  addPopover(element: HTMLElement, popover: {contents: () => HTMLElement | undefined, jslogContext?: string}): void {
     this.#elementPopoverHooks.set(element, popover);
   }
 

front_end/panels/network/NetworkWaterfallColumn.ts

@@ -227,7 +227,10 @@ export class NetworkWaterfallColumn extends UI.Widget.VBox {
     }
   }
 
-  private getPopoverRequest(event: MouseEvent): UI.PopoverHelper.PopoverRequest|null {
+  private getPopoverRequest(event: MouseEvent|KeyboardEvent): UI.PopoverHelper.PopoverRequest|null {
+    if (event instanceof KeyboardEvent) {
+      return null;
+    }
     if (!this.hoveredNode) {
       return null;
     }

front_end/panels/protocol_monitor/JSONEditor.ts

@@ -460,7 +460,7 @@ export class JSONEditor extends Common.ObjectWrapper.eventMixin<EventTypes, type
     };
   }
 
-  #handlePopoverDescriptions(event: MouseEvent):
+  #handlePopoverDescriptions(event: MouseEvent|KeyboardEvent):
       {box: AnchorBox, show: (popover: UI.GlassPane.GlassPane) => Promise<boolean>}|null {
     const hintElement = event.composedPath()[0] as HTMLElement;
     const elementData = this.#getDescriptionAndTypeForElement(hintElement);

front_end/panels/sources/DebuggerPlugin.ts

@@ -660,7 +660,10 @@ export class DebuggerPlugin extends Plugin {
         tokenType === 'PropertyDefinition';
   }
 
-  private getPopoverRequest(event: MouseEvent): UI.PopoverHelper.PopoverRequest|null {
+  private getPopoverRequest(event: MouseEvent|KeyboardEvent): UI.PopoverHelper.PopoverRequest|null {
+    if (event instanceof KeyboardEvent) {
+      return null;
+    }
     if (UI.KeyboardShortcut.KeyboardShortcut.eventHasCtrlEquivalentKey(event)) {
       return null;
     }

front_end/ui/legacy/PopoverHelper.ts

@@ -43,7 +43,7 @@ export class PopoverHelper {
   };
   private disableOnClick: boolean;
   private hasPadding: boolean;
-  private getRequest: (arg0: MouseEvent) => PopoverRequest | null;
+  private getRequest: (arg0: MouseEvent|KeyboardEvent) => PopoverRequest | null;
   private scheduledRequest: PopoverRequest|null;
   private hidePopoverCallback: (() => void)|null;
   readonly container: Element;
@@ -54,8 +54,11 @@ export class PopoverHelper {
   private readonly boundMouseDown: (event: Event) => void;
   private readonly boundMouseMove: (ev: Event) => void;
   private readonly boundMouseOut: (event: Event) => void;
+  private readonly boundKeyUp: (ev: Event) => void;
   jslogContext?: string;
-  constructor(container: Element, getRequest: (arg0: MouseEvent) => PopoverRequest | null, jslogContext?: string) {
+  constructor(
+      container: Element, getRequest: (arg0: MouseEvent|KeyboardEvent) => PopoverRequest | null,
+      jslogContext?: string) {
     this.disableOnClick = false;
     this.hasPadding = false;
     this.getRequest = getRequest;
@@ -70,9 +73,11 @@ export class PopoverHelper {
     this.boundMouseDown = this.mouseDown.bind(this);
     this.boundMouseMove = this.mouseMove.bind(this);
     this.boundMouseOut = this.mouseOut.bind(this);
+    this.boundKeyUp = this.keyUp.bind(this);
     this.container.addEventListener('mousedown', this.boundMouseDown, false);
     this.container.addEventListener('mousemove', this.boundMouseMove, false);
     this.container.addEventListener('mouseout', this.boundMouseOut, false);
+    this.container.addEventListener('keyup', this.boundKeyUp, false);
     this.setTimeout(1000);
   }
 
@@ -108,6 +113,23 @@ export class PopoverHelper {
     this.startShowPopoverTimer((event as MouseEvent), 0);
   }
 
+  private keyUp(ev: Event): void {
+    const event = ev as KeyboardEvent;
+    if (event.altKey && event.key === 'ArrowDown') {
+      if (this.isPopoverVisible()) {
+        this.hidePopover();
+      } else {
+        this.stopShowPopoverTimer();
+        this.startHidePopoverTimer(0);
+        this.startShowPopoverTimer(event, 0);
+      }
+      ev.stopPropagation();
+    } else if (event.key === 'Escape' && this.isPopoverVisible()) {
+      this.hidePopover();
+      ev.stopPropagation();
+    }
+  }
+
   private mouseMove(ev: Event): void {
     const event = (ev as MouseEvent);
     if (this.eventInScheduledContent(event)) {
@@ -163,7 +185,7 @@ export class PopoverHelper {
     }, timeout);
   }
 
-  private startShowPopoverTimer(event: MouseEvent, timeout: number): void {
+  private startShowPopoverTimer(event: MouseEvent|KeyboardEvent, timeout: number): void {
     this.scheduledRequest = this.getRequest.call(null, event);
     if (!this.scheduledRequest) {
       return;

front_end/ui/legacy/components/inline_editor/ColorMixSwatch.test.ts

@@ -19,42 +19,22 @@ function createSwatch(text: string, firstColor: string, secondColor: string) {
 }
 
 describeWithLocale('ColorMixSwatch', () => {
-  it('should render color-mix swatch with icon and text when the syntax is correct', () => {
+  it('should render color-mix swatch icon when the syntax is correct', () => {
     const swatch = createSwatch('color-mix(in srgb, red, blue)', 'red', 'blue');
 
     const swatchIcon = swatch.shadowRoot?.querySelector('.swatch-icon');
 
-    assert.strictEqual(swatch.shadowRoot?.textContent?.trim(), 'color-mix(in srgb, red, blue)');
+    assert.strictEqual(swatch?.getText(), 'color-mix(in srgb, red, blue)');
     assert.isNotNull(swatchIcon);
   });
 
   it('should changing the second color work correctly when the colors are the same', () => {
     const swatch = createSwatch('color-mix(in srgb, red, red)', 'red', 'red');
     swatch.setSecondColor('blue');
 
-    const swatchIcon = swatch.shadowRoot?.querySelector('.swatch-icon');
+    assert.strictEqual(swatch?.getText(), 'color-mix(in srgb, red, blue)');
 
-    assert.strictEqual(swatch.shadowRoot?.textContent?.trim(), 'color-mix(in srgb, red, blue)');
+    const swatchIcon = swatch.shadowRoot?.querySelector('.swatch-icon');
     assert.isNotNull(swatchIcon);
   });
-
-  it('calls the popover registration callback upon rendering', () => {
-    const swatch = createSwatch('color-mix(in srgb, red, red)', 'red', 'red');
-
-    const cb = sinon.stub<[InlineEditor.ColorMixSwatch.ColorMixSwatch], void>();
-    const values: string[] = [];
-    cb.callsFake(swatch => values.push(swatch.shadowRoot?.textContent?.trim() ?? ''));
-    swatch.setRegisterPopoverCallback(cb);
-    swatch.setFirstColor('blue');
-    swatch.setSecondColor('purple');
-    swatch.setColorMixText('color-mix(in hsl, yellow, yellow)');
-    assert.lengthOf(cb.getCalls(), 4);
-
-    assert.deepEqual(values, [
-      'color-mix(in srgb, red, red)',
-      'color-mix(in srgb, blue, red)',
-      'color-mix(in srgb, blue, purple)',
-      'color-mix(in hsl, yellow, yellow)',
-    ]);
-  });
 });

front_end/ui/legacy/components/inline_editor/ColorMixSwatch.ts

@@ -4,7 +4,7 @@
 
 import * as Common from '../../../../core/common/common.js';
 import * as Platform from '../../../../core/platform/platform.js';
-import {html, render} from '../../../lit/lit.js';
+import * as Lit from '../../../lit/lit.js';
 import * as VisualLogging from '../../../visual_logging/visual_logging.js';
 
 import colorMixSwatchStylesRaw from './colorMixSwatch.css.js';
@@ -13,6 +13,8 @@ import colorMixSwatchStylesRaw from './colorMixSwatch.css.js';
 const colorMixSwatchStyles = new CSSStyleSheet();
 colorMixSwatchStyles.replaceSync(colorMixSwatchStylesRaw.cssContent);
 
+const {html, render, Directives: {ref}} = Lit;
+
 export const enum Events {
   COLOR_CHANGED = 'colorChanged',
 }
@@ -26,7 +28,7 @@ export class ColorMixSwatch extends Common.ObjectWrapper.eventMixin<EventTypes,
   private colorMixText = '';     // color-mix(in srgb, hotpink, white)
   private firstColorText = '';   // hotpink
   private secondColorText = '';  // white
-  #registerPopoverCallback: undefined|((swatch: ColorMixSwatch) => void);
+  #icon: HTMLElement|null = null;
 
   constructor() {
     super();
@@ -35,12 +37,8 @@ export class ColorMixSwatch extends Common.ObjectWrapper.eventMixin<EventTypes,
     ];
   }
 
-  get icon(): Element|null {
-    return this.shadow.firstElementChild;
-  }
-
   mixedColor(): Common.Color.Color|null {
-    const colorText = this.icon?.computedStyleMap().get('color')?.toString() ?? null;
+    const colorText = this.#icon?.computedStyleMap().get('color')?.toString() ?? null;
     return colorText ? Common.Color.parse(colorText) : null;
   }
 
@@ -75,11 +73,6 @@ export class ColorMixSwatch extends Common.ObjectWrapper.eventMixin<EventTypes,
     this.#render();
   }
 
-  setRegisterPopoverCallback(callback: (swatch: ColorMixSwatch) => void): void {
-    this.#registerPopoverCallback = callback;
-    callback(this);
-  }
-
   getText(): string {
     return this.colorMixText;
   }
@@ -98,15 +91,15 @@ export class ColorMixSwatch extends Common.ObjectWrapper.eventMixin<EventTypes,
     // Note also that whitespace between nodes is removed on purpose to avoid pushing these elements apart. Do not
     // re-format the HTML code.
     render(
-      html`<div class="swatch-icon" jslog=${VisualLogging.cssColorMix()} style="--color: ${this.colorMixText}">
+      html`<div class="swatch-icon"
+      ${ref(e => {this.#icon = e as HTMLElement; })}
+      jslog=${VisualLogging.cssColorMix()}
+      style="--color: ${this.colorMixText}">
         <span class="swatch swatch-left" id="swatch-1" style="--color: ${this.firstColorText}"></span>
         <span class="swatch swatch-right" id="swatch-2" style="--color: ${this.secondColorText}"></span>
-        <span class="swatch swatch-mix" id="mix-result" style="--color: ${this.colorMixText}"></span>
-      </div><slot>${this.colorMixText}</slot>`,
+        <span class="swatch swatch-mix" id="mix-result" style="--color: ${this.colorMixText}"></span></div>`,
       this.shadow, {host: this});
     // clang-format on
-
-    this.#registerPopoverCallback && this.#registerPopoverCallback(this);
   }
 }
 

front_end/ui/legacy/components/inline_editor/colorMixSwatch.css

@@ -10,7 +10,7 @@
   inline-size: 15px;
   grid: [stack] 1fr / [stack] 1fr;
   margin-left: 1px;
-  margin-right: 2px;
+  margin-right: 1px;
   color: var(--color);
 }