feat(studio): the inspector slider and select row on the shared primitives - #3785
Draft
miguel-heygen wants to merge 2 commits into
Conversation
…tives FlatSlider drops its hand-rolled pointer-capture state machine and FlatSelectRow its native select; both wrap the design system's Slider and Select, so the flat inspector has no bare underlined row left and no control of its own. What the wrapper still owns is the write rate. This row's onCommit is the live canvas preview, so the shared Slider's continuous onPreview is throttled to one write per frame rather than dropped; the commit boundary flushes it and fires telemetry exactly once per drag. Right-click and Escape still abort a drag in flight and put the value back. A platform pointercancel now does the same through the shared Slider, which had no handler for it. Adds a ratcheted check that no new native select or hand-rolled range input appears under components/editor.
The Weight row was a copy of FlatSelectRow written inline: the same native select, the same underline, the same caret. With the row it copied now boxed, the copy was the last bare underlined field in the inspector, which the sweep's screenshot is judged on. FlatSelectRow already keeps an authored weight outside the offered list representable, which is what the local includes-branch was for, and it already reports the same telemetry name. The select box sizes to its content between a floor and a ceiling now. At a fixed 96px the longest weight label truncated to "700 . Bo...".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands the second inspector sweep PR (U10b: the flat slider and select row) of the Studio design-system foundation. Stacked on #3774. Hex ratchet unchanged at 468 (neither file held a literal). Bundle −0.7 KB. Net −423 lines. Adds the R13 native-controls ratchet over components/editor so the remaining section families lower it to zero.
Lands the second inspector sweep PR (U10b) of the Studio design-system foundation. Stacked on the primitives sweep (U10a). The inspector's slider and select row lose their hand-rolled implementations, and the panel has no bare underlined field left.
What
The two controls U10a stopped at now come from the design system.
FlatSliderdrops a ~250-line pointer-capture state machine and wraps theshared
Slider. Pointer capture, keyboard, ARIA and the thumb are Base UI's.FlatSelectRowdrops its native<select>and wraps the sharedSelect, soits box is the same box
Input,NumberFieldand the inspector's metricfield wear, and its popup is Studio's menu surface rather than the operating
system's.
FlatSelectRowwritten inline(same native select, same underline, same caret). It is a
FlatSelectRownow.It was the last bare underlined field in the panel.
<select>or hand-rolledtype="range"appears undercomponents/editor, and fails again when alisted file is converted and not removed from the list, so it only shrinks.
Also fixes a gap in the shared
Sliderthis migration surfaced: it answeredright-click and Escape as drag aborts but had no handler for the platform's own
pointercancel(a scroll or touch takeover, a pen leaving range), so an abortedgesture left whatever value the pointer last reached applied. The control being
replaced reverted on all three.
Why
R8 wants one implementation per control and no native
<select>left. R10 wantsan inspector value to read as a field before it is clicked. The row that carried
"700 · Bold" did neither: it was an unstyled OS dropdown under a 1px line, beside
three siblings that already had a real box.
The slider is the R4 half. The hand-rolled one carried its own pointer capture,
its own
lostpointercaptureresync, its ownaria-valuenow, and its ownkeyboard map. All of that is behaviour a headless library already owns and gets
right, and none of it was shared with the other sliders in the app.
How
The wrapper keeps the write rate, and only that. The shared
SlidersplitsonPreview(every intermediate value) fromonCommit(a boundary). This row hasone channel: a caller's
onCommitwrites the style, and that write is the livecanvas preview. So
onPreviewis throttled to at most one write per frame ratherthan dropped, and the commit boundary flushes it. Dropping it instead would have
been the quiet regression in this PR: every test still passes, and the canvas
stops moving until you let go.
Telemetry keeps its contract:
onTrackfires from the commit boundary only, soone drag is one event no matter how many pixels it crossed.
The abort path goes through
onPreviewtoo. Base UI has no cancel API for arunning gesture, but it hands every change an
eventDetails.cancel(), so anaborted drag stops applying moves while the button is still down and the pre-drag
value is re-applied through the same channel the intermediate values used.
Tests. The ~30 tests pinned to the old DOM were sorted by what they assert.
Behaviour tests were re-expressed through the new DOM: the thumb's own
input[type="range"]for value, range and keyboard;[data-slider-control]forpointer geometry;
role="combobox"androle="option"for the select. Teststhat only described the old implementation were deleted, each named below.
Driving a listbox from a test is not a one-liner the way setting
select.valuewas, so the open-and-choose sequence lives in one helper (
flatSelectHarness.ts)that six section test files share, rather than being rewritten in each.
Test plan
Studio suite, from
packages/studio, with a capped worker pool:bunx vitest run --poolOptions.forks.maxForks=4— 441 files, 4843 passed,18 todo. Baseline at the branch point: 440 files, 4845 passed, 18 todo. One
file added (the duplicate-controls ratchet, 3 tests) and five deleted, listed
below.
bun run typecheck— clean.bun run --filter @hyperframes/studio build— succeeds.bunx oxlintandbunx oxfmt --checkon the changed files — clean.bunx fallow audit --fail-on-issuesagainst the base branch — exit 0,"No issues in 82 changed files".
nobody was holding —
PASS: studio loaded with schema-valid API fixtures and no runtime errors.Five tests deleted, with the reason each is no longer a claim about this code:
aria-valuenow.Slider's and lives with it.Three separate abort tests (right-click, Escape, pointercancel) became one
it.eachover the three, so the executed count is unchanged there.One test was rewritten because the swap made it vacuous rather than failing: the
style section asserted that two unset blur sliders drew no fill element, and that
element no longer exists on any slider, so the assertion would have passed
without checking anything. It asserts both blur thumbs read zero now.
Verified in a real browser, because a slider that renders as an invisible
zero-height track passes every test above. Studio booted from source on its own
port, the Style section opened, and the Layer blur slider dragged with a real
mouse: the track and thumb render at the expected geometry, and the canvas
headline blurs during the drag, which is the live preview the throttle exists
to keep.
That same run surfaced one thing worth flagging: the row's numeric readout stays
at its old value through a drag while the canvas updates. It is not a
regression from this change. The identical probe on the base branch produces
the identical result, because the readout has always come from the panel's
styles prop rather than the control's own draft. It is left alone here.
Screenshots, before and after, from the sweep's capture script. The computed
style table is identical in both runs — this PR moves no control the table
measures — and the difference is in the inspector shot: the "Weight" row goes
from bare text under a hairline to the same 28px/6px box as "Size", "Letter
spacing" and "Line height".
Colour literals in the ratchet: unchanged at 468. Neither migrated file had an
entry, so both were already at zero, and this PR does not add one.
Bundle, gzipped across the app's chunks: 1,595,377 bytes before, 1,594,630
after. A delta of -747 bytes. The two shared primitives were already in the
bundle from earlier units, so what this PR removes is larger than what it adds.
Not covered
and colour grading, canvas menu, fx and block params. Ten files still carry a
native
<select>and five a hand-rolledtype="range"; they are the ratchet'sstarting list and the later inspector PRs' work.
one drew a filled track and a brighter knob for an explicit value. The shared
Sliderhas one look. The row's value text still carries the tier tint andthe reset button still appears only for an explicit value, so the signal moved
rather than disappearing, but it is a deliberate reduction and not a
like-for-like port.
selector list covers the metric input, not the select, and adding a row for it
needs a selector that does not also match the font-family combobox beside it.
The trigger carries
fieldBaseand the tests assert that; the screenshot isthe visual evidence.
guidance, and was not split.
Splitting at the slider/select seam does not get under it: the slider's own
files are about 1,200 of those lines, because 1,017 of the total are deletions
of the implementation being replaced. The two halves also share the six test
files, so a slider-only PR would leave the panel with one bare underlined row
and one migrated row, which is the "only makes sense next to its sibling" case
a split is supposed to avoid.
the whole stack instead of this branch's own diff.