Merge pull request #2844 from heygen-com/fix/studio-color-hex-shortcuts-panel

fix(studio): colour hex editing and shortcuts panel dismissal

packages/studio/src/components/editor/propertyPanelColor.test.tsx

@@ -1,43 +1,85 @@
 // @vitest-environment happy-dom
 
 import React, { act } from "react";
-import { createRoot } from "react-dom/client";
-import { afterEach, describe, expect, it, vi } from "vitest";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { __resetDesignInputThrottle } from "../../utils/designInputTracking";
 import { ColorField } from "./propertyPanelColor";
 
+const trackStudioEvent = vi.hoisted(() => vi.fn());
+
+vi.mock("../../utils/studioTelemetry", () => ({
+  trackStudioEvent: (...args: unknown[]) => trackStudioEvent(...args),
+}));
+
 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
 
+const roots: Root[] = [];
+
+beforeEach(() => {
+  trackStudioEvent.mockReset();
+  __resetDesignInputThrottle();
+});
+
 afterEach(() => {
+  for (const root of roots) act(() => root.unmount());
+  roots.length = 0;
   document.body.innerHTML = "";
 });
 
-function renderColorField(onCommit: (value: string) => void): void {
+function renderColorField({
+  value = "#333333",
+  onPreview,
+  onCommit = vi.fn(),
+}: {
+  value?: string;
+  onPreview?: (value: string) => void;
+  onCommit?: (value: string) => void;
+} = {}): HTMLElement {
   const host = document.createElement("div");
   document.body.append(host);
   const root = createRoot(host);
+  roots.push(root);
   act(() => {
-    root.render(<ColorField flat label="Color" value="rgb(255, 176, 32)" onCommit={onCommit} />);
+    root.render(
+      <ColorField flat label="Color" value={value} onPreview={onPreview} onCommit={onCommit} />,
+    );
   });
+  return host;
+}
+
+function changeInput(input: HTMLInputElement, value: string): void {
+  const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set;
+  if (!setter) throw new Error("expected native input value setter");
+  setter.call(input, value);
+  input.dispatchEvent(new Event("input", { bubbles: true }));
+}
+
+function openHexInput(host: HTMLElement): HTMLInputElement {
+  const trigger = host.querySelector<HTMLButtonElement>('[data-flat-color-trigger="true"]');
+  if (!trigger) throw new Error("Color trigger was not rendered");
+  act(() => trigger.click());
+  const input = document.querySelector<HTMLInputElement>('input[spellcheck="false"]');
+  if (!input) throw new Error("Hex input was not rendered");
+  return input;
+}
+
+function clickOutside(): void {
+  document.body.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true }));
 }
 
 describe("ColorField flat trigger", () => {
   it("renders label and value inline with a small swatch, no boxed border", () => {
-    const host = document.createElement("div");
-    document.body.append(host);
-    const root = createRoot(host);
-    act(() => {
-      root.render(<ColorField flat label="Color" value="rgb(255, 176, 32)" onCommit={vi.fn()} />);
-    });
+    const host = renderColorField({ value: "rgb(255, 176, 32)" });
     const trigger = host.querySelector<HTMLButtonElement>('[data-flat-color-trigger="true"]');
     expect(trigger).not.toBeNull();
     expect(trigger?.className).not.toContain("border-neutral-800");
     expect(host.textContent).toContain("Color");
-    act(() => root.unmount());
   });
 
   it("persists one keyboard slider gesture on keyup", () => {
     const onCommit = vi.fn();
-    renderColorField(onCommit);
+    renderColorField({ value: "rgb(255, 176, 32)", onCommit });
     const trigger = document.querySelector<HTMLButtonElement>('[data-flat-color-trigger="true"]');
     if (!trigger) throw new Error("Color trigger was not rendered");
     act(() => {
@@ -57,3 +99,130 @@ describe("ColorField flat trigger", () => {
     expect(onCommit).toHaveBeenCalledOnce();
   });
 });
+
+describe("ColorField hex editing", () => {
+  it("allows #333333 to be backspaced to #3 without snapping", () => {
+    const input = openHexInput(renderColorField());
+
+    for (const value of ["#33333", "#3333", "#333", "#33", "#3"]) {
+      act(() => changeInput(input, value));
+      expect(input.value).toBe(value);
+    }
+  });
+
+  it("does not silently change #22CC66 to #2222CC while backspacing", () => {
+    const input = openHexInput(renderColorField({ value: "#22CC66" }));
+
+    for (const value of ["#22CC6", "#22CC", "#22C"]) {
+      act(() => changeInput(input, value));
+      expect(input.value).toBe(value);
+      expect(input.value).not.toBe("#2222CC");
+    }
+  });
+
+  it("commits a full replacement after selecting the existing value", () => {
+    const onCommit = vi.fn();
+    const input = openHexInput(renderColorField({ onCommit }));
+    input.focus();
+    input.select();
+
+    act(() => changeInput(input, "#12AB34"));
+    act(() => input.blur());
+
+    expect(onCommit).toHaveBeenCalledOnce();
+    expect(onCommit).toHaveBeenCalledWith("rgb(18, 171, 52)");
+  });
+
+  it("commits a complete pending hex on outside-click", () => {
+    const onCommit = vi.fn();
+    const input = openHexInput(renderColorField({ onCommit }));
+
+    act(() => changeInput(input, "#12AB34"));
+    act(clickOutside);
+
+    expect(onCommit).toHaveBeenCalledOnce();
+    expect(onCommit).toHaveBeenCalledWith("rgb(18, 171, 52)");
+  });
+
+  it("commits a 3-digit hex shorthand on outside-click", () => {
+    // parseCssColor accepts shorthand, so the gesture resolver has to as well;
+    // #F00 is ordinary designer input and used to be dropped in silence.
+    const onCommit = vi.fn();
+    const input = openHexInput(renderColorField({ onCommit }));
+
+    act(() => changeInput(input, "#F00"));
+    act(clickOutside);
+
+    expect(onCommit).toHaveBeenCalledOnce();
+    expect(onCommit).toHaveBeenCalledWith("rgb(255, 0, 0)");
+  });
+
+  it("does not commit an incomplete pending hex on outside-click", () => {
+    const onCommit = vi.fn();
+    const host = renderColorField({ value: "#224466", onCommit });
+    const input = openHexInput(host);
+
+    act(() => changeInput(input, "#12AB3"));
+    act(clickOutside);
+
+    expect(onCommit).not.toHaveBeenCalled();
+    // The settle also has to put the field back, or the panel re-opens showing
+    // a value the composition never took.
+    expect(openHexInput(host).value).toBe("#224466");
+  });
+
+  it("cancels a pending hex edit on Escape and restores the previous value", () => {
+    const onPreview = vi.fn();
+    const onCommit = vi.fn();
+    const host = renderColorField({ value: "#224466", onPreview, onCommit });
+    const input = openHexInput(host);
+
+    act(() => changeInput(input, "#12AB34"));
+    act(() => document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })));
+
+    expect(onCommit).not.toHaveBeenCalled();
+    expect(onPreview).toHaveBeenLastCalledWith("rgb(34, 68, 102)");
+    expect(openHexInput(host).value).toBe("#224466");
+  });
+
+  it("still commits a complete hex on Tab-blur", () => {
+    const onCommit = vi.fn();
+    const input = openHexInput(renderColorField({ onCommit }));
+    input.focus();
+
+    act(() => changeInput(input, "#12AB34"));
+    act(() => input.blur());
+
+    expect(onCommit).toHaveBeenCalledOnce();
+    expect(onCommit).toHaveBeenCalledWith("rgb(18, 171, 52)");
+  });
+
+  it("live-previews only a hex length that parses, 3 or 6 digits", () => {
+    const onPreview = vi.fn();
+    const input = openHexInput(renderColorField({ value: "#112233", onPreview }));
+
+    // 4 and 5 digits are mid-typing, so they must stay silent.
+    act(() => changeInput(input, "#3333"));
+    act(() => changeInput(input, "#33333"));
+    expect(onPreview).not.toHaveBeenCalled();
+
+    act(() => changeInput(input, "#333333"));
+    expect(onPreview).toHaveBeenCalledOnce();
+    expect(onPreview).toHaveBeenCalledWith("rgb(51, 51, 51)");
+
+    act(() => changeInput(input, "#333"));
+    expect(onPreview).toHaveBeenCalledTimes(2);
+    expect(onPreview).toHaveBeenLastCalledWith("rgb(51, 51, 51)");
+  });
+
+  it("tracks exactly once per completed edit, not once per keystroke", () => {
+    const input = openHexInput(renderColorField());
+
+    for (const value of ["#", "#1", "#12", "#12A", "#12AB", "#12AB3", "#12AB34"]) {
+      act(() => changeInput(input, value));
+    }
+    act(clickOutside);
+
+    expect(trackStudioEvent).toHaveBeenCalledOnce();
+  });
+});

packages/studio/src/components/editor/propertyPanelColor.tsx

@@ -184,11 +184,27 @@ export function ColorField({
   const brightnessPercent = Math.round(hsv.value * 100);
   const alphaPercent = Math.round(draftColor.alpha * 100);
 
-  const updateColorDraft = useCallback((nextValue: string) => {
+  const updateColorDraft = useCallback((nextValue: string, source: "hex" | "picker") => {
     const nextColor = parseCssColor(nextValue);
     if (!nextColor) return;
     setDraftColor(nextColor);
-    setHexDraft(toHexColor(nextColor).toUpperCase());
+    if (source === "picker") setHexDraft(toHexColor(nextColor).toUpperCase());
+  }, []);
+  const resolveColorGestureValue = useCallback((nextValue: string) => {
+    const source = nextValue.startsWith("#") ? "hex" : "picker";
+    // Only a COMPLETE hex resolves, so a half-typed one neither previews nor
+    // commits. Both lengths parseCssColor accepts count as complete: gating on
+    // 6 alone silently dropped #F00 and friends, which the old onBlur committed.
+    if (source === "hex" && !/^#([0-9a-f]{3}|[0-9a-f]{6})$/i.test(nextValue)) return null;
+    const nextColor = parseCssColor(nextValue);
+    if (!nextColor) return null;
+    return {
+      source,
+      value: formatCssColor({
+        ...nextColor,
+        alpha: source === "hex" ? draftColorRef.current.alpha : nextColor.alpha,
+      }),
+    } as const;
   }, []);
   const persistColorValue = useCallback(
     (nextValue: string) => {
@@ -203,12 +219,17 @@ export function ColorField({
     settle: settleColorGesture,
     cancel: cancelColorGesture,
   } = useInspectorGestureTransaction({
-    sourceValue: value,
+    sourceValue: formatCssColor(colorFromCss(value)),
     onPreview: (nextValue) => {
-      updateColorDraft(nextValue);
-      onPreview?.(nextValue);
+      const resolved = resolveColorGestureValue(nextValue);
+      if (!resolved) return;
+      updateColorDraft(resolved.value, resolved.source);
+      onPreview?.(resolved.value);
+    },
+    onCommit: (nextValue) => {
+      const resolved = resolveColorGestureValue(nextValue);
+      if (resolved) persistColorValue(resolved.value);
     },
-    onCommit: persistColorValue,
   });
 
   useEffect(() => {
@@ -288,13 +309,11 @@ export function ColorField({
     commitHsv({ saturation, value: nextValue });
   };
 
-  const handleHexCommit = (nextHex: string) => {
+  const handleHexChange = (nextHex: string) => {
     setHexDraft(nextHex);
     const normalized = nextHex.trim().startsWith("#") ? nextHex.trim() : `#${nextHex.trim()}`;
-    const parsed = parseCssColor(normalized);
-    if (!parsed) return;
-    const nextValue = formatCssColor({ ...parsed, alpha: draftColorRef.current.alpha });
-    updateColorDraft(nextValue);
+    beginColorGesture();
+    previewColorGesture(normalized);
   };
 
   const picker = open
@@ -413,21 +432,8 @@ export function ColorField({
               <span className={LABEL}>Hex</span>
               <input
                 value={hexDraft}
-                onChange={(event) => handleHexCommit(event.target.value)}
-                onBlur={() => {
-                  const normalized = hexDraft.trim().startsWith("#")
-                    ? hexDraft.trim()
-                    : `#${hexDraft.trim()}`;
-                  const parsed = parseCssColor(normalized);
-                  if (parsed) {
-                    const nextValue = formatCssColor({
-                      ...parsed,
-                      alpha: draftColorRef.current.alpha,
-                    });
-                    persistColorValue(nextValue);
-                  }
-                  setHexDraft(toHexColor(draftColorRef.current).toUpperCase());
-                }}
+                onChange={(event) => handleHexChange(event.target.value)}
+                onBlur={settleColorGesture}
                 className={`${FIELD} h-10 w-full text-[11px] font-medium outline-none`}
                 spellCheck={false}
               />

packages/studio/src/player/components/ShortcutsPanel.test.tsx

@@ -0,0 +1,158 @@
+// @vitest-environment happy-dom
+
+import React, { act, Profiler } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { ShortcutsPanel } from "./ShortcutsPanel";
+
+(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+const roots: Root[] = [];
+
+afterEach(() => {
+  for (const root of roots.splice(0)) act(() => root.unmount());
+  document.body.innerHTML = "";
+});
+
+function renderPanel(onRender = vi.fn()) {
+  const host = document.createElement("div");
+  document.body.append(host);
+  const root = createRoot(host);
+  roots.push(root);
+  act(() => {
+    root.render(
+      <Profiler id="shortcuts-panel" onRender={onRender}>
+        <ShortcutsPanel
+          disabled={false}
+          duration={10}
+          inPoint={null}
+          outPoint={null}
+          setInPoint={vi.fn()}
+          setOutPoint={vi.fn()}
+          onSeek={vi.fn()}
+        />
+      </Profiler>,
+    );
+  });
+
+  const trigger = host.querySelector<HTMLButtonElement>(
+    'button[aria-label="Shortcuts and tools"]',
+  )!;
+  return { host, trigger, onRender };
+}
+
+function pressAndClick(target: HTMLElement): void {
+  target.dispatchEvent(
+    new PointerEvent("pointerdown", { bubbles: true, cancelable: true, button: 0 }),
+  );
+  target.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0 }));
+  target.dispatchEvent(new MouseEvent("mouseup", { bubbles: true, cancelable: true, button: 0 }));
+  target.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true, button: 0 }));
+}
+
+function openPanel(trigger: HTMLButtonElement): void {
+  act(() => pressAndClick(trigger));
+  expect(trigger.getAttribute("aria-expanded")).toBe("true");
+}
+
+describe("ShortcutsPanel", () => {
+  it.each(["page", "button", "panel"] as const)(
+    "closes with Escape from the %s-focused position",
+    (focusPosition) => {
+      const { host, trigger } = renderPanel();
+      openPanel(trigger);
+
+      let eventTarget: Document | HTMLElement;
+      if (focusPosition === "page") {
+        document.body.tabIndex = -1;
+        document.body.focus();
+        eventTarget = document;
+        expect(document.activeElement).toBe(document.body);
+      } else if (focusPosition === "button") {
+        trigger.focus();
+        eventTarget = trigger;
+        expect(document.activeElement).toBe(trigger);
+      } else {
+        const panelInput = host.querySelector<HTMLInputElement>('[aria-label="Jump to frame"]')!;
+        panelInput.focus();
+        eventTarget = panelInput;
+        expect(document.activeElement).toBe(panelInput);
+      }
+
+      act(() => {
+        eventTarget.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }));
+      });
+
+      expect(trigger.getAttribute("aria-expanded")).toBe("false");
+      expect(host.querySelector('[aria-label="Jump to frame"]')).toBeNull();
+    },
+  );
+
+  it("closes on capture-phase pointerdown even when its default is prevented", () => {
+    const preventDefault = (event: Event) => event.preventDefault();
+    document.addEventListener("pointerdown", preventDefault, true);
+    const { host, trigger } = renderPanel();
+    openPanel(trigger);
+    const outside = document.createElement("div");
+    document.body.append(outside);
+    const pointerDown = new PointerEvent("pointerdown", {
+      bubbles: true,
+      cancelable: true,
+      button: 0,
+    });
+
+    act(() => outside.dispatchEvent(pointerDown));
+
+    document.removeEventListener("pointerdown", preventDefault, true);
+    expect(pointerDown.defaultPrevented).toBe(true);
+    expect(trigger.getAttribute("aria-expanded")).toBe("false");
+    expect(host.querySelector('[aria-label="Jump to frame"]')).toBeNull();
+  });
+
+  it("does not close on a click inside the panel", () => {
+    const { host, trigger } = renderPanel();
+    openPanel(trigger);
+    const panelInput = host.querySelector<HTMLInputElement>('[aria-label="Jump to frame"]')!;
+
+    act(() => pressAndClick(panelInput));
+
+    expect(trigger.getAttribute("aria-expanded")).toBe("true");
+    expect(host.querySelector('[aria-label="Jump to frame"]')).not.toBeNull();
+  });
+
+  it("toggles once per trigger click", () => {
+    const { trigger } = renderPanel();
+
+    act(() => pressAndClick(trigger));
+    expect(trigger.getAttribute("aria-expanded")).toBe("true");
+
+    act(() => pressAndClick(trigger));
+    expect(trigger.getAttribute("aria-expanded")).toBe("false");
+  });
+
+  it("does not re-render when Escape is pressed while closed", () => {
+    const { trigger, onRender } = renderPanel();
+    expect(onRender).toHaveBeenCalledTimes(1);
+
+    act(() => {
+      document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }));
+    });
+
+    expect(trigger.getAttribute("aria-expanded")).toBe("false");
+    expect(onRender).toHaveBeenCalledTimes(1);
+  });
+
+  it("connects the trigger to the dialog with aria-controls", () => {
+    const { host, trigger } = renderPanel();
+    openPanel(trigger);
+    const panelId = trigger.getAttribute("aria-controls");
+    const panel = panelId ? host.querySelector<HTMLElement>(`#${CSS.escape(panelId)}`) : null;
+
+    expect(panelId).not.toBeNull();
+    expect(panel?.getAttribute("role")).toBe("dialog");
+    // Non-modal on purpose: focus is not trapped and the editor behind stays
+    // operable, so aria-modal would lie to assistive tech about inertness.
+    expect(panel?.getAttribute("aria-modal")).toBeNull();
+    expect(panel?.id).toBe(panelId);
+  });
+});

packages/studio/src/player/components/ShortcutsPanel.tsx

@@ -1,6 +1,7 @@
-import { useState, useCallback, useRef, useEffect, memo } from "react";
+import { useState, useCallback, useId, memo } from "react";
 import { formatTime, frameToSeconds } from "../lib/time";
 import { Tooltip } from "../../components/ui";
+import { useContextMenuDismiss } from "../../hooks/useContextMenuDismiss";
 
 const SHORTCUT_SECTIONS = [
   {
@@ -108,20 +109,9 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
 }: ShortcutsPanelProps) {
   const [showShortcuts, setShowShortcuts] = useState(false);
   const [jumpFrame, setJumpFrame] = useState("");
-  const shortcutsPanelRef = useRef<HTMLDivElement>(null);
-
-  useEffect(() => {
-    if (!showShortcuts) return;
-    const handleMouseDown = (e: MouseEvent) => {
-      if (shortcutsPanelRef.current && !shortcutsPanelRef.current.contains(e.target as Node)) {
-        setShowShortcuts(false);
-      }
-    };
-    document.addEventListener("mousedown", handleMouseDown);
-    return () => {
-      document.removeEventListener("mousedown", handleMouseDown);
-    };
-  }, [showShortcuts]);
+  const shortcutsPanelId = useId();
+  const closeShortcuts = useCallback(() => setShowShortcuts(false), []);
+  const shortcutsPanelRef = useContextMenuDismiss(closeShortcuts);
 
   const commitJumpFrame = useCallback(() => {
     if (disabled) return;
@@ -158,6 +148,7 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
           }`}
           aria-label="Shortcuts and tools"
           aria-expanded={showShortcuts}
+          aria-controls={shortcutsPanelId}
         >
           <svg
             width="11"
@@ -177,6 +168,11 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
       </Tooltip>
       {showShortcuts && (
         <div
+          id={shortcutsPanelId}
+          role="dialog"
+          // Deliberately NOT aria-modal. This is a non-modal disclosure: focus is
+          // not trapped and the rest of the editor stays operable, so claiming
+          // modality would make assistive tech treat the whole app as inert.
           className="absolute bottom-full right-0 mb-2 z-50 rounded-lg shadow-xl min-w-[220px] overflow-y-auto"
           style={{
             background: "#161618",