feat: modernize Switch to MD3#4957
Conversation
|
The mobile version of example app from this branch is ready! You can see it here. |
adrcotfas
force-pushed
the
@adrcotfas/tokens_structure
branch
from
91d86b4 to
be1c417
Compare
adrcotfas
force-pushed
the
@adrcotfas/tokens_structure
branch
from
be1c417 to
b41006a
Compare
adrcotfas
force-pushed
the
@adrcotfas/switch
branch
from
562f947 to
7731d0e
Compare
adrcotfas
force-pushed
the
@adrcotfas/tokens_structure
branch
from
b41006a to
f675baf
Compare
adrcotfas
force-pushed
the
@adrcotfas/switch
branch
from
7731d0e to
9e41a0b
Compare
|
@satya164 Findings were fixed. I discovered an issue with the dynamic theme when testing custom switch colors and applied a fix as a separate commit. |
satya164
left a comment
satya164
left a comment
There was a problem hiding this comment.
Quickly clicking on the Switch feels bad, probably because the Switch grows and changes position unanimated before the animation is visible
CleanShot.2026-05-26.at.17.00.57-optimised.mp4
RTL seems to be broken on Web
CleanShot.2026-05-26.at.16.39.44-optimised.mp4
I've merged some updates to main which should let you toggle RTL on Web for testing
| const [hovered, setHovered] = React.useState(false); | ||
| const [pressed, setPressed] = React.useState(false); | ||
| const [focused, setFocused] = React.useState(false); |
There was a problem hiding this comment.
make these shared values. ideally, we should have event -> animation rather than event -> state update -> effect -> animation. user interactions should be as responsive as possible, even if JS thread is busy.
| <SwitchFocusRing | ||
| focused={focused} | ||
| style={[ | ||
| styles.focusRing, | ||
| { | ||
| borderColor: colors.focusIndicatorColor, | ||
| borderWidth: FOCUS_THICKNESS, | ||
| top: OVERLAY_TOP + FOCUS_RING_INSET, | ||
| left: FOCUS_RING_INSET, | ||
| right: FOCUS_RING_INSET, | ||
| bottom: OVERLAY_TOP + FOCUS_RING_INSET, | ||
| borderRadius: cornerFull, | ||
| }, | ||
| ]} | ||
| /> |
There was a problem hiding this comment.
i'd prefer to do this with CSS instead of react state. it'd be a relatively similar amount of code to write these styles inside <style>, and even less code with a reusable utility. is there any benefit to doing this with react state?
we also need to hide the outline from the default styles, as both of them are visible:
| const handleFocus: PressableProps['onFocus'] = (e) => { | ||
| if (document.activeElement?.matches(':focus-visible')) { | ||
| onFocus?.(e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
it seems a bit too much abstraction for this small component. the web code could be just a conditional with Platform.OS === 'web' check. in the same component instead of 2 platform specific files, and for such small logic, the whole thing could be inlined and it'd be less code.
| @@ -0,0 +1,11 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
don't use .web extension. metro supports .native by default. web bundlers don't support .web by default. even though most configs configure it, it also becomes problematic when more tools come into picture such as importing on node (e.g. for SSR).
use .native extension whenever a native/web split is needed.
| style: StyleProp<ViewStyle>; | ||
| }) { | ||
| if (!focused) return null; | ||
| return <View pointerEvents="none" style={style} />; |
There was a problem hiding this comment.
all of the styling logic is in Switch, so a separate component for just a View with pointerEvents="none" seems a bit unnecessary abstraction.
also pointerEvents prop warns on web. check the console warnings. it needs to be astyle.
2 participants
Motivation
Update the Switch to use the latest MD specs, including motion and RTL.
Related issue
Closes #4939
Partially closes #4892
Test plan
Demo
Screen_recording_20260520_171630.webm
Simulator.Screen.Recording.-.iPhone.16e.-.2026-05-20.at.17.46.36.mov