chore: upgrade to react native 81 and expo 54#6875
chore: upgrade to react native 81 and expo 54#6875
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR performs a comprehensive upgrade of the Android build toolchain, iOS project configuration, and project dependencies. Updates include Android SDK/build tools (35→36), Kotlin version, Gradle wrapper, React Native (0.79→0.81), Expo (53→54), React (18→19), iOS Xcode project configuration, and introduction of new build flags and Expo module integrations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…t.Chat.ReactNative into react-native-81
…t.Chat.ReactNative into react-native-81
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/containers/Button/index.tsx (1)
9-20:⚠️ Potential issue | 🟠 MajorPrevent
enabledfrom bypassingdisabled/loadingsafeguards.Because
IButtonPropsextendsRectButtonProps(line 9) and{...otherProps}is spread last (line 97), a caller can passenabled={true}to overrideenabled={!isDisabled}(line 93). This allows re-enabling presses whileloadingordisabledis true.Apply the proposed fix to exclude problematic props from the interface and ensure explicit props take precedence:
Proposed fix
-interface IButtonProps extends RectButtonProps { +interface IButtonProps extends Omit<RectButtonProps, 'enabled' | 'onPress' | 'style'> { title: string; onPress: () => void; type?: 'primary' | 'secondary'; @@ return ( <RectButton + {...otherProps} onPress={onPress} enabled={!isDisabled} style={containerStyle} accessibilityLabel={title} accessibilityRole='button' - {...otherProps}> + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/Button/index.tsx` around lines 9 - 20, The component allows callers to override the derived enabled state because IButtonProps extends RectButtonProps and {...otherProps} is spread after setting enabled, so callers can pass enabled to bypass disabled/loading; fix by excluding enabled from the inherited props and/or filtering it out before the spread: change the props type to extend Omit<RectButtonProps, 'enabled'> (i.e., IButtonProps extends Omit<RectButtonProps, 'enabled'>) or destructure enabled out of otherProps (const { enabled, ...rest } = otherProps) and spread rest instead of otherProps, and ensure the component always sets enabled={!isDisabled} (or similar) when rendering the RectButton so explicit props cannot override it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/containers/AudioPlayer/Seek.tsx`:
- Around line 76-80: The scale animation is being restarted on every gesture
update because withTiming(1.3, ...) is called inside the .onUpdate handler; move
the scale-up call into the gesture's .onStart handler so it runs once when the
gesture begins (e.g., call scale.value = withTiming(1.3, { duration: 150 }) in
.onStart), keep translateX.value and clamp logic inside .onUpdate using contextX
and maxWidth, and ensure you restore scale (e.g., scale.value = withTiming(1) or
appropriate reset) in .onEnd/.onFinalize so the knob returns to normal after the
gesture.
In `@app/containers/UIKit/Overflow.tsx`:
- Around line 44-57: The module-scoped touchable map (touchable) used in
Overflow leaks refs and can mis-anchor Popover when blockId repeats or is empty;
replace it with an instance-local ref inside the Overflow component (e.g.,
remove touchable and create const touchableRef = useRef<View | null>(null)
inside the Overflow function), attach that ref to the touchable element, and
remove all accesses to the global touchable[blockId] so the ref lifecycle is
tied to the component instance (update any code using touchableRef variable name
or blockId-dependent lookup to use the local touchableRef and, if needed, handle
blockId changes with useEffect).
In `@app/views/SecurityPrivacyView.tsx`:
- Around line 106-112: The "Send_crash_report" List.Item is using the wrong
accessibility label variable; replace the additionalAccessibilityLabel value
currently set to analyticsEventsState with crashReportState so VoiceOver reports
the correct enabled/disabled state for the crash report toggle. Update the
List.Item (title 'Send_crash_report') to use crashReportState for
additionalAccessibilityLabel and keep the existing Switch props
(value={crashReportState}, onValueChange={toggleCrashReport}, testID) unchanged.
---
Outside diff comments:
In `@app/containers/Button/index.tsx`:
- Around line 9-20: The component allows callers to override the derived enabled
state because IButtonProps extends RectButtonProps and {...otherProps} is spread
after setting enabled, so callers can pass enabled to bypass disabled/loading;
fix by excluding enabled from the inherited props and/or filtering it out before
the spread: change the props type to extend Omit<RectButtonProps, 'enabled'>
(i.e., IButtonProps extends Omit<RectButtonProps, 'enabled'>) or destructure
enabled out of otherProps (const { enabled, ...rest } = otherProps) and spread
rest instead of otherProps, and ensure the component always sets
enabled={!isDisabled} (or similar) when rendering the RectButton so explicit
props cannot override it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bddc7b3f-32f5-4a46-9548-0793d81b218b
⛔ Files ignored due to path filters (25)
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/DirectoryItem/__snapshots__/DirectoryItem.test.tsx.snapis excluded by!**/*.snapapp/containers/Header/components/HeaderButton/__snapshots__/HeaderButtons.test.tsx.snapis excluded by!**/*.snapapp/containers/InAppNotification/__snapshots__/NotifierComponent.test.tsx.snapis excluded by!**/*.snapapp/containers/List/__snapshots__/List.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/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snapapp/containers/SearchBox/__snapshots__/SearchBox.test.tsx.snapis excluded by!**/*.snapapp/containers/ServerItem/__snapshots__/ServerItem.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/markdown/__snapshots__/Markdown.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/CreateChannelView/RoomSettings/__snapshots__/SwitchItem.test.tsx.snapis excluded by!**/*.snapapp/views/DiscussionsView/__snapshots__/Item.test.tsx.snapis excluded by!**/*.snapapp/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.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!**/*.snap
📒 Files selected for processing (33)
.maestro/tests/accessibilityAndAppearance/ToastsAndDialogs.yml.maestro/tests/assorted/join-from-directory.yaml.maestro/tests/assorted/profile.yaml.maestro/tests/room/room-actions.yamlandroid/app/src/main/java/chat/rocket/reactnative/MainApplication.ktandroid/app/src/main/java/chat/rocket/reactnative/networking/SSLPinningTurboModule.javaapp/containers/ActionSheet/Handle.tsxapp/containers/AudioPlayer/Seek.tsxapp/containers/Button/index.tsxapp/containers/List/ListItem.tsxapp/containers/MessageComposer/components/RecordAudio/RecordAudio.tsxapp/containers/MessageComposer/hooks/useEmojiKeyboard.tsxapp/containers/RoomItem/Actions.tsxapp/containers/RoomItem/Touchable.tsxapp/containers/RoomItem/interfaces.tsapp/containers/UIKit/Overflow.tsxapp/lib/encryption/encryption.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/helpers/fileDownload.tsapp/lib/methods/helpers/fileUpload/Upload.android.tsapp/lib/methods/helpers/sslPinning.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/AttachmentView.tsxapp/views/CreateDiscussionView/index.tsxapp/views/DirectoryView/Options.tsxapp/views/RoomView/List/components/List.tsxapp/views/SecurityPrivacyView.tsxapp/views/ShareListView/index.tsxapp/views/ShareView/Thumbs.tsxapp/views/UserNotificationPreferencesView/index.tsxapp/views/UserPreferencesView/index.tsxbabel.config.jsios/NotificationService/Info.plist
✅ Files skipped from review due to trivial changes (10)
- app/containers/MessageComposer/components/RecordAudio/RecordAudio.tsx
- app/lib/encryption/encryption.ts
- app/views/AttachmentView.tsx
- app/lib/methods/helpers/fileDownload.ts
- .maestro/tests/accessibilityAndAppearance/ToastsAndDialogs.yml
- app/lib/methods/handleMediaDownload.ts
- app/containers/ActionSheet/Handle.tsx
- app/views/UserPreferencesView/index.tsx
- app/views/UserNotificationPreferencesView/index.tsx
- app/containers/RoomItem/interfaces.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- babel.config.js
- android/app/src/main/java/chat/rocket/reactnative/MainApplication.kt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E Run Android (12) / Android Tests
- GitHub Check: E2E Run Android (5) / Android Tests
- GitHub Check: E2E Run Android (11) / Android Tests
- GitHub Check: E2E Build iOS / ios-build
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:06.688Z
Learning: The RocketChat/Rocket.Chat.ReactNative repository uses a fork of react-native-image-crop-picker (RocketChat/react-native-image-crop-picker) with custom Android edge-to-edge fixes, not the upstream ivpusic/react-native-image-crop-picker package. Dependencies should reference commit pins from the RocketChat fork.
📚 Learning: 2026-03-17T19:15:26.536Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:26.536Z
Learning: In YAML test files under .maestro/tests/room, use tapping the empty area (e.g., tapOn: point: 5%,10%) to dismiss both the bottom sheet and keyboard when needed. Do not rely on action-sheet-handle alone if the keyboard also needs to be dismissed in the same step. This pattern is acceptable for tests where a single tap should close both UI elements.
Applied to files:
.maestro/tests/room/room-actions.yaml
📚 Learning: 2026-03-05T14:28:10.004Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6997
File: .maestro/tests/room/message-markdown-click.yaml:28-39
Timestamp: 2026-03-05T14:28:10.004Z
Learning: In Maestro YAML selector fields (text, id) within the Rocket.Chat React Native repository, use the contains pattern '.*keyword.*' (leading and trailing '.*') for matching text. The pattern '.*keyword*.' is incorrect and will fail to match cases where the keyword appears at the end of the element's text. This guideline applies to all Maestro YAML selector fields across the codebase.
Applied to files:
.maestro/tests/room/room-actions.yaml.maestro/tests/assorted/profile.yaml.maestro/tests/assorted/join-from-directory.yaml
📚 Learning: 2026-03-17T19:15:30.463Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
Applied to files:
.maestro/tests/assorted/profile.yaml
📚 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/views/CreateDiscussionView/index.tsxapp/containers/List/ListItem.tsx
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/containers/UIKit/Overflow.tsxapp/views/ShareListView/index.tsx
📚 Learning: 2026-03-04T20:13:17.288Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RocketChat Watch App/Views/MessageComposerView.swift:37-55
Timestamp: 2026-03-04T20:13:17.288Z
Learning: In the WatchOS app (ios/RocketChat Watch App) for Rocket.Chat React Native, using SwiftUI `Button` inside a `ScrollView` on WatchOS causes accidental message sends because button tap targets can be triggered during scroll gestures. `Text` with `.onTapGesture` is the preferred pattern for tappable items in scroll views on WatchOS. To preserve accessibility, add `.accessibilityAddTraits(.isButton)` and `.accessibilityLabel()` to the `Text` element instead.
Applied to files:
app/containers/List/ListItem.tsx
🔇 Additional comments (20)
app/containers/List/ListItem.tsx (2)
23-25: Good E2E/iOS gating strategy.This guard is consistent with the codebase pattern and keeps behavior narrowly scoped to iOS E2E runs.
177-177: Conditionalaccessibleusage looks correct.Switching to
accessible={!shouldDisableAccessibility}is a clean way to avoid iOS E2E accessibility interference without affecting normal app behavior.ios/NotificationService/Info.plist (2)
25-26: LGTM!The
IS_OFFICIALkey reordering is a cosmetic change with no functional impact.
36-37: LGTM — New Architecture flag aligns with RN 0.81 upgrade.Enabling
RCTNewArchEnabledis appropriate for React Native 0.81, where the New Architecture is the default. Adding this flag consistently across all iOS targets (main app, share extension, notification service) ensures uniform build configuration.Note: If the NotificationService extension is purely native Swift/ObjC code that doesn't use React Native directly, this flag may have no runtime effect but maintains consistency for any shared React Native dependencies.
.maestro/tests/room/room-actions.yaml (1)
124-127: Change fromextendedWaitUntiltoscrollUntilVisiblelooks appropriate.Using
scrollUntilVisibleafter the swipe gesture ensures the 'Star' option is brought into view, which is more robust if the action sheet content varies in length.However, similar action sheet interactions for 'Unstar' (lines 174-177) and 'Pin' (lines 229-232) still use
extendedWaitUntilafter the swipe. If the 'Star' option requires scrolling, the same may apply to these other actions depending on their position in the sheet.[approve_code_changes, request_verification]
#!/bin/bash # Description: Check for similar patterns in this file and other Maestro tests # to verify if other action sheet interactions should also use scrollUntilVisible. # Find all swipe followed by extendedWaitUntil patterns in this file echo "=== Patterns in room-actions.yaml ===" rg -n -A 5 'swipe:' .maestro/tests/room/room-actions.yaml # Check other Maestro tests for similar action-sheet patterns echo -e "\n=== Similar patterns in other Maestro test files ===" rg -n -B 2 -A 5 'action-sheet-handle' .maestro/tests/ --glob '*.yaml' | head -100.maestro/tests/assorted/profile.yaml (1)
99-100: LGTM!Good refactor to use the shared
hide-keyboard.yamlhelper instead of brittle text-matchingtapOncommands for dismissing the keyboard after username and nickname input. This improves maintainability and aligns with the project's pattern of centralizing common Maestro interactions.Also applies to: 115-116
.maestro/tests/assorted/join-from-directory.yaml (1)
100-103: Good move to stableidselectors for directory filters.Line 100/103 and Line 148/151 now target deterministic element ids, which should reduce Maestro flakiness across locales and UI state changes.
Also applies to: 148-151
app/views/DirectoryView/Options.tsx (2)
50-50:List.RadiotestID wiring is correct and matches E2E usage.Line 50 generates the ids used by the updated Maestro flow (
directory-switch-users/directory-switch-teams).
74-74: Nice addition of explicit testID on the global users switch.Line 74 improves automation targeting for this control and keeps behavior unchanged.
app/views/SecurityPrivacyView.tsx (1)
96-110: LGTM on testID relocation.Moving
testIDfromList.Itemto the nestedSwitchcomponent is consistent with the PR pattern. E2E tests targetingsecurity-privacy-view-analytics-eventsandsecurity-privacy-view-crash-reportwill now interact with the toggle controls directly.app/views/CreateDiscussionView/index.tsx (1)
172-177: Remove concern about E2E test relocation—testID is newly added, not moved.The
testID='room-actions-encrypt'appears only on the Switch component and does not exist elsewhere in the codebase. This is a new testID addition, not a relocation from the List.Item. Since no existing E2E or Maestro tests reference this testID, there are no tests to update.> Likely an incorrect or invalid review comment.app/containers/RoomItem/Actions.tsx (1)
12-12: LGTM —scheduleOnRNmigration is correct.The migration from
runOnJS(triggerHideAnimation)(value)toscheduleOnRN(triggerHideAnimation, value)correctly maintains the same behavior for scheduling JS-thread work from the worklet context. ThetoValueparameter is passed appropriately for both RTL and LTR swipe threshold transitions.Also applies to: 84-91
app/views/RoomView/List/components/List.tsx (1)
3-4: LGTM — Scroll handler migration is correct.The
scheduleOnRN(setVisible, boolean)pattern correctly schedules the React state update from the animated scroll handler worklet context.Also applies to: 26-34
app/containers/RoomItem/Touchable.tsx (1)
9-9: LGTM — Gesture callback migration is correct.Both long press and pan gesture end callbacks are properly migrated:
scheduleOnRN(handleLongPress)correctly handles the no-argument casescheduleOnRN(handleRelease, event)correctly passes the gesture event containingtranslationXneeded for the release logicAlso applies to: 173-189
app/containers/MessageComposer/hooks/useEmojiKeyboard.tsx (1)
4-6: LGTM — Emoji keyboard state synchronization is correctly migrated.The
useAnimatedReactioncallbacks properly schedule React state updates viascheduleOnRN(setState, value). The booleancurrentValueis passed correctly to synchronizeshowEmojiKeyboardandshowEmojiSearchbarstates.Also applies to: 159-159, 175-175
app/containers/AudioPlayer/Seek.tsx (2)
67-85: LGTM — Gesture API migration fromPanGestureHandlertoGesture.Pan()is correct.The migration correctly maps:
- Context capture via
contextXshared valueonUpdatefor translation updates with clampingonEndschedulingonChangeTimewith milliseconds (matching expo-av expectations)GestureDetectorwrapper replacingPanGestureHandlerAlso applies to: 125-127
12-12: ThescheduleOnRNAPI usage is compatible and correctly implemented.The function signature matches the expected pattern (function reference + arguments). However, be aware there are reported iOS crashes in audio worklet contexts (see react-native-worklets documentation). Test thoroughly on iOS devices to ensure no crashes occur when seeking completes in your audio player implementation.
app/containers/Button/index.tsx (1)
3-3: Import migration looks good.No concerns on this segment.
app/views/ShareListView/index.tsx (1)
104-104: The code is safe as written. The share extension saves all media files locally to the app's cache directory (iOS) or app group container (Android) and passes onlyfile://URIs to the React component. According to the Expo legacy FileSystem API documentation,getInfoAsync(uri)returns thesizefield by default forfile://URIs without requiring the{ size: true }option. The{ size: true }option is only required for remote URIs like MediaLibrary (ph://) URIs, which do not occur in this code path.> Likely an incorrect or invalid review comment.app/views/ShareView/Thumbs.tsx (1)
96-113: Nice simplification using sharedTouchwrapper.This removes platform branching in
Thumbwhile keeping the interaction flow intact in this component.
| .onUpdate(event => { | ||
| const newX = contextX.value + event.translationX; | ||
| translateX.value = clamp(newX, 0, maxWidth.value); | ||
| scale.value = withTiming(1.3, { duration: 150 }); | ||
| }) |
There was a problem hiding this comment.
Scale animation triggers repeatedly on every pan update.
The withTiming(1.3, ...) call inside .onUpdate() executes on every gesture update event (potentially 60+ times per second), restarting the 150ms animation each time. This is inefficient and may cause visual jitter.
Move the scale-up animation to .onStart() where it runs once when the gesture begins.
🔧 Proposed fix
.onStart(() => {
contextX.value = translateX.value;
+ scale.value = withTiming(1.3, { duration: 150 });
})
.onUpdate(event => {
const newX = contextX.value + event.translationX;
translateX.value = clamp(newX, 0, maxWidth.value);
- scale.value = withTiming(1.3, { duration: 150 });
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/containers/AudioPlayer/Seek.tsx` around lines 76 - 80, The scale
animation is being restarted on every gesture update because withTiming(1.3,
...) is called inside the .onUpdate handler; move the scale-up call into the
gesture's .onStart handler so it runs once when the gesture begins (e.g., call
scale.value = withTiming(1.3, { duration: 150 }) in .onStart), keep
translateX.value and clamp logic inside .onUpdate using contextX and maxWidth,
and ensure you restore scale (e.g., scale.value = withTiming(1) or appropriate
reset) in .onEnd/.onFinalize so the knob returns to normal after the gesture.
| const touchable: { [key: string]: React.RefObject<View | null> } = {}; | ||
|
|
||
| export const Overflow = ({ element, loading, action, parser }: IOverflow) => { | ||
| const { theme } = useTheme(); | ||
| const options = element?.options || []; | ||
| const blockId = element?.blockId || ''; | ||
| const [show, onShow] = useState(false); | ||
|
|
||
| if (!touchable[blockId]) { | ||
| touchable[blockId] = React.createRef(); | ||
| } | ||
|
|
||
| const touchableRef = touchable[blockId] as React.RefObject<any>; | ||
|
|
There was a problem hiding this comment.
Avoid module-scoped ref registry for anchors.
The global touchable map can leak refs over time and can collide when blockId repeats/empties, which may anchor Popover to the wrong element. Prefer an instance-local ref.
💡 Suggested fix
-import React, { useState } from 'react';
-import { FlatList, StyleSheet, Text, type View } from 'react-native';
+import React, { useRef, useState } from 'react';
+import { FlatList, StyleSheet, Text } from 'react-native';
@@
-const touchable: { [key: string]: React.RefObject<View | null> } = {};
-
export const Overflow = ({ element, loading, action, parser }: IOverflow) => {
const { theme } = useTheme();
const options = element?.options || [];
- const blockId = element?.blockId || '';
const [show, onShow] = useState(false);
-
- if (!touchable[blockId]) {
- touchable[blockId] = React.createRef();
- }
-
- const touchableRef = touchable[blockId] as React.RefObject<any>;
+ const touchableRef = useRef<React.ElementRef<typeof Touch>>(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/containers/UIKit/Overflow.tsx` around lines 44 - 57, The module-scoped
touchable map (touchable) used in Overflow leaks refs and can mis-anchor Popover
when blockId repeats or is empty; replace it with an instance-local ref inside
the Overflow component (e.g., remove touchable and create const touchableRef =
useRef<View | null>(null) inside the Overflow function), attach that ref to the
touchable element, and remove all accesses to the global touchable[blockId] so
the ref lifecycle is tied to the component instance (update any code using
touchableRef variable name or blockId-dependent lookup to use the local
touchableRef and, if needed, handle blockId changes with useEffect).
| <List.Item | ||
| title='Send_crash_report' | ||
| testID='security-privacy-view-crash-report' | ||
| right={() => <Switch value={crashReportState} onValueChange={toggleCrashReport} />} | ||
| right={() => ( | ||
| <Switch value={crashReportState} onValueChange={toggleCrashReport} testID='security-privacy-view-crash-report' /> | ||
| )} | ||
| additionalAccessibilityLabel={analyticsEventsState} | ||
| /> |
There was a problem hiding this comment.
Pre-existing bug: wrong accessibility label for crash report toggle.
Line 111 uses analyticsEventsState for the "Send_crash_report" item's additionalAccessibilityLabel, but it should use crashReportState. This causes VoiceOver to announce the wrong enabled/disabled state for this toggle.
Since you're modifying adjacent code, consider fixing this while you're here.
🐛 Proposed fix
<List.Item
title='Send_crash_report'
right={() => (
<Switch value={crashReportState} onValueChange={toggleCrashReport} testID='security-privacy-view-crash-report' />
)}
- additionalAccessibilityLabel={analyticsEventsState}
+ additionalAccessibilityLabel={crashReportState}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <List.Item | |
| title='Send_crash_report' | |
| testID='security-privacy-view-crash-report' | |
| right={() => <Switch value={crashReportState} onValueChange={toggleCrashReport} />} | |
| right={() => ( | |
| <Switch value={crashReportState} onValueChange={toggleCrashReport} testID='security-privacy-view-crash-report' /> | |
| )} | |
| additionalAccessibilityLabel={analyticsEventsState} | |
| /> | |
| <List.Item | |
| title='Send_crash_report' | |
| right={() => ( | |
| <Switch value={crashReportState} onValueChange={toggleCrashReport} testID='security-privacy-view-crash-report' /> | |
| )} | |
| additionalAccessibilityLabel={crashReportState} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/views/SecurityPrivacyView.tsx` around lines 106 - 112, The
"Send_crash_report" List.Item is using the wrong accessibility label variable;
replace the additionalAccessibilityLabel value currently set to
analyticsEventsState with crashReportState so VoiceOver reports the correct
enabled/disabled state for the crash report toggle. Update the List.Item (title
'Send_crash_report') to use crashReportState for additionalAccessibilityLabel
and keep the existing Switch props (value={crashReportState},
onValueChange={toggleCrashReport}, testID) unchanged.
Proposed changes
Depends on:
Merge Plan
Pending:
Update typescript to version 5.8.3
RemoveUIDesignRequiresCompatibilityfrom info.plistIssue(s)
https://rocketchat.atlassian.net/browse/CORE-1578
Closes #6874
Closes #6803
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Chores
New Features
Tests