Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Clickable component in the react-native-gesture-handler library, intended to serve as a unified replacement for BaseButton, RectButton, and BorderlessButton. The component supports configurable visual feedback (underlay or whole-component opacity changes) and native Android ripple effects. Supporting changes include extracting visual style properties (background color, border radius, etc.) through to the native button in GestureHandlerButton, type improvements to RawButton and BorderlessButton, and a new example screen.
Changes:
- New
Clickablecomponent with configurable animation feedback (feedbackTarget,feedbackType,activeOpacity,underlayColor) and long-press support GestureHandlerButtonnow passes visual style properties (backgroundColor, borderRadius, border styles) to the nativeButtonComponent, andRawButtongets proper type generics- Example screen demonstrating
Clickablein various configurations (base, rect-like, borderless-like, custom)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/v3/components/Clickable.tsx |
New Clickable component with animation, long-press, and ripple support |
src/v3/components/index.ts |
Exports Clickable and ClickableProps (from incorrect path) |
src/v3/index.ts |
Re-exports Clickable from components |
src/components/GestureHandlerButton.tsx |
Extracts visual properties (backgroundColor, border*) into buttonStyle passed to native button |
src/v3/components/GestureButtons.tsx |
Adds type generics to RawButton, extracts ref in BorderlessButton |
src/v3/types/NativeWrapperType.ts |
Adds ` |
apps/common-app/src/new_api/components/clickable/index.tsx |
Example screen showcasing Clickable configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/react-native-gesture-handler/src/v3/components/index.ts
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/components/Clickable/Clickable.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/react-native-gesture-handler/src/v3/components/Clickable/ClickableProps.ts
Show resolved
Hide resolved
| left, | ||
| start, | ||
| end, | ||
| overflow, | ||
| }), |
There was a problem hiding this comment.
overflow was previously included in layoutStyle and passed to the ButtonComponent. It is now removed from layoutStyle but not added to buttonStyle, so it is no longer forwarded to the native button at all. It is only used for the wrapper View's conditional clipping logic (line 254). If this is intentional (the native button shouldn't get overflow), this is fine. But if it was previously needed by the native button, removing it here is a regression.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/react-native-gesture-handler/src/v3/components/Clickable/Clickable.tsx
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/components/Clickable/ClickableProps.ts
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/components/Clickable/Clickable.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/react-native-gesture-handler/src/v3/components/Clickable/Clickable.tsx
Outdated
Show resolved
Hide resolved
apps/common-app/src/new_api/components/clickable_stress/index.tsx
Outdated
Show resolved
Hide resolved
packages/react-native-gesture-handler/src/v3/components/Clickable/Clickable.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export const RawButton = createNativeWrapper(GestureHandlerButton, { | ||
| export const RawButton = createNativeWrapper< | ||
| ReturnType<typeof GestureHandlerButton>, |
| }); | ||
|
|
||
| /** | ||
| * @deprecated `RectButton` is deprecated, use `Clickable` with `underlayInitialOpacity={0.7}` instead |
| onPressIn?.(e); | ||
|
|
||
| if (e.pointerInside) { | ||
| startLongPressTimer(); | ||
|
|
| onPressOut?.(e); | ||
|
|
||
| if (longPressTimeout.current !== undefined) { | ||
| clearTimeout(longPressTimeout.current); | ||
| longPressTimeout.current = undefined; | ||
| } |
| // Unmount then remount for next run | ||
| setState({ phase: 'idle' }); | ||
| setTimeout(() => { | ||
| setState({ phase: 'running', run: currentRun + 1 }); | ||
| }, 50); | ||
| }, []); |
| onPressIn={() => console.log(`[${name}] onPressIn`)} | ||
| onPress={() => console.log(`[${name}] onPress`)} | ||
| onLongPress={() => console.log(`[${name}] onLongPress`)} | ||
| onPressOut={() => console.log(`[${name}] onPressOut`)} |
| KINDA_BLUE: '#5f97c8', | ||
| ANDROID: '#34a853', | ||
| WEB: '#1067c4', | ||
| }; |
There was a problem hiding this comment.
Please use the common colours, if needed add new colours there.
| @@ -105,6 +107,8 @@ export const NEW_EXAMPLES: ExamplesSection[] = [ | |||
| { name: 'FlatList example', component: FlatListExample }, | |||
| { name: 'ScrollView example', component: ScrollViewExample }, | |||
| { name: 'Buttons example', component: ButtonsExample }, | |||
There was a problem hiding this comment.
If we want to remove the buttons from v3 ButtonsExample should be removed
| <ClickableWrapper | ||
| name="Rect" | ||
| color={COLORS.WEB} | ||
| underlayActiveOpacity={0.105} |
There was a problem hiding this comment.
As far as I see we set all the config values manually, didn't we discuss adding some presets which would mimick old behaviour? Manually set values would of course take precedence. I think it would simplify transition. Robots may know what to plug as the values anyway, but ordinary people will have to do some research before they get the same behaviour when migrating.
There was a problem hiding this comment.
Should the buttons be included in v3 at all?
There was a problem hiding this comment.
Yes, so you don't need to change your code after upgrading just to get the app running.
| /** | ||
| * Background color of underlay. Works only when `animationTarget` is set to `UNDERLAY`. | ||
| */ |
| const { | ||
| underlayColor, | ||
| underlayInitialOpacity, | ||
| underlayActiveOpacity, | ||
| initialOpacity, | ||
| activeOpacity, | ||
| androidRipple, | ||
| delayLongPress = 600, | ||
| onLongPress, | ||
| onPress, | ||
| onPressIn, | ||
| onPressOut, | ||
| onActiveStateChange, | ||
| style, | ||
| children, | ||
| ref, | ||
| ...rest | ||
| } = props; | ||
|
|
||
| const animatedValue = useRef(new Animated.Value(0)).current; | ||
|
|
||
| const underlayStartOpacity = underlayInitialOpacity ?? 0; | ||
| const componentStartOpacity = initialOpacity ?? 1; |
There was a problem hiding this comment.
| const { | |
| underlayColor, | |
| underlayInitialOpacity, | |
| underlayActiveOpacity, | |
| initialOpacity, | |
| activeOpacity, | |
| androidRipple, | |
| delayLongPress = 600, | |
| onLongPress, | |
| onPress, | |
| onPressIn, | |
| onPressOut, | |
| onActiveStateChange, | |
| style, | |
| children, | |
| ref, | |
| ...rest | |
| } = props; | |
| const animatedValue = useRef(new Animated.Value(0)).current; | |
| const underlayStartOpacity = underlayInitialOpacity ?? 0; | |
| const componentStartOpacity = initialOpacity ?? 1; | |
| const { | |
| underlayColor, | |
| underlayInitialOpacity = 0, | |
| underlayActiveOpacity, | |
| initialOpacity = 1, | |
| activeOpacity, | |
| androidRipple, | |
| delayLongPress = 600, | |
| onLongPress, | |
| onPress, | |
| onPressIn, | |
| onPressOut, | |
| onActiveStateChange, | |
| style, | |
| children, | |
| ref, | |
| ...rest | |
| } = props; | |
| const animatedValue = useRef(new Animated.Value(0)).current; | |
| borderRadius: resolvedStyle.borderRadius, | ||
| borderTopLeftRadius: resolvedStyle.borderTopLeftRadius, | ||
| borderTopRightRadius: resolvedStyle.borderTopRightRadius, | ||
| borderBottomLeftRadius: resolvedStyle.borderBottomLeftRadius, | ||
| borderBottomRightRadius: resolvedStyle.borderBottomRightRadius, |
There was a problem hiding this comment.
Is this needed? button content should be clipped, no?
Description
This PR introduces new
Clickablecomponent, which is meant to be a replacement for buttons.Note
Docs for
Clickablewill be added in #4022, as I don't want to release them right away after merging this PR.borderlessFor now,
borderlessdoesn't work. I've tested clickable with some changes that allowborderlessripple to be visible, however we don't want to introduce them here because it would break other things. Also it should be generl fix, not in the PR with new component.Stress test
Render list with 2000 buttons 50 times (50ms delay between renders), drop 5 best and 5 worst results. Then calculate average.
On android tests were limited to 25 repetitions with dropout of 3 best and worst results. This is because of OOM error.
Stress test example is available in this PR.
Android
BaseButtonRectButtonBorderlessButtoniOS
BaseButtonRectButtonBorderlessButtonWeb
BaseButtonRectButtonBorderlessButtonTest plan
New examples