fix(studio): keep the shortcuts panel above the player controls and inside the window (#5457)

* fix(studio): keep the shortcuts panel above the player controls and inside the window

The panel was drawn inline next to its button at z-50, so a host layer
above that (the desktop's playback bar, z-55) covered its Playback rows,
and the preview pane's overflow clip cut its top off in a short window.
It now opens through Studio's Popover on the shared popup layer: portaled
out of the button's boxes, held above the button, and capped to the room
there with a scrolling body.

* fix(studio): the shortcuts button keeps its tooltip description for screen readers
This commit is contained in:
Miguel Ángel
2026-10-11 10:21:21 +00:00
committed by GitHub
parent d4dd6c097d
commit ba08acb1f5
3 changed files with 122 additions and 83 deletions
@@ -35,12 +35,16 @@ interface PopoverProps extends Omit<ComponentPropsWithoutRef<typeof BasePopover.
side?: "top" | "bottom" | "left" | "right";
align?: "start" | "center" | "end";
sideOffset?: number;
collisionAvoidance?: ComponentPropsWithoutRef<
typeof BasePopover.Positioner
>["collisionAvoidance"];
/** Points the popup at its trigger. Off by default; the default gap from the trigger grows by the arrow's height. */
arrow?: boolean;
/** Portal target. Pass the shadow root when the trigger lives in one; `null` keeps it inline. */
container?: PortalContainer;
/** Names the popup for assistive tech. */
"aria-label"?: string;
"aria-describedby"?: string;
/**
* What to focus on open. Defaults to Base UI's behaviour (the first tabbable
* element), which is what a rename field wants.
@@ -57,17 +61,19 @@ export function Popover({
align = "center",
arrow = false,
sideOffset = arrow ? SIDE_OFFSET + ARROW_PX : SIDE_OFFSET,
collisionAvoidance,
container,
className,
initialFocus,
"aria-label": ariaLabel,
"aria-describedby": describedBy,
"data-preview-state": previewState,
...root
}: PopoverProps) {
const inline = useInlinePortal(container);
return (
<BasePopover.Root {...root}>
<BasePopover.Trigger render={trigger} />
<BasePopover.Trigger render={trigger} aria-describedby={describedBy} />
{inline.host}
<BasePopover.Portal {...inline.portal}>
<BasePopover.Positioner
@@ -75,6 +81,7 @@ export function Popover({
align={align}
sideOffset={sideOffset}
collisionPadding={VIEWPORT_MARGIN}
collisionAvoidance={collisionAvoidance}
arrowPadding={ARROW_CORNER_CLEARANCE}
className={POPUP_LAYER}
>
@@ -7,6 +7,7 @@ import { PlayerControls } from "./PlayerControls";
import { DEFAULT_SHORTCUT_SECTIONS, type ShortcutSection } from "./studioShortcuts";
import { createHappyDomRootHarness } from "./testRootHarness";
import { usePlayerStore } from "../store/playerStore";
import { POPUP_LAYER } from "../../components/ui/Menu";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
@@ -52,11 +53,16 @@ function openPanel(trigger: HTMLButtonElement): void {
expect(trigger.getAttribute("aria-expanded")).toBe("true");
}
/** The open popover a trigger controls; it portals out of the trigger's host. */
function panelOf(trigger: HTMLButtonElement): HTMLElement {
return document.getElementById(trigger.getAttribute("aria-controls")!)!;
}
describe("ShortcutsPanel", () => {
it.each(["page", "button", "panel"] as const)(
"closes with Escape from the %s-focused position",
(focusPosition) => {
const { host, trigger } = renderPanel();
const { trigger } = renderPanel();
openPanel(trigger);
let eventTarget: Document | HTMLElement;
@@ -70,7 +76,9 @@ describe("ShortcutsPanel", () => {
eventTarget = trigger;
expect(document.activeElement).toBe(trigger);
} else {
const panelInput = host.querySelector<HTMLInputElement>('[aria-label="Jump to frame"]')!;
const panelInput = document.querySelector<HTMLInputElement>(
'[aria-label="Jump to frame"]',
)!;
panelInput.focus();
eventTarget = panelInput;
expect(document.activeElement).toBe(panelInput);
@@ -81,40 +89,47 @@ describe("ShortcutsPanel", () => {
});
expect(trigger.getAttribute("aria-expanded")).toBe("false");
expect(host.querySelector('[aria-label="Jump to frame"]')).toBeNull();
expect(document.querySelector('[aria-label="Jump to frame"]')).toBeNull();
},
);
it("closes on capture-phase pointerdown even when its default is prevented", () => {
it("closes on an outside press whose pointerdown a canvas gesture prevented", () => {
const preventDefault = (event: Event) => event.preventDefault();
document.addEventListener("pointerdown", preventDefault, true);
const { host, trigger } = renderPanel();
openPanel(trigger);
// The canvas exists before the panel opens, as in the editor.
const outside = document.createElement("div");
document.body.append(outside);
const { trigger } = renderPanel();
openPanel(trigger);
const pointerDown = new PointerEvent("pointerdown", {
bubbles: true,
cancelable: true,
button: 0,
});
act(() => outside.dispatchEvent(pointerDown));
// A prevented pointerdown suppresses mousedown and mouseup; the browser still sends the click.
act(() => {
outside.dispatchEvent(pointerDown);
outside.dispatchEvent(
new MouseEvent("click", { bubbles: true, cancelable: true, button: 0 }),
);
});
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();
expect(document.querySelector('[aria-label="Jump to frame"]')).toBeNull();
});
it("does not close on a click inside the panel", () => {
const { host, trigger } = renderPanel();
const { trigger } = renderPanel();
openPanel(trigger);
const panelInput = host.querySelector<HTMLInputElement>('[aria-label="Jump to frame"]')!;
const panelInput = document.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();
expect(document.querySelector('[aria-label="Jump to frame"]')).not.toBeNull();
});
it("toggles once per trigger click", () => {
@@ -140,10 +155,10 @@ describe("ShortcutsPanel", () => {
});
it("connects the trigger to the dialog with aria-controls", () => {
const { host, trigger } = renderPanel();
const { trigger } = renderPanel();
openPanel(trigger);
const panelId = trigger.getAttribute("aria-controls");
const panel = panelId ? host.querySelector<HTMLElement>(`#${CSS.escape(panelId)}`) : null;
const panel = panelId ? document.querySelector<HTMLElement>(`#${CSS.escape(panelId)}`) : null;
expect(panelId).not.toBeNull();
expect(panel?.getAttribute("role")).toBe("dialog");
@@ -153,6 +168,21 @@ describe("ShortcutsPanel", () => {
expect(panel?.id).toBe(panelId);
});
it("opens on the popup layer, above its button and outside the boxes that hold it", async () => {
const { host, trigger } = renderPanel();
// A button at the window's top edge: the panel must not flip down over the timeline.
trigger.getBoundingClientRect = () => new DOMRect(900, 0, 28, 28);
openPanel(trigger);
await act(async () => {});
const panel = panelOf(trigger);
expect(host.contains(panel)).toBe(false);
expect(panel.closest(`.${POPUP_LAYER}`)).not.toBeNull();
expect(panel.dataset.side).toBe("top");
expect(panel.dataset.align).toBe("end");
expect(panel.classList).toContain("max-h-[min(280px,var(--available-height))]");
});
it("lists what an embedder passes to PlayerControls instead of Studio's defaults", () => {
const listed = (sections?: readonly ShortcutSection[]) => {
const host = document.createElement("div");
@@ -162,8 +192,11 @@ describe("ShortcutsPanel", () => {
<PlayerControls onTogglePlay={vi.fn()} onSeek={vi.fn()} shortcutSections={sections} />,
);
});
openPanel(host.querySelector<HTMLButtonElement>('button[aria-label="Shortcuts and tools"]')!);
return host.textContent ?? "";
const trigger = host.querySelector<HTMLButtonElement>(
'button[aria-label="Shortcuts and tools"]',
)!;
openPanel(trigger);
return panelOf(trigger).textContent ?? "";
};
const withoutSplit = DEFAULT_SHORTCUT_SECTIONS.map((section) => ({
...section,
@@ -196,12 +229,32 @@ describe("ShortcutsPanel", () => {
});
openPanel(host.querySelector<HTMLButtonElement>('button[aria-label="Shortcuts and tools"]')!);
expect(host.textContent).toContain("Select");
expect(host.textContent).toContain("Snap");
expect(document.body.textContent).toContain("Select");
expect(document.body.textContent).toContain("Snap");
expect(errors.mock.calls.flat().join(" ")).not.toMatch(/same key/);
errors.mockRestore();
});
it("ties the shortcuts button to its tooltip for screen readers", async () => {
const host = document.createElement("div");
document.body.append(host);
act(() => {
mount(host).render(<ShortcutsButton sections={[{ title: "Host", hints: [] }]} />);
});
const button = host.querySelector<HTMLButtonElement>(
'button[aria-label="Shortcuts and tools"]',
)!;
act(() => button.focus());
await act(async () => {
await new Promise((resolve) => setTimeout(resolve, 0));
});
const tip = document.querySelector<HTMLElement>('[role="tooltip"]');
expect(tip?.textContent).toBe("Shortcuts and tools");
expect(button.getAttribute("aria-describedby")).toBe(tip?.id);
});
it("renders standalone for a host toolbar, reading the player store", () => {
const { duration, inPoint, requestSeek: storeSeek } = usePlayerStore.getState();
const requestSeek = vi.fn();
@@ -213,10 +266,10 @@ describe("ShortcutsPanel", () => {
mount(host).render(<ShortcutsButton sections={[{ title: "Host", hints: [] }]} />);
});
openPanel(host.querySelector<HTMLButtonElement>('button[aria-label="Shortcuts and tools"]')!);
expect(host.textContent).toContain("Host");
expect(host.textContent).toContain("00:02");
expect(document.body.textContent).toContain("Host");
expect(document.body.textContent).toContain("00:02");
const input = host.querySelector<HTMLInputElement>('input[aria-label="Jump to frame"]')!;
const input = document.querySelector<HTMLInputElement>('input[aria-label="Jump to frame"]')!;
act(() => {
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(
input,
@@ -1,8 +1,7 @@
import { useState, useCallback, useEffect, useId, useRef, memo } from "react";
import { useState, useCallback, useRef, memo } from "react";
import { formatTime, frameToSeconds } from "../lib/time";
import { Tooltip } from "../../components/ui";
import { Popover, Tooltip } from "../../components/ui";
import { flatActive, flatIdle } from "../../components/timelineToolbarStyles";
import { useContextMenuDismiss } from "../../hooks/useContextMenuDismiss";
import { usePlayerStore } from "../store/playerStore";
import { DEFAULT_SHORTCUT_SECTIONS, type ShortcutSection } from "./studioShortcuts";
@@ -17,6 +16,9 @@ interface ShortcutsPanelProps {
sections?: readonly ShortcutSection[];
}
/** Stays above its button, shrinking to the room there, rather than flipping down over the timeline. */
const STAY_ABOVE = { side: "none", align: "shift", fallbackAxisSide: "none" } as const;
export const ShortcutsPanel = memo(function ShortcutsPanel({
disabled,
duration,
@@ -29,23 +31,8 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
}: ShortcutsPanelProps) {
const [showShortcuts, setShowShortcuts] = useState(false);
const [jumpFrame, setJumpFrame] = useState("");
const shortcutsPanelId = useId();
const closeShortcuts = useCallback(() => setShowShortcuts(false), []);
const shortcutsPanelRef = useContextMenuDismiss(closeShortcuts);
// Focus lands on the scrolling body, not the frame input, so keyboard users can scroll and read it.
const panelBodyRef = useRef<HTMLDivElement>(null);
const triggerRef = useRef<HTMLButtonElement>(null);
// Move focus into the panel on open so keyboard users can scroll and read
// it; hand focus back to the trigger on close.
// eslint-disable-next-line no-restricted-syntax
useEffect(() => {
if (!showShortcuts) return;
const trigger = triggerRef.current;
panelBodyRef.current?.focus();
return () => {
trigger?.focus();
};
}, [showShortcuts]);
const commitJumpFrame = useCallback(() => {
if (disabled) return;
@@ -72,48 +59,40 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
);
return (
<div ref={shortcutsPanelRef} className="relative shrink-0">
<Tooltip label="Shortcuts and tools">
<button
ref={triggerRef}
type="button"
onClick={() => setShowShortcuts((v) => !v)}
className={showShortcuts ? flatActive : flatIdle}
aria-label="Shortcuts and tools"
aria-expanded={showShortcuts}
aria-controls={shortcutsPanelId}
>
<svg
width="11"
height="11"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
strokeWidth="1.75"
strokeLinecap="round"
strokeLinejoin="round"
aria-hidden="true"
<Tooltip label="Shortcuts and tools">
<Popover
open={showShortcuts}
onOpenChange={setShowShortcuts}
side="top"
align="end"
collisionAvoidance={STAY_ABOVE}
aria-label="Keyboard shortcuts and tools"
initialFocus={panelBodyRef}
className="flex max-h-[min(280px,var(--available-height))] min-w-[220px] flex-col border-border bg-raised p-0"
trigger={
<button
type="button"
className={`${showShortcuts ? flatActive : flatIdle} shrink-0`}
aria-label="Shortcuts and tools"
>
<rect x="2" y="4" width="20" height="16" rx="2" />
<path d="M6 8h.01M10 8h.01M14 8h.01M18 8h.01M6 12h.01M10 12h.01M14 12h.01M18 12h.01M8 16h8" />
</svg>
</button>
</Tooltip>
{showShortcuts && (
<div
id={shortcutsPanelId}
ref={panelBodyRef}
tabIndex={-1}
role="dialog"
aria-label="Keyboard shortcuts and tools"
// 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 border border-border bg-raised shadow-popover min-w-[220px] overflow-y-auto outline-hidden"
style={{
maxHeight: "min(280px, calc(100vh - 80px))",
}}
>
<svg
width="11"
height="11"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
strokeWidth="1.75"
strokeLinecap="round"
strokeLinejoin="round"
aria-hidden="true"
>
<rect x="2" y="4" width="20" height="16" rx="2" />
<path d="M6 8h.01M10 8h.01M14 8h.01M18 8h.01M6 12h.01M10 12h.01M14 12h.01M18 12h.01M8 16h8" />
</svg>
</button>
}
>
<div ref={panelBodyRef} tabIndex={-1} className="min-h-0 overflow-y-auto outline-hidden">
<div className="px-3 pt-3 pb-2.5">
<p className="text-[9px] font-medium text-neutral-500 uppercase tracking-wider mb-1.5">
Jump to frame
@@ -247,8 +226,8 @@ export const ShortcutsPanel = memo(function ShortcutsPanel({
))}
</div>
</div>
)}
</div>
</Popover>
</Tooltip>
);
});