feat: fix residual md3 deviations in switch component - #5101
Closed
likevy wants to merge 5 commits into
Closed
Conversation
BREAKING CHANGE: `Switch` now requires `onValueChange`, `readOnly`, or `disabled`, and reserves the 48dp minimum touch target (40dp before). See the 6.x migration guide.
There was a problem hiding this comment.
🟡 Changes recommended
Native accessibility focus, runtime fallback semantics, and stale interaction states need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Aligns the MD3 Switch with accessibility, focus, color, touch-target, and operability requirements.
Changes:
- Adds explicit interactive, read-only, and disabled modes.
- Corrects focus styling, icon color, press animation, and touch-target sizing.
- Updates tests, examples, generated docs, and migration guidance.
File summaries
| File | Description |
|---|---|
src/index.tsx |
Exports narrowed Switch props. |
src/components/Switch/utils.ts |
Adds focus handle colors. |
src/components/Switch/tokens.ts |
Updates MD3 tokens and sizing. |
src/components/Switch/Switch.tsx |
Implements operability and visual changes. |
src/components/__tests__/Switch.test.tsx |
Tests focus, accessibility, and operability. |
src/components/__tests__/__snapshots__/Switch.test.tsx.snap |
Updates Switch snapshots. |
example/src/Examples/TextInputExample.tsx |
Marks nested Switches read-only. |
example/src/Examples/SwitchExample.tsx |
Adds a read-only example. |
example/src/Examples/FABExample.tsx |
Marks the nested Switch read-only. |
example/src/DrawerItems.tsx |
Marks drawer Switches read-only. |
docs/src/data/componentDocs6x.json |
Regenerates Switch API documentation. |
docs/6.x/docs/guides/migration.md |
Documents breaking migration steps. |
docs/6.x/docs/components/Switch/Switch.mdx |
Updates Switch props and theming docs. |
Review details
Suppressed comments (2)
src/components/Switch/Switch.tsx:189
- This fallback only removes interaction handlers; its paint and
aria-disabled/aria-readonlystill describe an enabled switch. An untyped<Switch value />therefore remains an enabled control that does nothing, which is the invalid state this runtime guard is intended to prevent. Either reject rendering or map the missing-operability case to a consistent disabled/read-only fallback.
const isMissingOperability = !onValueChange && !isReadOnly && !isDisabled;
if (isMissingOperability) {
console.warn(
'Switch: pass `onValueChange` to make the switch operable, or set `readOnly` or `disabled` to render it as a state indicator.'
src/components/Switch/Switch.tsx:421
- On Android,
focusable={false}also prevents accessibility focus, so read-only and disabled switches become undiscoverable by TalkBack instead of having their state announced. Restrict tab-order suppression to web, or otherwise separate keyboard focus from native accessibility focus.
focusable={isInteractive}
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ( | ||
| | { | ||
| onValueChange: (value: boolean) => void; | ||
| readOnly?: false; |
Comment on lines
+390
to
+394
| // A non-interactive switch gets no press, hover, or focus affordances at all. | ||
| // It stays in the accessibility tree, so its state is still announced. | ||
| const interactionProps = isInteractive | ||
| ? { | ||
| onPress: () => onValueChange?.(!checked), |
Comment on lines
+160
to
+161
| expect(control).toHaveProp('focusable', false); | ||
| expect(control).toBeChecked(); |
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.
Motivation
Switchis one of the MD3 reference components, but a review against the spec found small residual deviations, mostly accessibility gaps.The significant one is breaking: a switch must now declare how it can be operated —
onValueChange,readOnly, ordisabled.readOnlykeeps the enabled appearance and stays in the accessibility tree so its state is announced, and carriesaria-readonlyon web; it is not reported as disabled. Enforced by the props type plus a runtime guard for untyped callers.Two things worth mentioning:
Propsand the operability union sits on the component's declared type.SwitchPropsexports the narrowed type, so the public type matches what the component accepts. Commented in place so it isn't "simplified" later.Related issue
This closes the Switch review checklist:
primary; MD3 spec isonPrimaryContainer.role="switch"with noonValueChange(an enabled no-op); require or infer explicit operability (handler / read-only / disabled).Test plan
yarn lint,yarn typecheckandyarn testpass — 684 tests, 168 snapshots. Each commit is independently green, so the series bisects cleanly.9 new tests in
Switch.test.tsxcover the focus indicator, state layer and handle colour, the 52×48 touch target, and operability. The state-layer and handle-colour assertions fail if the focus fix is reverted (mutation-tested).Manual, on the Switch example screen:
onPrimaryContainer), not purple.readOnly, with the row owning the press.primaryContainerhandle; a mouse click must not light the ring. Read-only is skipped in the tab order and hasaria-readonly="true".Run on Android, iOS and web, plus a screen-reader pass. All behaved as described.