Skip to content

feat(studio): the Renders panel on the shared primitives - #3772

Merged
miguel-heygen merged 6 commits into
mainfrom
feat/studio-u13-renders-sweep
Sep 19, 2026
Merged

miguel-heygen merged 6 commits into
mainfrom
feat/studio-u13-renders-sweep

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Lands unit U13 (Renders panel sweep) of the Studio design-system foundation. Stacked on the value-controls PR (#3626). Hex ratchet unchanged: the two touched files hold no colour literals; the raw palette classes removed here are the token gate's domain. Bundle +19.2 KiB gzipped (first consumer of Select), inside the 100 KB budget.

What

Unit U13 of the Studio design-system plan: the Renders panel sweep.

  • The Format, Resolution, Frame rate and Quality controls become the shared Select. No native <select> is left in the Renders tab.
  • The panel's Export drops the type-size override it carried in its own className. Both Exports now measure the same.
  • The per-row Download and Delete controls become IconButton, each wrapped in the shared Tooltip carrying what the title attribute used to say.
  • The panel's colours and type sizes move onto the semantic tokens (text-4, bg-input, border-border, danger, accent, text-step-*).
  • SelectOption gains an optional disabled, passed through to Base UI's Select.Item.

Why

R8 asks for exactly one implementation of every control, and no native <select>: a native popup is the operating system's, so its radius, surface and type belong to someone else and no token can reach them.

AE3 asks that the header Export and the Renders Export share height, radius, fill and type size. They did not. The panel's Export carried its own type size in className, and cn resolves that by dropping the size recipe's own step, so the two Exports sat a type step apart. The captured computed-style table, before and after:

Control Height Radius Font size Background
Header Export (before and after) 28px 6px 12px rgb(60, 230, 172)
Renders Export (before) 28px 6px 11px rgb(60, 230, 172)
Renders Export (after) 28px 6px 12px rgb(60, 230, 172)

The disabled flag on SelectOption is not new behaviour, it is preserved behaviour. The Resolution list disables a preset that is not an exact integer upscale of the authored size, because the producer rejects that scale at render time. Without the flag the shared Select would have offered a choice that fails the render.

How

Option lists become data (FORMAT_OPTIONS, QUALITY_OPTIONS, FPS_OPTIONS, and the Resolution list built per render from the composition's dimensions), which is the shape Select takes. Each onCommit keeps the literal-union cast and the persistence side effect its onChange handler had. Frame rate crosses the string boundary explicitly: String(fps) in, Number(next) out.

The quality option list also carried a title field that nothing ever rendered; it is gone.

Test plan

RenderQueue.test.tsx, all through the component's public render, driven with the pointer and key sequences a browser produces:

  • AE3: the Export's class list equals the shared primary/medium recipe exactly, plus w-full. Set equality, not "contains", so an extra class that displaces a recipe class fails. Proved non-vacuous by re-adding the type-size override: the assertion fails naming the dropped step.
  • R8: querySelector("select") returns null.
  • KTD13: the Format trigger is classified by isTypingTarget and by the playback-shortcut selector exactly as a native <select> is, and both are asserted true rather than merely equal.
  • Changing the Format Select persists "mov", the literal union member, not the "MOV (ProRes)" label. Read back through the real store, not a spy.
  • Choosing 4K still submits the canonical landscape-4k preset.
  • On a 1280x720 composition, the 1080p option is offered with its explanation but cannot be committed: highlighting it and pressing Enter leaves the resolution unchanged. Proved non-vacuous by dropping the disabled pass-through.
  • The FFmpeg gate cases are unchanged and still green.

Also run: the full Studio suite (440 files, 4841 tests, green, including the token gate and the hex ratchet), typecheck, the Studio build, oxlint, oxfmt --check, fallow audit --base origin/main --fail-on-issues (exit 0), and the design-shots capture before and after against this branch's base.

Bundle: the app's gzipped JS and CSS go from 1,573,539 to 1,593,184 bytes, +19,645 bytes (+19.2 KiB). This is the first consumer of Select in the shipped app, so it is where Base UI's select entry point lands. Nothing trimmed.

Not covered

  • The ratchet baseline is unchanged, and correctly so. Both touched files already held zero colour literals, so they are absent from the baseline and there is nothing to lower. The raw colours removed here (red-400, neutral-700, bg-red-500/10) are Tailwind palette classes, which the token gate governs, not the hex ratchet. The ratchet test fails on a fall as well as on a rise, and the suite is green, which is the evidence.
  • The thumbnail stays a plain <button>. It is an 80x45 media surface, not a control on the 24/28/32 grid, and IconButton would force it into a square.
  • FormatInfoTooltip stays a local popover. Its content is a heading plus three descriptions, which the one-line shared Tooltip does not carry; the file already says so.
  • FfmpegRequiredNotice.tsx is not in this unit's file list and is untouched. Its amber card still uses palette classes.
  • The inspector (U10) and the timeline (U11) are separate units.

Before

Origin main's current tip (f24ff83), on the repo's own swiss-grid example resized to 1280x720. Each image below is a combined before/after sheet (before on the left, after on the right).

Renders panel default state

After

This branch (16b6c82). Only #3772's own diff shows here: the four selects on the shared Select, the disabled-option label staying reachable by keyboard, and the panel's icon buttons. Export's box is identical on both sides (already on the shared Button primitive via #3622; this PR drops a redundant class with no visible effect).

Resolution control before and after

Export button idle and hovered

Measured computed values and the isolation note are in the capture README; this replaces an earlier capture pass I caught using a stale BEFORE commit.

The four native selects become the shared Select, so no OS popup is left in
the panel and the four controls read as the same control as everything
around them. Disabled options survive the move: SelectOption gains a
disabled flag, which is what keeps a resolution the composition cannot
reach out of the keyboard's path while its label still explains why.

Export drops its own type size. cn resolved that override by dropping the
size recipe's step, which is what left the panel Export a type step below
the header's; the computed-style table now reads 28px / 6px / 12px for
both. Its two per-row siblings become IconButton with a Tooltip carrying
what the title attribute used to say.

The panel's colours move to the semantic tokens.
@miguel-heygen
miguel-heygen force-pushed the feat/studio-u13-renders-sweep branch from 4fe3390 to 142a68c Compare September 19, 2026 20:28
@miguel-heygen
miguel-heygen changed the base branch from feat/studio-u5-primitives-inputs to main September 19, 2026 20:48
@miguel-heygen
miguel-heygen marked this pull request as ready for review September 19, 2026 21:39

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE. Three non-blocking findings below, all in the seam between this panel and the shared primitives rather than in the panel itself.

Most of this diff is provably a no-op, and the theme file is the proof

The panel-* → unprefixed migration is not a re-colouring at all — those tokens are aliases:

--color-panel-input: var(--color-input);      /* theme.css:137 */
--color-panel-hover: var(--color-hover);      /* :139 */
--color-panel-border: var(--color-border);    /* :140 */
--color-panel-text-0…5: var(--color-text-0…5);/* :143-148 */
--color-panel-accent: var(--color-accent);    /* :151 */
--color-panel-danger: var(--color-danger);    /* :152 */

Same for the type scale: --text-step-9/10/11 are 9px/10px/11px (:71-73), so every text-[10px]text-step-10 is exact. That settles the large majority of 305/181 without opening a capture.

Which makes the genuine deltas a short list, and worth having as the checklist for the capture pass — each is a real pixel change, not an alias:

site before after direction
error text, failed dot red-400 oklch(70.4%) danger = #ef4444 (red-500) deeper red
FormatInfoTooltip border neutral-700 oklch(37.1%) border-input #27272a darker
FormatInfoTooltip divider neutral-800 oklch(26.9%) border #1e1e1e darker
cancelled dot neutral-600 oklch(43.9%) text-5 #3f3f46 darker
FormatInfoTooltip shadow-lg shadow-menu deliberate

Also verified every new token resolves — text-0…5, surface, input, hover, border, border-input, accent, danger, shadow-menu are all defined. A token rename that lands on a name the theme doesn't define fails silently, so this was the thing worth checking.

1. The disabled Download's tooltip probably never appears

New copy, RenderQueueItem: label={isComplete ? "Download" : "Available when the render finishes"}, and the comment says disabled: on the shared Button "keeps pointer events alive precisely so a disabled one can still say why."

The mechanism doesn't hold up:

  • IconButton.tsx:28 passes the native disabled attribute to the <button>. Browsers suppress pointer events on disabled form controls outright — that suppression is not CSS, so not adding pointer-events-none (buttonBase:41 is only disabled:opacity-40 disabled:cursor-not-allowed) doesn't buy the hover back.
  • Tooltip.tsx:29 renders the trigger as <span className="contents">. display: contents generates no box, so the wrapper has no hit area of its own to catch what the disabled child won't emit. Wrapping a disabled control in a box-generating element is the usual fix precisely because contents isn't one — and the docblock states that as the intent ("so a disabled control still gets hover"), which is the reading I'd expect anyone to take on faith.

So the "why" copy is likely dead, and disabled controls get no focus either, so keyboard users don't reach it another way. Non-blocking: the old code had no title when incomplete either, so nothing regresses — this is a new affordance that may not exist rather than a broken one. And I could not run a browser, so what I can say precisely is that nothing asserts it: no test covers the tooltip on a disabled trigger. If you want it, the change is in Tooltip.tsx (give the trigger a real box, e.g. inline-flex), which is out of scope here.

2. "the keyboard skips it" — your own test says otherwise

refuses a resolution the composition cannot reach arrows down once from the default and hits Enter on a 1280×720 comp, then expects "auto". If the keyboard skipped the disabled 1080p it would have landed on 4K — a 3× integer scale of 1280×720, so enabled — and Enter would have committed landscape-4k. The sibling 4K test arrows two steps and does commit, so the sequence isn't inert.

So the real behaviour looks like the opposite of the comment: the highlight can land on the disabled option and Enter declines, leaving the prior value. Worth fixing the comment, because the next person changing scaleApplies will test against the wrong model.

One rider on that test: its expected value is the default, so that half would also pass if the interaction did nothing. The 4K test is what rules that out — fine as a pair, just not independently.

3. Delete lost its destructive hover

Old: text-panel-text-5 hover:text-red-400. New: IconButton at the default ghost variant, so the red-on-hover cue for the one destructive control in the row is gone. Deliberate design-system convergence is a fine answer; flagging it because it's the only affordance in the diff that changed rather than moved.

Credits worth naming

  • AE3expect(classes).toEqual(new Set([...shared.split(/\s+/), "w-full"])) is set equality, not contains, so it catches the extra-class-wins-the-merge bug by construction. That's the right instrument for a bug about cn resolution.
  • KTD13 — asserting both references are true before comparing trigger to reference. Two falses would have agreed and proven nothing; you closed that explicitly.
  • Dropping title from QUALITY_OPTIONS loses nothing: the old <option> render only ever emitted {q.label}, so those strings were already dead.
  • localStorage.clear() in beforeEach — necessary once the controls persist, and easy to omit.

Gate

Checked rather than taken on faith: CODEOWNERS covers only the three capture scripts, none of the four changed files; all five commits are attributed to miguel-heygen; no other reviews exist and reviewDecision was REVIEW_REQUIRED. So one approval does clear it, as you said.

— Rames

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-stamp at 51538421. The "content unchanged from 16b6c828" claim holds at the strongest level I can check, so my earlier review still describes exactly what merges.

Verified, one pass:

  • Blob SHAs identical for all four files at both heads — RenderQueue.test.tsx 7346628b, RenderQueue.tsx 3a404838, RenderQueueItem.tsx 72dab352, Select.tsx 0c7acf67. The bytes I read are the bytes shipping.
  • File set identical — same four, nothing added or dropped under the stamp.
  • Counts unchanged133+/25-, 99+/92-, 69+/64-, 4+/0-.
  • The base move brought nothing into this PR's surface. 51538421 is a merge of 16b6c828 with main at ac503920. Comparing the old merge base 9bec64a2 against ac503920 across all 41 files in components/ui/ and components/renders/: every blob identical. So main touched neither the files this PR edits nor the shared primitives it consumes, and the merge could not have reshaped the change.

Approving on code merit, as before. I did not use CI as a basis — for the record it is still running at this head.

— Rames

@miguel-heygen
miguel-heygen merged commit 12d8c42 into main Sep 19, 2026
50 checks passed
@miguel-heygen
miguel-heygen deleted the feat/studio-u13-renders-sweep branch September 19, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants