Skip to content

fix(Tabs): keep the tab selection controlled from the first render - #3048

Merged
mfal merged 3 commits into
mainfrom
fix/tabs-uncontrolled-to-controlled
Sep 4, 2026
Merged

fix(Tabs): keep the tab selection controlled from the first render#3048
mfal merged 3 commits into
mainfrom
fix/tabs-uncontrolled-to-controlled

Conversation

@mfal

@mfal mfal commented Sep 1, 2026

Copy link
Copy Markdown
Member

Part of #3014 — the A component changed from uncontrolled to controlled half. Split out of #3025, which carries the PressResponder half. Neither PR closes the issue on its own; close it once both are merged.

The warning (14×) — all of them Tabs

Not SegmentedControl, as the issue guessed: the two pages that looked like SegmentedControl cases both happen to contain a Tabs example, as does detail-page.

Tabs passed selectedKeyProps ?? selectedKeyState to Aria.Tabs. With neither selectedKey nor defaultSelectedKey given that is undefined on the first render, so Aria.Tabs starts uncontrolled. The tab titles reach Aria.TabList through a tunnel and commit one paint later, so react-aria's collection is still empty on render 1 and only picks the default tab in the next commit. That selection lands in selectedKeyState, the following render passes a defined key, and useControlledState warns.

Fix

?? null. useControlledState treats only undefined as uncontrolled, so null reads as "controlled, nothing selected yet" and the auto-selection behaves exactly as before. Aria.TabsProps narrows selectedKey to Key while the state hook behind it accepts null — one documented cast bridges that upstream type gap.

Verified

  • Reproduced first against the real stories with captureOwnerStack() to name the culprit.
  • Red-green: reverting ?? null fails the added test.
  • Behavioural assertions, not warning-absence: the first tab is selected on mount, and clicking a tab shows its panel.
  • Story sweep after the fix — Tabs/Default, Tabs/Collapsed, Tabs/TabNotFound, SegmentedControl/Default: zero occurrences of the warning.
  • No rendered output changes, so no visual baseline moves.

🤖 Generated with Claude Code

@mfal
mfal requested a review from a team September 1, 2026 09:29
@mfal mfal self-assigned this Sep 1, 2026
mfal added a commit that referenced this pull request Sep 1, 2026
The `uncontrolled to controlled` half of #3014 now lives in #3048. Nothing about
the overlay trigger changes here — this only takes `Tabs` back to main so the two
halves can be reviewed and released apart.
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for ./packages/components/

Status Category Percentage Covered / Total
🔵 Lines 78.69% 746 / 948
🔵 Statements 78.57% 763 / 971
🔵 Functions 80.09% 165 / 206
🔵 Branches 70.33% 377 / 536
File CoverageNo changed files found.
Generated in workflow #6675 for commit 269699c by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

Type URL
docs pr-3048.docs.review.flow-components.de
storybook pr-3048.storybook.review.flow-components.de

Images:

  • docs: ghcr.io/mittwald/flow/docs:pr-3048
  • storybook: ghcr.io/mittwald/flow/storybook:pr-3048

@mfal
mfal enabled auto-merge (squash) September 1, 2026 11:28
@mfal
mfal requested a review from Lisa18289 September 1, 2026 11:32
`Tabs` passed `selectedKeyProps ?? selectedKeyState` to `Aria.Tabs`. With
neither `selectedKey` nor `defaultSelectedKey` given that is `undefined` on the
first render, so `Aria.Tabs` starts uncontrolled. The tab titles reach
`Aria.TabList` through a tunnel and commit one paint later, so react-aria's
collection is still empty on render 1 and only picks the default tab in the next
commit. That selection lands in `selectedKeyState`, the following render passes
a defined key, and `useControlledState` warns "A component changed from
uncontrolled to controlled".

`?? null` instead: `useControlledState` treats only `undefined` as uncontrolled,
so `null` reads as "controlled, nothing selected yet" and the auto-selection
behaves exactly as before. `Aria.TabsProps` narrows `selectedKey` to `Key` while
the state hook behind it accepts `null` — one documented cast bridges that
upstream type gap.

Split out of #3025, which carries the second warning from the same issue.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal
mfal force-pushed the fix/tabs-uncontrolled-to-controlled branch from 75fa4ff to df1009f Compare September 2, 2026 11:53
mfal added a commit that referenced this pull request Sep 2, 2026
The `uncontrolled to controlled` half of #3014 now lives in #3048. Nothing about
the overlay trigger changes here — this only takes `Tabs` back to main so the two
halves can be reviewed and released apart.
mfal and others added 2 commits September 3, 2026 09:00
The rationale for `?? null` was a 14-line block comment in `Tabs.tsx`, and
#3078 carries the same react-aria fact again in
`useControlledHostValueProps`. Two copies in two files that are never read
together.

Move it to one bullet in the package's non-obvious conventions: react-aria's
`useControlledState` reads only `undefined` as uncontrolled, so a component
that mirrors the value in its own state has to pass a sentinel. The bullet
covers both sentinel shapes (`null` for "nothing selected yet", the type's
empty value otherwise), the `??` trap once the sentinel is `null`, and the
silent case (`CodeEditor` changes owner without warning).

The comment at the call site keeps what a reader needs there: which value
keeps it controlled, a pointer to the convention, and why the cast exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mfal added a commit that referenced this pull request Sep 4, 2026
The hook explained react-aria's controlled/uncontrolled rule in full, and
#3048 explained the same rule again in `Tabs.tsx`. That rationale now lives
in one bullet in the package's AGENTS.md (added by #3048), so both call sites
can stop carrying a copy.

What stays here is what a reader needs at this spot: what `emptyValue` is per
field type, and why the initialisation spells out `!== undefined` instead of
`??` — a caller-supplied `null` controls a `DateRangePicker` with no range
selected and must not fall through to `defaultValue`.

Comment-only; no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal
mfal merged commit dd29fa5 into main Sep 4, 2026
21 of 22 checks passed
@mfal
mfal deleted the fix/tabs-uncontrolled-to-controlled branch September 4, 2026 08:13
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