Review 5201

#5201 — feat(core): promote Stepper from Lab

ernestt · open · view on GitHub

Verdict: approve · 2 rounds

These records predate the Review Presentation format for the most part, so the problem/solution/API/theme-target/breaking slots are only filled where the review itself carried them. Reviews from 2026-08-23 on use the full template.

Round 1 — request-changes · 2026-08-19

Thanks for doing this properly — the move is clean and the evidence is thorough.

One thing I'd like fixed before this API is permanent: icon and indicator are the same axis and they collide silently. <Step icon={...} indicator="none" /> renders no indicator at all, and indicator="number" with an icon drops the number — no warning either way. indicator already takes a ReactNode, so I'd drop icon and keep one spelling. Once this is in core, removing either one is a breaking change for everyone who picked the wrong spelling, so I'd rather we only ship one.

Nothing catches a duplicate or skipped step index either: two steps at step={1} both get aria-current="step". Counting children is out (#4897) — what shape would you want, a dev warning or registering through context?

[Reviewed by Robohands]

Round 2 — approve · 2026-08-21

Thanks — one indicator axis and a dev warning on duplicate indices is exactly it.

Approving the promotion. One thing for next time: the connector animation is a separate change riding along on this one, and it's the piece most likely to attract debate. Promotion is what's blocking people — motion goes in its own PR on top, so the settled part can land without waiting on the part that isn't.

[Reviewed by Robohands]

Round 3 — approve · 2026-08-23

Head reviewed: 65b17f0e79f51f564b5a3189b5f2e7341922d47e

Round 1's duplicate-axis finding is genuinely resolved: Step.icon is gone from the interface, not just from the doc, and indicator?: StepIndicatorPreset | ReactNode = 'auto' carries the whole axis. Two things the docs commit left behind, neither blocking:

1. Two translation catalogs still document the removed `icon` prop
   → whoever authors a Step from the dense catalog or the zh docsite writes
     `<Step icon={…} />` and it does not compile; nothing checks the
     translations, so it cannot fail loudly    · Stepper.doc.mjs:222 (and :336)

2. `useStepperContext` is exported, `StepperContextValue` is not
   → the first person building a custom step against the context cannot
     annotate it; AvatarGroup, Table, InputGroup and CommandPalette all export
     theirs                                     · Stepper/index.ts:13

3. The two content blocks ship inputs nobody can type in
   → someone runs the Custom Content block, clicks "Project name" and the field
     is inert (`value=""`, no onChange)   · StepperCustomContent.tsx:36

Breaking, at this round: API — lab canary consumers only, the lab export is deleted and Step.icon stops compiling; matches the BottomSheet promotion contract. Visual — deliberate: 20→16px number badge, 24→16px description box, even horizontal track division, unbroken on-track connector. Theme — nothing breaks; the five targets keep their names and variant keys.

Performance: one mount-only effect per Step for index registration; previousActiveStep derived by render-phase setState rather than an effect; no listeners, no observers, no layout reads, no new dependency.

Risk class: needs judgement — new permanent core API surface. Behavior and performance clean.

Thanks — icon is properly gone from the interface, not just the doc. Two doc catalogs still list it (Stepper.doc.mjs:222 and :336), and StepperContextValue isn't exported next to useStepperContext. Worth fixing before merge; approving.

What changed before posting

Inlines were dropped to the summary — GitHub rejected the anchors as outside the diff hunks.