feat(core): let a flow drop the Stepper's collapsed controls and label - #5935
Open
ernestt wants to merge 3 commits into
Open
feat(core): let a flow drop the Stepper's collapsed controls and label#5935ernestt wants to merge 3 commits into
ernestt wants to merge 3 commits into
Conversation
Feedback on the narrow-container collapse: on a phone the summary row often repeats the page around it. A wizard that brings its own Back/Continue gets two pairs of controls, and one that heads its own step gets the step named twice. The collapse is right to exist; it just had no way to yield the half a given flow already covers. Stepper now takes `hasCollapsedControls` and `hasCollapsedLabel`, both defaulting to true, so a stepper that sets neither behaves exactly as it did before. Dropping one leaves the other in the row; dropping both removes the row rather than leaving an empty one to spend the frame's gap. `Step` needed no change — it already gates its summary portal on the slot existing, so withholding the slot is the whole implementation of `hasCollapsedLabel`. The names were arbitrated rather than picked. Four naming arms times four prompts, isolated agents, arm docs machine-generated from one template so no arm got a lexical head start, plus a five-sample recall probe that described the behavior with the props deliberately unnamed. On the generation battery the arms tied at 16/16 correct, so the naming does not change whether an agent can use the API. The probe is what separated them: 5/5 reached for two independent booleans and nobody produced an enum, which settles the shape; a bare `hasLabel` collides with the existing `label` prop that names the whole sequence; and `Summary` only ever won by leaking an internal name off a data attribute, while `Collapsed` was the unprompted first instinct. The probe preferred `show*` 5/5, but packages/core/src has ~30 `has*` booleans and zero `show*`, and the 16/16 tie is the evidence that taking the house convention costs nothing for a prop that is always documented. Harness and full findings in internal/vibe-tests/stepper-collapse-naming-test. Both props are purely visual. The row is aria-hidden and the <ol> keeps every step's name at any width, so neither can shorten what a screen reader hears. One sharp edge is documented rather than designed away, and it was the test's largest finding: no agent in any arm could tell whether a collapsed track is still tappable. In the `separated` layout the step targets go with the labels, so turning the controls off leaves nothing to press until the stepper is wide again. That is the intent when the flow has its own controls, but the JSDoc now names the consequence and the `on-track` exception rather than leaving it to be discovered. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ernestt
marked this pull request as ready for review
September 3, 2026 23:04
ernestt
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
September 3, 2026 23:04
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.
Note
Stacked on #5659. Base is
core-stepper-narrow-collapse, so this diff is just the two new props. Review #5659 first; this can retarget tomainonce that lands.Why
Feedback on #5659: the collapsed summary row often repeats the page around it on a phone. A wizard that brings its own Back/Continue gets two pairs of controls, and one that heads its own step gets the step named twice. The collapse is right to exist — it just had no way to yield the half a given flow already covers.
What
Two independent booleans on
Stepper, both@default true:hasCollapsedControlshasCollapsedLabelDefaults preserve today's behavior exactly. Dropping one leaves the other in the row; dropping both removes the row entirely rather than leaving an empty one to spend the frame's gap.
Stepneeded no change — it already gates its summary portal on the slot existing, so withholding the slot is the entire implementation ofhasCollapsedLabel.Accessibility
Both props are purely visual. The summary row is
aria-hiddenand the<ol>keeps every step's name at any width, so neither prop can shorten what a screen reader hears. Asserted directly by a test that reads back the full sequence with both halves off.The one sharp edge
Turning the controls off in the
separatedlayout leaves a collapsed stepper with nothing to press — that layout gives up its step click targets along with its labels. That is the intent when the flow has its own controls, but it does mean the stepper is purely a progress indicator until it is wide again.on-trackkeeps its indicators pressable as nodes on the rail, so there it costs nothing. Named in thehasCollapsedControlsJSDoc and the prop table rather than left to be discovered.How the names were chosen
Arbitrated with a vibe test rather than picked; harness and full findings in
internal/vibe-tests/stepper-collapse-naming-test. Four naming arms × four prompts with isolated agents, arm docs machine-generated from one template so no arm got a lexical head start, plus a five-sample recall probe that described the behavior with the props deliberately unnamed.hasLabelis out: it collides with the existinglabelprop, which names the whole sequence. A real collision in the shipping API, not an artifact of the test.Summaryis out: it only ever won by leaking an internal name off a data attribute, and bothSummarychoosers started atCollapsedand switched for that reason. The judge also found that B and C results stopped to verify the prop would not kill the desktop labels 4/4, while A and D never did 0/4 — theCollapsedqualifier removes real scope doubt.hasovershow: the probe preferredshow*5/5, butpackages/core/srchas ~30has*booleans and zeroshow*. The 16/16 tie is the evidence that taking the house convention costs nothing for a prop that is always documented.Docs
Updated in all three
Stepper.doc.mjsvariants (full,--dense,--zh): both props in the prop tables, a new Collapsed summary anatomy row soastryx-stepper-summaryis no longer a theme target for a part the docs never mention, and a corrected best practice — the old "use the vertical orientation for narrow containers" now contradicted the component, and would still send an agent toorientation="vertical"on mobile.Also picked up pre-existing drift while in the same tables:
indicatorPositionwas missing from both the dense and Chinese prop tables.Test plan
Six new tests in
Stepper.test.tsx:on-tracknodes pressable after the controls are droppedLocal verification:
Stepper.test.tsx+docs-drift— 65 passeddocPropReferences+docPropLiterals— 22 passed, confirming the documented props match the real signatureastryx component Stepperrendered in all three variants; eslint and prettier cleanKnown gap, pre-existing and left alone:
docsZhhas noanatomyblock at all, so the new anatomy row does not reach Chinese readers. Closing that means translating all five entries and felt like a separate change.