feat(studio): shared menu, popover and value-control primitives - #4191
Conversation
One menu chrome and one popover chrome, on Base UI, wearing the theme tokens. Dismissal is plain Base UI with no capture-phase wrapper: its outside-press listeners are registered on document with capture set, so the canvas overlay's bubble-phase stopPropagation cannot starve them. Menu items carry an optional mono shortcut hint, a disabled state and a danger tone. ContextMenu opens at the pointer. Popover shares the surface, radius and open motion but owns no item semantics, so a text field inside it keeps its own keys. Also adds [role='menuitemradio'] to the playback-shortcut ignore list. The hand-rolled speed menu spelled its rows as buttons, which that list already matched; the shared radio item is a div with the same role, and nothing else in the list matched it, so an open speed menu would have leaked arrow keys and Space to the player.
…imitives The five value controls exist once, on Base UI, wearing the theme tokens. Each is boxed with a visible border and a focus ring so an inspector value reads as an input rather than as its own label. Value writes happen at a commit boundary, never per keystroke or per pointer move: one drag is one onCommit and one telemetry call. Right-click and Escape abort a drag in flight and put the pre-drag value back. The switch role joins the typing-target selector list, so a Toggle keeps the keys the native checkbox it replaces owned. No consumer moves onto these yet.
… string reading as a class Main's comment gate rejects long comment blocks, and the class gate read the MenuItem tone default as a Tailwind class. The prop is optional instead.
A host mounting Studio pieces in its own tree imports them from the package: cn, Menu family, Popover, Input, NumberField, Select, Slider and Toggle.
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
APPROVING at ab8afaa1 — new-primitives-only scope confirmed; fold is byte-identical to source PRs; two additive selector rows are covered by KTD13 parity tests
Stacked on hf#4181 (c610d891) and hf#4186 (e505dcc5). Isolated hf#4191's own contribution against e505dcc5 (author-scoped delta above the sibling squashes).
Scope — new primitives + tests + 2 additive selector rows, nothing else
13 files, +1789/−0. Every touch verified:
- New (10):
Input.tsx,NumberField.tsx,Select.tsx,Slider.tsx,Toggle.tsx,Menu.tsx,Popover.tsx+Menu.test.tsx+valueControls.test.tsx(~900 lines of tests). Add-only, no existing files touched. - Barrel exports (2):
packages/studio/src/components/ui/index.ts(+22 exports for the new primitives) andpackages/studio/src/index.ts(+23 package-root exports). Pure additive. - Selector-list rows (2):
typingTarget.ts(+1 selector:[role='switch']) andplaybackShortcuts.ts(+1 selector:[role='menuitemradio']). Both add-only, no removals.
Grep for imports of the new primitives outside packages/studio/src/components/ui/: only packages/studio/src/index.ts (the package barrel). No in-tree consumer wired in. Miguel's "no call sites changed yet" claim holds.
Fold parity — replay is byte-identical to source PRs
hf#3625 (Menu family) and hf#3626 (value controls) are both still OPEN. I compared each file in #4191 at the pre-trim commit (f897548f7) against the source PRs' heads (05961175 / 7263c927):
- IDENTICAL:
Menu.tsx,Menu.test.tsx,Popover.tsxvs hf#3625 - IDENTICAL:
Input.tsx,NumberField.tsx,Select.tsx,Slider.tsx,Toggle.tsx,valueControls.test.tsxvs hf#3626
All 9 replayed files are blob-SHA-identical to their sources. Nothing dropped in the fold.
The trim commit — pure comment shrink + one semantic change
0a04fffd5 trims 235 lines from 10 files. I scanned every non-blank/non-comment diff line in the affected files:
Menu.tsx: ONE non-comment change — removedtone = "default"default value:Consumer code uses-export function MenuItem({ shortcut, tone = "default", className, children, ...props }...) +export function MenuItem({ shortcut, tone, className, children, ...props }...)
tone === "danger" && itemDanger, soundefined === "danger"is false — semantically identical to"default" === "danger". The reason for the fix: the class gate was reading the string default as a Tailwind candidate. Makingtoneoptional stops that without changing runtime behaviour.Input.tsx,NumberField.tsx,Popover.tsx,Select.tsx,Slider.tsx,Toggle.tsx,playbackShortcuts.ts: zero non-comment/non-blank diff lines. Pure comment trim.
Space-key / typing-target — verified by mutation-red parity test
valueControls.test.tsx:485 ("hotkey classification (KTD13)") is the discriminator. It pairs the native reference on the left with the primitive on the right:
["native-checkbox", host.querySelector('[role="switch"]')!],
...
for (const [nativeId, primitive] of pairs) {
expect(isTypingTarget(primitive)).toBe(isTypingTarget(native(nativeId)));
expect(shouldIgnorePlaybackShortcutTarget(primitive)).toBe(shouldIgnorePlaybackShortcutTarget(native(nativeId)));
}Native <input type="checkbox"> matches the existing input selector → typing target = true. The <div role="switch"> primitive would NOT match any pre-PR selector → typing target = false → parity fails. Removing the new [role='switch'] line in typingTarget.ts breaks the test. Same mutation-red property for [role='menuitemradio'] in playbackShortcuts.ts (asserted at Menu.test.tsx:326).
Toggle also has a direct Space test at valueControls.test.tsx:461: focus → keydown/keyup{key:" "} → expect(flips).toEqual([true, true]). That's the positive path; KTD13 is the guard proving a global shortcut can't steal it.
PR-body follow-ups (author-declared, non-blocking)
Selectbuilds its own popup instead of usingpopupSurface— deferred hygiene.- Menu rows are not typing targets for app-level plain-key hotkeys (matches the old menus' behaviour).
- No test for
Inputkeeping a half-typed draft when value changes from outside.
None of these gate the merge; all are marked "not changed here" in the PR body.
CI
Studio and player captures and Test both show FAILURE at the earlier run, both superseded by later re-runs (per [[feedback_check_runs_api_returns_stale_runs.md]] — statusCheckRollup keeps stale rows). Latest state: 45 SUCCESS + 22 SKIPPED, 4 IN_PROGRESS (CI/Test re-run, Windows studio-core, Windows studio-engine-cli, Smoke). No live FAILURE. Stamping on code merit per [[feedback_ci_is_reference_not_gate]]; hf-oss require_last_push_approval=true means my APPROVE binds to the head; merge waits on the IN_PROGRESS lanes settling.
Stamp mechanics
reviewDecision: REVIEW_REQUIRED, mergeStateStatus: BLOCKED. APPROVE binds to ab8afaa1dc145a437ae3bec118ab17fb0c605cb3. Miguel miguel-heygen on trusted stamp list. Note this branch is stacked on hf#4181 + hf#4186 (both my approvals at their current heads); once those merge, hf#4191 rebases to their squash-commits.
— Review by tai (pr-review)
2 participants
Studio gets the shared Menu, ContextMenu, Popover, Input, NumberField, Select, Slider and Toggle primitives, built on the Button, Tabs and Tooltip primitives that already landed, and the package entry now exports them. Nothing in Studio uses them yet: a host app that mounts Studio pieces in its own tree (the desktop app) and the next sweeps (Renders panel, inspector, timeline toolbar) build on them.
What changes
fieldBase) so they all match.cn, the Menu family,Popover,Input,NumberField,Select,Slider,Toggle,fieldBase,fieldTextand their prop types are exported from@hyperframes/studio.Shape
MenuItem'stone = "default"string default was read as a Tailwind class, sotoneis optional now (same behaviour, since only"danger"is compared).Selectbuilds its own popup instead of usingpopupSurface; menu rows are not typing targets for the app-level plain-key hotkeys (the old menus behaved the same); no test forInputkeeping a half-typed draft when the value changes from outside.Before
Studio today: the inspector's Media section uses hand-rolled controls (select, toggle, sliders), and there is no shared menu, popover or value-control set.
After
The new primitives rendered together on the dark theme from a throwaway gallery page (not committed): input, number field, select, slider, switch, an open menu with a danger item, and an open popover. No Studio screen uses them yet.
Verification
uiandstylestests, 95 passed in 11 files, including the class gate and the hex ratchet (miga, exit 0);tsc --noEmitexit 0 forpackages/studio; oxlint 0 warnings;comment-checkclean against main.tonedefault.tonedefaultPopoverandfieldTextresolves through the package entry