chore: migrate TouchableOpacity to Pressable #6870
Conversation
WalkthroughReplaces platform-specific touchable wrappers across the app with a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
while migrating i noticed In <View style={styles.addServerButtonContainer}>
<Button
title={I18n.t('Add_Server')}
type='primary' // expected to be blue
onPress={addServer}
testID='rooms-list-header-server-add'
style={styles.buttonCreateWorkspace}
color={colors.buttonFontSecondary}
backgroundColor={colors.buttonBackgroundSecondaryDefault} // explicitly being set as secondary
/>
</View>Rocket.Chat.ReactNative/app/views/RoomsListView/components/ServersList.tsx Lines 143 to 153 in 3315fb4 |
…back with Pressable
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
app/views/ScreenLockedView.tsx (1)
78-80: Add Android ripple feedback for better UX.The close button uses
disableOpacityOnAndroidbut doesn't provide anandroid_rippleconfiguration. This means Android users get no visual feedback when pressing the button. Consider adding ripple feedback for a better user experience.🔎 Suggested enhancement
-<PressableOpacity onPress={onCancel} style={styles.close} disableOpacityOnAndroid disableOpeningMessageModal> +<PressableOpacity + onPress={onCancel} + style={styles.close} + android_ripple={{ color: colors.surfaceNeutralPress }} + disableOpacityOnAndroid + disableOpeningMessageModal> <CustomIcon name='close' size={30} /> </PressableOpacity>Note: You'll need to destructure
colorsfromuseTheme()at the component level.app/containers/UIKit/Overflow.tsx (1)
66-82: Overflow component migration looks good; consider simplifying style prop.The PressableOpacity migration is correct. However, the style prop wraps a single style object in an array (
style={[styles.menu]}), which is unnecessary for a single style.🔎 Optional simplification for style prop
- style={[styles.menu]}> + style={styles.menu}>app/containers/AudioPlayer/Seek.tsx (2)
108-110: Update the outdated comment.The comment still references
TouchableNativeFeedback, but the code now usesPressableOpacity.🔎 Proposed fix
- // TouchableNativeFeedback is avoiding do a long press message when seeking the audio + // PressableOpacity is avoiding a long press message when seeking the audio return ( <PressableOpacity style={{ flex: 1 }} disableAndroidRipple>
110-110: Consider extracting the inline style object.The inline style object
{{ flex: 1 }}is recreated on every render. While the performance impact is negligible here, extracting it to the StyleSheet or a constant would follow React best practices.🔎 Proposed refactor
Add to the styles object:
const styles = StyleSheet.create({ + seekWrapper: { + flex: 1 + }, seekContainer: {Then use it:
- <PressableOpacity style={{ flex: 1 }} disableAndroidRipple> + <PressableOpacity style={styles.seekWrapper} disableAndroidRipple>app/views/DiscussionsView/Item.tsx (1)
69-69: Consider extracting the inline style object (optional).The inline style object
{{ backgroundColor: colors.surfaceRoom }}is recreated on every render. For a minor optimization, consider using a memoized style or conditional style from the StyleSheet.Note: This is a minor optimization and may not be necessary if colors don't change frequently. The current implementation is functionally correct.
app/containers/Avatar/Avatar.tsx (1)
100-106: Inconsistent Android ripple configuration compared to other components.This Avatar component uses
disableAndroidRippleto completely disable the ripple effect, while other migrated components (Reply, ThreadMessagesView Item, IncomingCallNotification buttons) configureandroid_ripplewith theme colors.If this is intentional (e.g., avatars shouldn't have ripple feedback), it's fine. Otherwise, consider adding ripple configuration for consistency:
💡 Optional: Add ripple configuration for consistency
<PressableOpacity accessible={accessible} accessibilityLabel={avatarAccessibilityLabel} - disableAndroidRipple + android_ripple={{ color: themes[theme].surfaceNeutral }} + disableOpacityOnAndroid onPress={onPress}> {image} </PressableOpacity>Note: This would require importing
themesanduseThemehook.app/containers/AudioPlayer/PlaybackSpeed.tsx (1)
22-31: LGTM! View wrapper correctly contains ripple effects.The
overflow: 'hidden'wrapper View is the correct pattern to ensure Android ripple effects respect the rounded corners of the PressableOpacity component.Optional: Extract inline style to styles file
For consistency with the rest of the codebase, consider moving the inline style to the styles file:
In
styles.ts:+containerWrapper: { + overflow: 'hidden' +}In
PlaybackSpeed.tsx:-<View style={{ overflow: 'hidden' }}> +<View style={styles.containerWrapper}>app/containers/message/Message.tsx (1)
220-229: Remove redundant commented code.Line 228 contains a commented-out assignment that would restore the default
rippleColorvalue, which is already set on line 221. This commented code serves no purpose and should be removed for code cleanliness.🔎 Proposed fix
if (props.highlighted) { backgroundColor = colors.surfaceNeutral; - // rippleColor = colors.surfaceNeutralPress; }app/containers/message/Components/Attachments/Image/Button.tsx (1)
18-28: LGTM! Consider clarifying the commented foreground property.The migration is correct. The ripple uses
colors.surfaceNeutral(instead ofsurfaceNeutralPressused elsewhere), which likely provides a subtler effect appropriate for image attachments.Optional: Clarify commented foreground property
Consider making the comment clearer about why
foreground: trueis commented out:android_ripple={{ color: colors.surfaceNeutral - // foreground: true // if the effect should be applied above the image + // foreground: true // Disabled: ripple should appear behind the image }}Or remove the comment entirely if the foreground option is not relevant for this use case.
app/containers/PressableOpacity.tsx (5)
12-12: Replacereact-native-animatableView withreact-nativeView.The View imported from
react-native-animatableis used only as a plain container (line 118) without any animation features. Import View fromreact-nativeinstead to reduce bundle size.🔎 Proposed fix
-import { View } from 'react-native-animatable'; +import { View } from 'react-native';
122-125: Verify null assignment behavior for Pressable props.The handlers and
android_rippleare explicitly set tonullwhen disabled. While this is valid React Native syntax, verify thatnullcorrectly disables these features rather than causing unexpected behavior. Consider using conditional spreading instead:{...(!shouldBlockOpacityAnimationOnAndroid && { onPressIn: handlePressIn, onPressOut: handlePressOut })} {...(!disableAndroidRipple && { android_ripple: androidRippleConfig })}
109-112: Consider conditional long press handler definition.The
handleLongPressis always defined even when bothonLongPressPropEventandonLongPressFromContextare undefined. Consider defining it conditionally to avoid unnecessary function allocation:const handleLongPress = (onLongPressPropEvent || onLongPressFromContext) ? (e: GestureResponderEvent) => { onLongPressPropEvent?.(e); onLongPressFromContext?.(e); } : undefined;
136-142: Document limitation with function-style styles.The
extractStylesfunction returnsundefinedfor function-based styles (line 138), which means the ripple containment fix won't apply when styles are dynamic. While this is a reasonable limitation, consider adding a comment to document this behavior for future maintainers.
54-132: Comprehensive testing recommended for this large migration.This component replaces TouchableOpacity across 40+ files with new behavior (Android ripple, conditional opacity, MessageContext integration, and complex style processing). While the PR description mentions local testing passed, ensure:
- Visual regression testing on both Android and iOS
- Testing with various style configurations (shadows, borders, positioning, overflow)
- Testing long-press behavior with and without MessageContext
- Testing ripple appearance on Android with different themes
The style manipulation logic is particularly fragile and could break in edge cases.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (16)
app/containers/Avatar/__snapshots__/Avatar.test.tsx.snapis excluded by!**/*.snapapp/containers/Button/__snapshots__/Button.test.tsx.snapis excluded by!**/*.snapapp/containers/Chip/__snapshots__/Chip.test.tsx.snapis excluded by!**/*.snapapp/containers/LoginServices/__snapshots__/LoginServices.test.tsx.snapis excluded by!**/*.snapapp/containers/MessageComposer/__snapshots__/MessageComposer.test.tsx.snapis excluded by!**/*.snapapp/containers/ReactionsList/__snapshots__/ReactionsList.test.tsx.snapis excluded by!**/*.snapapp/containers/RoomHeader/__snapshots__/RoomHeader.test.tsx.snapis excluded by!**/*.snapapp/containers/TextInput/__snapshots__/TextInput.test.tsx.snapis excluded by!**/*.snapapp/containers/UIKit/__snapshots__/UiKitMessage.test.tsx.snapis excluded by!**/*.snapapp/containers/UIKit/__snapshots__/UiKitModal.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/CannedResponsesListView/__snapshots__/CannedResponseItem.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapapp/views/RoomView/LoadMore/__snapshots__/LoadMore.test.tsx.snapis excluded by!**/*.snapapp/views/ThreadMessagesView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (49)
app/containers/AudioPlayer/PlayButton.tsxapp/containers/AudioPlayer/PlaybackSpeed.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Avatar/Avatar.tsxapp/containers/Button/index.tsxapp/containers/CallHeader.tsxapp/containers/InAppNotification/IncomingCallNotification/index.tsxapp/containers/InAppNotification/NotifierComponent.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/NativeButton/index.tsxapp/containers/PressableOpacity.tsxapp/containers/ReactionsList/ReactionsList.test.tsxapp/containers/RoomHeader/RoomHeader.tsxapp/containers/TabView/index.tsxapp/containers/TextInput/FormTextInput.tsxapp/containers/ThreadDetails.tsxapp/containers/UIKit/DatePicker.tsxapp/containers/UIKit/MultiSelect/Chips.tsxapp/containers/UIKit/MultiSelect/Input.tsxapp/containers/UIKit/MultiSelect/Items.tsxapp/containers/UIKit/Overflow.tsxapp/containers/UIKit/VideoConferenceBlock/components/VideoConferenceEnded.tsxapp/containers/UIKit/VideoConferenceBlock/components/VideoConferenceOutgoing.tsxapp/containers/message/Broadcast.tsxapp/containers/message/CallButton.tsxapp/containers/message/Components/Attachments/CollapsibleQuote/index.tsxapp/containers/message/Components/Attachments/Image/Button.tsxapp/containers/message/Components/Attachments/Reply.tsxapp/containers/message/Components/Attachments/Video.tsxapp/containers/message/Components/RightIcons/Encrypted.tsxapp/containers/message/Components/RightIcons/MessageError.tsxapp/containers/message/Discussion.tsxapp/containers/message/Message.tsxapp/containers/message/Reactions.tsxapp/containers/message/Touchable.tsxapp/containers/message/Urls.tsxapp/containers/message/User.tsxapp/containers/message/styles.tsapp/lib/constants/colors.tsapp/views/CannedResponsesListView/CannedResponseItem.tsxapp/views/ChangePasscodeView.tsxapp/views/DiscussionsView/Item.tsxapp/views/RoomView/UploadProgress.tsxapp/views/RoomsListView/components/Header.tsxapp/views/RoomsListView/components/ServersList.tsxapp/views/ScreenLockedView.tsxapp/views/ShareView/Thumbs.tsxapp/views/ThreadMessagesView/Item.tsxpackage.json
💤 Files with no reviewable changes (3)
- package.json
- app/containers/message/Touchable.tsx
- app/containers/NativeButton/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/UIKit/MultiSelect/Chips.tsxapp/containers/UIKit/MultiSelect/Input.tsxapp/containers/CallHeader.tsxapp/containers/InAppNotification/IncomingCallNotification/index.tsxapp/containers/AudioPlayer/PlayButton.tsxapp/views/RoomView/UploadProgress.tsxapp/containers/UIKit/MultiSelect/Items.tsx
🧬 Code graph analysis (22)
app/containers/message/Discussion.tsx (2)
app/lib/constants/colors.ts (1)
colors(286-308)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/UIKit/DatePicker.tsx (1)
app/lib/constants/colors.ts (1)
themes(310-310)
app/containers/Button/index.tsx (3)
app/containers/PressableOpacity.tsx (1)
IPressableOpacityProps(18-46)app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/message/Components/RightIcons/MessageError.tsx (2)
app/lib/constants/colors.ts (1)
themes(310-310)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/PressableOpacity.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/message/Components/Attachments/Reply.tsx (1)
app/lib/constants/colors.ts (1)
themes(310-310)
app/containers/UIKit/MultiSelect/Input.tsx (1)
app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/CallHeader.tsx (2)
app/lib/constants/colors.ts (1)
colors(286-308)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
app/containers/List/List.stories.tsx (1)
Pressable(43-51)
app/views/ThreadMessagesView/Item.tsx (1)
app/lib/constants/colors.ts (1)
themes(310-310)
app/containers/UIKit/Overflow.tsx (2)
app/lib/constants/colors.ts (1)
themes(310-310)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/AudioPlayer/PlaybackSpeed.tsx (1)
app/lib/constants/colors.ts (1)
colors(286-308)
app/views/ShareView/Thumbs.tsx (1)
app/lib/constants/colors.ts (1)
themes(310-310)
app/containers/InAppNotification/IncomingCallNotification/index.tsx (4)
app/theme.tsx (1)
useTheme(29-29)app/lib/methods/helpers/notifications.ts (1)
hideNotification(3-3)app/lib/constants/colors.ts (1)
colors(286-308)app/actions/videoConf.ts (2)
cancelCall(62-67)acceptCall(69-74)
app/containers/message/Reactions.tsx (3)
app/lib/constants/colors.ts (1)
themes(310-310)app/containers/message/utils.ts (1)
BUTTON_HIT_SLOP(25-30)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/message/CallButton.tsx (2)
app/lib/constants/colors.ts (1)
themes(310-310)app/containers/CustomIcon/index.tsx (1)
CustomIcon(47-47)
app/containers/ReactionsList/ReactionsList.test.tsx (1)
app/containers/PressableOpacity.tsx (1)
IPressableOpacityProps(18-46)
app/containers/message/Message.tsx (1)
app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/AudioPlayer/PlayButton.tsx (2)
app/lib/constants/colors.ts (1)
colors(286-308)app/containers/AudioPlayer/constants.ts (1)
AUDIO_BUTTON_HIT_SLOP(3-3)
app/containers/UIKit/MultiSelect/Items.tsx (2)
app/containers/UIKit/utils.ts (1)
textParser(8-8)app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/message/Components/Attachments/Image/Button.tsx (1)
app/lib/constants/colors.ts (1)
colors(286-308)
app/containers/InAppNotification/NotifierComponent.tsx (1)
app/lib/methods/helpers/notifications.ts (1)
hideNotification(3-3)
🔇 Additional comments (56)
app/views/RoomsListView/components/ServersList.tsx (1)
144-150: Good fix! Button type now matches its intended styling.This change correctly addresses the configuration mismatch noted in the PR comments, where the "Add Server" button was configured as
type='primary'but styled with secondary colors. The button type now aligns with its visual appearance.app/containers/message/Components/Attachments/Video.tsx (1)
106-115: LGTM! Clean migration to PressableOpacity with proper Android ripple configuration.The migration from
TouchabletoPressableOpacityis implemented correctly:
- Android ripple configured with appropriate color and foreground effect
disableOpacityOnAndroidproperly applied- Original
onPressbehavior preserved via_onPress- Consistent with the migration pattern across the PR
app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
1-1: Consider using PressableOpacity for consistency with other PR components.This file imports
Pressablefromreact-native-gesture-handler, while other components in this PR appear to migrate to the customPressableOpacitycomponent. If this is part of a broader refactoring pattern, this component at lines 73-84 should align with that migration, including theandroid_rippleconfiguration used elsewhere.app/views/RoomsListView/components/Header.tsx (1)
69-83: LGTM!The migration to
PressableOpacitypreserves all functionality correctly. UsingdisableAndroidRipplefor the header button is appropriate as it maintains a clean, non-distracting appearance for navigation UI.app/containers/message/Discussion.tsx (1)
32-44: LGTM!The migration correctly preserves all button functionality, including
hitSlopfor better touch targets. The use ofcolors.buttonBackgroundPrimaryPressfor the Android ripple is appropriate for this action button.app/containers/TextInput/FormTextInput.tsx (2)
197-204: LGTM!The clear input button migration correctly preserves the
onPresshandler andtestID, while adding appropriate Android ripple feedback withcolors.surfaceSelected.
220-232: LGTM!The password visibility toggle migration correctly preserves all functionality, including accessibility labels and the
onPresshandler. The Android ripple withcolors.surfaceSelectedprovides appropriate visual feedback.app/lib/constants/colors.ts (1)
9-9: LGTM! Press-state color tokens added consistently across themes.The new
surfaceNeutralPressandstatusBackgroundWarning2Pressproperties are added consistently to all three theme definitions (light, dark, and black) with appropriate theme-specific values. These align with the broader migration to PressableOpacity components with themed Android ripple effects.Also applies to: 44-44, 103-103, 138-138, 197-197, 232-232
app/containers/UIKit/Overflow.tsx (1)
30-40: LGTM! Option component migrated to PressableOpacity correctly.The migration from Touchable to PressableOpacity is implemented correctly with proper Android ripple configuration and custom props. The themed ripple color (
surfaceNeutral) provides visual feedback consistent with the design system.app/containers/UIKit/MultiSelect/Input.tsx (1)
24-42: LGTM! Input component migrated to PressableOpacity correctly.The migration from Touchable to PressableOpacity is implemented correctly with:
- Proper Android ripple configuration using themed colors
- Custom props for opacity and modal behavior
- Preserved
disabledprop handling- Unchanged layout and styling
app/containers/ReactionsList/ReactionsList.test.tsx (1)
10-10: LGTM! Test mock updated correctly for PressableOpacity migration.The TabView mock has been properly updated to use PressableOpacity instead of TouchableOpacity, with the correct type imports and props (
disableAndroidRippleanddisableOpeningMessageModal). The test behavior is preserved with thejumpTonavigation logic remaining unchanged.Also applies to: 65-88
app/views/RoomView/UploadProgress.tsx (1)
218-222: LGTM! Try Again button migrated to PressableOpacity correctly.The migration from TouchableOpacity to PressableOpacity is straightforward and correct. The
disableAndroidRippleprop is appropriate for this text-only button to maintain a clean visual appearance, and the existing onPress behavior and A11y structure are preserved.app/containers/UIKit/DatePicker.tsx (2)
16-16: LGTM! Clean import of the new component.The import follows the established project structure and supports the migration to PressableOpacity.
65-91: Migration is correct; theme properties are available.The migration to PressableOpacity is well-structured and preserves all existing behavior. The android_ripple configuration and disableOpacityOnAndroid prop correctly provide Android-specific feedback. The theme properties
surfaceNeutralandsurfaceRoomare properly defined across all theme variants.app/containers/message/User.tsx (2)
2-4: LGTM! Clean migration of imports.The import changes correctly replace TouchableOpacity with the new PressableOpacity component.
120-129: LGTM! Username header migration preserves all functionality.The migration correctly replaces TouchableOpacity with PressableOpacity while maintaining the testID, styling, and navigation behavior. The use of
disableAndroidRippleis appropriate for this subtle header interaction.app/containers/message/Components/RightIcons/MessageError.tsx (2)
9-9: LGTM! Import addition supports the migration.
23-30: Error icon wrapper correctly configured.The PressableOpacity component is properly migrated with appropriate touch target sizing via hitSlop and correct Android ripple color using the existing
surfaceNeutralPresstheme property. All functionality is preserved.app/containers/message/CallButton.tsx (2)
11-11: LGTM! Import supports the migration.
20-32: Call button migration is correct;buttonBackgroundPrimaryPressis properly defined in the theme.All functionality is preserved, including the hitSlop for touch accessibility and the Android ripple effect with a valid theme color reference.
app/containers/TabView/index.tsx (2)
7-7: LGTM! Import addition supports tab migration.
31-33: LGTM! Tab navigation migration preserves functionality.The migration correctly replaces TouchableOpacity with PressableOpacity while maintaining the tab switching behavior. The use of
disableAndroidRippleis appropriate since tabs rely on color changes and the underline indicator for visual feedback rather than ripple effects. ThehitSlopof 10 improves the touch target accessibility.app/containers/RoomHeader/RoomHeader.tsx (1)
14-14: LGTM!The migration from TouchableOpacity to PressableOpacity is correctly implemented. The
disableAndroidRippleprop appropriately prevents ripple effects on the room header press, maintaining a clean UX. All original props and accessibility attributes are preserved.Also applies to: 204-225
app/containers/UIKit/VideoConferenceBlock/components/VideoConferenceOutgoing.tsx (1)
9-10: LGTM!The migration to PressableOpacity with theming support is well-implemented. The android_ripple configuration with
colors.buttonBackgroundPrimaryPressprovides appropriate visual feedback for the primary action button, anddisableOpacityOnAndroidensures ripple is the primary interaction cue on Android.Also applies to: 14-14, 17-24
app/containers/message/Broadcast.tsx (1)
12-12: LGTM!The migration to PressableOpacity is correct, and the developer's note on ripple color choice (using
buttonBackgroundPrimaryPressfor better tint matching) demonstrates thoughtful attention to visual consistency.Also applies to: 25-39
app/containers/UIKit/VideoConferenceBlock/components/VideoConferenceEnded.tsx (1)
12-12: LGTM!The migration to PressableOpacity is correctly implemented. The component appropriately uses default ripple behavior without custom colors, and the
disableOpeningMessageModalflag prevents modal conflicts during call-back actions.Also applies to: 35-43
app/containers/UIKit/MultiSelect/Items.tsx (1)
13-13: LGTM!The migration to PressableOpacity is correctly implemented with appropriate ripple color (
surfaceSelected) for selection UI. The accessibility implementation is excellent—including the selection state in theaccessibilityLabelensures clear state announcement on iOS VoiceOver, following best practices for checkbox components.Also applies to: 37-62
app/containers/ThreadDetails.tsx (1)
90-95: LGTM! Clean migration with good accessibility.The migration to PressableOpacity is correct. The accessibilityLabel provides good screen reader support, and the disableOpacityOnAndroid prop ensures consistent behavior across platforms.
app/containers/message/Components/Attachments/CollapsibleQuote/index.tsx (1)
175-177: Verify the ripple color access pattern.This file uses
themes[theme].surfaceNeutralfor the android_ripple color, while other files in this PR (DiscussionsView/Item.tsx, Urls.tsx) usecolors.surfaceNeutralfrom theuseTheme()hook. Both patterns access the theme, but the inconsistency may indicate a mistake.Please verify which pattern is correct for this codebase:
themes[theme].surfaceNeutral(direct theme access)colors.surfaceNeutral(from useTheme hook)If
colorsis the preferred pattern, consider updating this usage for consistency:🔎 Suggested change for consistency
android_ripple={{ - color: themes[theme].surfaceNeutral + color: colors.surfaceNeutral }}app/views/DiscussionsView/Item.tsx (1)
66-87: LGTM! Consistent migration pattern.The migration to PressableOpacity follows the established pattern with proper android_ripple configuration and disableOpacityOnAndroid. The testID ensures testability.
app/containers/message/Urls.tsx (2)
168-190: LGTM! Proper handling of press and long-press interactions.The migration correctly preserves both
onPress(opens link) andonLongPress(copies to clipboard) behaviors. The android_ripple configuration and conditional styling based on content presence are well implemented.
185-185: No action needed. ThedisableOpeningMessageModalprop is properly documented with a JSDoc comment explaining its purpose ("disable the modal when item is long pressed"), correctly declared in theIPressableOpacityPropsinterface as an optional boolean, and actively implemented in the component logic to conditionally disable the MessageContext's onLongPress handler. The prop is consistently used across the codebase.app/views/ThreadMessagesView/Item.tsx (1)
12-12: LGTM! Consistent migration pattern.The migration correctly replaces Touchable with PressableOpacity while preserving all functionality. The inline style with
backgroundColorand theandroid_rippleconfiguration follow the established pattern from other migrated components.Also applies to: 77-99
app/containers/InAppNotification/IncomingCallNotification/index.tsx (2)
18-19: LGTM! Proper theme integration.The addition of
useThemehook and extraction ofcolorsenables type-safe access to theme-specific ripple colors for the three action buttons.Also applies to: 46-46
88-101: Semantic ripple colors correctly applied for each action type.Each button uses an appropriately themed ripple color:
- Dismiss:
buttonBackgroundSecondaryPress(neutral action)- Decline:
buttonBackgroundDangerPress(destructive action)- Accept:
buttonBackgroundSuccessPress(positive action)All accessibility props,
hitSlopconfiguration, anddisableOpacityOnAndroidare consistently maintained across all three buttons.app/containers/AudioPlayer/PlayButton.tsx (1)
10-10: Migration successfully completed with proper color token configuration.The migration from NativeButton to PressableOpacity is well-executed:
- All props correctly preserved (accessible, accessibilityLabel, style, disabled, onPress, hitSlop)
- Android ripple configured with existing
buttonBackgroundPrimaryPresscolor token- Dynamic accessibility label maintained through
getPlayButtonAccessibilityLabelfunctiondisableOpacityOnAndroidappropriately used with ripple effectapp/containers/message/Components/Attachments/Reply.tsx (1)
19-19: LGTM! Clean migration to PressableOpacity.The migration correctly preserves all props (testID, onPress, style, disabled) while adding Android-specific behavior. The
android_rippleanddisableOpacityOnAndroidpattern is consistent with the broader migration approach. ThesurfaceNeutralcolor value is properly defined in all theme variants.app/containers/InAppNotification/NotifierComponent.tsx (2)
15-15: LGTM! Clean migration to PressableOpacity.The migration correctly preserves all interaction props and uses borderless ripple for the notification container, maintaining the expected touch feedback behavior.
Also applies to: 112-131
132-139: LGTM! Close button correctly migrated.The close action properly uses PressableOpacity with borderless ripple, maintaining consistent touch feedback with the notification container.
app/views/CannedResponsesListView/CannedResponseItem.tsx (2)
9-9: LGTM! Wrapper correctly migrated to PressableOpacity.The container properly uses themed ripple color and disables opacity animation on Android to avoid conflicting with the native ripple effect.
Also applies to: 30-36
47-60: Verify the Button styling changes.The Button styling has been significantly modified beyond the touchable migration:
- Padding explicitly set to 0 to prevent text overflow (per comment)
- Added
smallprop- Changed to
type='secondary'- Added explicit alignment and centering
These changes appear intentional but should be verified against the provided screenshots to ensure the visual appearance matches expectations.
app/views/ShareView/Thumbs.tsx (1)
3-3: LGTM! Excellent unification of platform-specific touchable logic.The migration successfully replaces the multi-platform ThumbButton wrapper with PressableOpacity, using:
dimOpacity: 0.7for iOS/default opacity animationandroid_ripplewith reduced opacity color for Android-native feedbackforeground: truefor proper ripple layeringThis simplifies the code while maintaining platform-appropriate touch feedback.
Also applies to: 11-11, 96-123
app/containers/message/Reactions.tsx (2)
12-12: LGTM! AddReaction correctly migrated.The add reaction button properly uses PressableOpacity with themed ripple color and disabled opacity animation on Android, maintaining consistent touch feedback.
Also applies to: 38-51
63-88: LGTM! Reaction correctly migrated with context-aware props.The reaction button properly uses PressableOpacity with the
disableOpeningMessageModalprop, which prevents the message modal from opening during reaction interactions. This is appropriate given the component handles both press and long-press events for reaction-specific actions.app/containers/CallHeader.tsx (2)
33-50: LGTM! Color handling correctly extended.The
handleColorsfunction properly returnsbuttonRipplecolors for all states, supporting the new Android ripple configuration while maintaining existing button and icon color logic.
62-73: LGTM! Camera button correctly migrated.The camera button properly uses PressableOpacity with consistent state handling across background color, ripple color, and icon color. The disabled state and accessibility label are correctly preserved.
app/containers/Button/index.tsx (2)
70-78: LGTM! Well-structured ripple color logic.The implementation correctly:
- Provides theme-based default ripple colors for primary/secondary button types
- Allows customization via
android_ripple.colorprop- Preserves other
android_rippleproperties when merging
97-108: The @ts-ignore suppression is appropriate and does not require fixes.The type mismatch for
containerStyleoccurs because it's an array mixing multiple style types (including conditionals and a heterogeneous style prop that can beStyleProp<ViewStyle> | StyleProp<ViewStyle>[]). TypeScript cannot properly infer this complex nested union structure againstPressableProps['style']. However, React Native's Pressable genuinely accepts arrays of style objects at runtime, making this a false positive from TypeScript's type checker. The@ts-ignorepragmatically acknowledges this limitation while preserving correct runtime behavior.app/containers/message/Components/RightIcons/Encrypted.tsx (1)
22-29: LGTM! Clean migration to PressableOpacity.The migration correctly:
- Preserves all existing functionality (
onPress,hitSlop,style)- Adds theme-based Android ripple effect
- Uses
disableOpacityOnAndroidfor native-like behaviorapp/containers/message/Message.tsx (1)
242-256: ThedisableOpeningMessageModalprop is necessary to prevent duplicate long-press handler execution.The MessageContext's
onLongPressis passed both as a prop to PressableOpacity and exists in MessageContext. WithoutdisableOpeningMessageModal=true, PressableOpacity would invoke the same handler twice via different paths: once through theonLongPressprop and again through MessageContext. Setting this prop to true disables the context-based call, ensuring the handler executes only once.However, the inline comment "redunant call to openActionSheet" is inaccurate. The issue is not redundancy in the traditional sense but rather preventing duplicate execution of the same handler. Consider updating the comment to clarify:
// Prevent calling the same onLongPress handler twice via contextor similar.app/containers/PressableOpacity.tsx (6)
18-46: LGTM! Well-designed interface.The interface is well-documented and provides appropriate extension points for customization while maintaining compatibility with the base
PressableProps.
48-52: LGTM! Appropriate animation defaults.The animated component creation and default timing values are appropriate for TouchableOpacity-like behavior.
1-1: Justify or remove the immutability eslint disable.The
react-hooks/immutabilityrule is disabled at the file level, but it's unclear which code violates immutability. If this is for theopacity.valuemutations in the event handlers (lines 99, 104), consider documenting why this is necessary for react-native-reanimated shared values.
180-180: No action needed. Theoverflow: 'hidden'default on line 180 is an intentional architectural decision required for Android ripple effect clipping according to the code comments on lines 88 and 117. The implementation correctly preserves custom overflow values via the nullish coalescing operator (flattenedStyle.overflow ?? 'hidden'), allowing components to explicitly passoverflow: 'visible'in the style prop if needed. No evidence of breaking changes in migrated components.Likely an incorrect or invalid review comment.
200-203: Position reset to 'relative' is intentional but restricts absolute positioning use cases.The position reset on line 201 is part of PressableOpacity's two-layer architecture (outer View + inner AnimatedPressable) designed to constrain the ripple effect within the container's border radius. When any position value exists in the style, the inner Pressable receives
position: 'relative'and positioning properties (top, left, right, bottom) set to 0, preventing it from being absolutely positioned.This design choice means PressableOpacity cannot be used as an absolutely positioned element—positioning must be applied to its parent container instead. No current usages in the codebase pass
position: 'absolute'directly to PressableOpacity, so migrated components are not affected. However, document this constraint if it differs from the original component's behavior.
88-90: Verify impact of width/height exclusion from container on all migrated usages.Width and height are commented out from
containerStyle(lines 166-167), keeping them on the innerPressablerather than the outerViewwrapper. This is intentional to prevent Android ripple overflow. However, this could affect layouts that rely on the outer container having explicit dimensions. Verify that all 40+ migrated files render correctly with dimensions applied only to the inner Pressable element, especially components with fixed or constrained widths/heights.
1 participant
Proposed changes
TouchableOpacityto more modern componentPressablereact-native-platform-touchablePressableOpacity(which is wrapper around Pressable and have similar opacity Animation as TouchableOpacity when clicked but more coustomizable)NativeButtonandTouchablethat relies on Touchable from thereact-native-platform-touchableIssue(s)
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Screenshots
Summary by CodeRabbit
Improved Features
Breaking Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.