feat(a11y): Physical Keyboard Navigation#7010
Open
OtavioStasiak wants to merge 52 commits intodevelopfrom
Open
feat(a11y): Physical Keyboard Navigation#7010OtavioStasiak wants to merge 52 commits intodevelopfrom
OtavioStasiak wants to merge 52 commits intodevelopfrom
Conversation
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Android keyboard accessibility for inverted message lists: a new KeyboardA11y TurboModule, FocusUtils, native inverted scroll/content views with exit-focus handling and keyboard navigation, MainActivity key remapping when enabled, and React changes to expose/refocus header, composer, and wire native props. Changes
Sequence DiagramsequenceDiagram
actor User
participant MainActivity
participant KeyboardA11yModule
participant FocusUtils
participant InvertedScrollView
participant RoomHeader
User->>MainActivity: Press TAB / DPAD key
MainActivity->>KeyboardA11yModule: isEnabled()
KeyboardA11yModule-->>MainActivity: true
MainActivity->>FocusUtils: hasInvertedParent(focusedView)?
FocusUtils-->>MainActivity: true
MainActivity->>MainActivity: Remap key (TAB ↔ DPAD up/down)
MainActivity->>InvertedScrollView: Dispatch remapped KeyEvent
InvertedScrollView->>InvertedScrollView: Move focus between list cells
alt reached list boundary with exitFocusNativeId
InvertedScrollView->>RoomHeader: focus exit target (nativeID lookup)
RoomHeader->>RoomHeader: set accessibility focus (useImperativeHandle)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.70.0.108314 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNSHg7TdML4PFPLPaD3vbMSQN1kCdTXPzueVdh4N44q6GZfR1wk0ZNlhk_GdG11POn-I-qGjSYJWmIjmQQKs |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.70.0.108317 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRigBMM3Bes80dsLl0Wof7_pwq8w6zFWBUUGsRGp2rRCO4FnN6K1TeJ4P-FQhppg3r60B8pyKQNdZQTh6Zy |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.70.0.108318 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQfUhHNuTEb7FEFekyzv7gX_VAjowLUgqhtyPgrwQ7XZI58MKTI6S0_cS2vJpxejQfVdMxEh288m2qGEJxg |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.71.0.108405 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTIPfz9s2BYeUgTdgwAbLARyfJ33Zu2uCC3-3khGJcUxnB3F036QKV341pD1p8E0edHZAtGH8Rj5BHqnrWZ |
Contributor
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx (1)
37-37: Assigning tocreateRef().currentis a TypeScript anti-pattern.
React.createRef()returns aRefObjectwith a readonlycurrentproperty. While this works at runtime, it bypasses TypeScript's intent. Since you need a mutable ref, use{ current: null }directly or a callback ref pattern:♻️ Proposed fix using mutable ref object
- private scrollRef = React.createRef<any>(); + private scrollRef: { current: any } = { current: null };Also applies to: 159-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx` at line 37, Replace the readonly RefObject created with React.createRef() with a mutable ref object so assignments to .current are type-safe: change the declaration private scrollRef = React.createRef<any>(); to a MutableRefObject like private scrollRef: React.MutableRefObject<any> = { current: null }; and update the other similar refs (the ones referenced in the later block that assign to .current) to use the same pattern or a callback ref; keep usages that set scrollRef.current unchanged but ensure their ref declarations are MutableRefObject types (referencing scrollRef and the other ref identifiers in this file).app/lib/native/KeyboardInversionA11yAndroid.ts (1)
17-19: Redundant default parameter.Since
scopeis typed as literal'room-view', the default value doesn't add flexibility—callers can only ever pass'room-view'. Consider simplifying:♻️ Proposed simplification
-export const enableRoomViewKeyboardA11y = (scope: 'room-view' = 'room-view') => { - NativeModule.enable(scope); +export const enableRoomViewKeyboardA11y = () => { + NativeModule.enable('room-view'); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/native/KeyboardInversionA11yAndroid.ts` around lines 17 - 19, The function enableRoomViewKeyboardA11y currently declares a redundant literal-typed parameter scope with a default; remove the parameter and hardcode the literal when calling NativeModule.enable (i.e., change enableRoomViewKeyboardA11y to take no args and call NativeModule.enable('room-view')), keeping the function and NativeModule.enable usage to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.java`:
- Around line 30-40: The enable()/disable() methods perform two separate
volatile writes to sEnabled and sScope which can be observed in an inconsistent
intermediate state by readers such as MainActivity.dispatchKeyEvent or getState;
wrap the compound write in a single atomic operation by synchronizing or using
an atomic holder: either place both assignments inside a synchronized block
(e.g., synchronized on KeyboardA11yModule.class) in enable(String) and
disable(), or replace sEnabled/sScope with a single AtomicReference or immutable
state object (e.g., KeyboardA11yState) updated atomically; update readers
(getState, dispatchKeyEvent) to read the single synchronized/atomic state to
ensure visibility and consistency.
In `@android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt`:
- Around line 47-82: The current dispatchKeyEvent remaps only ACTION_DOWN events
causing mismatched ACTION_UP keycodes; update the condition that checks the
event action in dispatchKeyEvent (used with KeyboardA11yModule.isEnabled() and
FocusUtils.hasInvertedParent(current)) to also handle KeyEvent.ACTION_UP (e.g.,
if event.action == KeyEvent.ACTION_DOWN || event.action == KeyEvent.ACTION_UP),
perform the same keyCode mapping logic for ACTION_UP, and construct/dispatch the
inverted KeyEvent (preserving downTime, eventTime, repeatCount, metaState,
deviceId, scanCode, flags, source) so both press and release events are
consistently remapped.
In
`@android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java`:
- Line 69: In InvertedScrollView.java the code casts getChildAt(0) directly to
ViewGroup into variable contentView which can throw ClassCastException if the
child is not a ViewGroup; change this to first retrieve the child into a View,
check "instanceof ViewGroup" (and for null), then assign to contentView only
when the check passes, and handle the else case safely (e.g., return early, fall
back to a no-op, or log an error) so the view hierarchy change won't crash the
method.
- Around line 85-92: The current focus-handling in InvertedScrollView (around
variables exitDir and exitTarget) always returns true even when
findExitTarget(exitDir) returns null, which consumes the key at the list
boundary and can trap keyboard users; update the method so that when exitTarget
== null it does not consume the event—either return false to allow default focus
handling or trigger accessible feedback (e.g., call
announceForAccessibility(...) or an EdgeEffect bounce) before returning true;
locate the block using exitDir/exitTarget in InvertedScrollView and replace the
final unconditional "return true" with logic that returns false when exitTarget
is null (or invokes an accessibility feedback helper then returns true) so users
receive proper feedback or focus moves.
In `@app/containers/Button/index.tsx`:
- Around line 91-101: Add an explicit accessibilityState to the RectButton so
screen readers announce when the button is disabled: update the RectButton (in
this file's Button component) to include accessibilityState={{ disabled:
!!isDisabled }} alongside existing props (e.g., onPress, enabled={!isDisabled},
accessibilityLabel={title}) so the disabled state is conveyed to assistive tech.
---
Nitpick comments:
In `@app/lib/native/KeyboardInversionA11yAndroid.ts`:
- Around line 17-19: The function enableRoomViewKeyboardA11y currently declares
a redundant literal-typed parameter scope with a default; remove the parameter
and hardcode the literal when calling NativeModule.enable (i.e., change
enableRoomViewKeyboardA11y to take no args and call
NativeModule.enable('room-view')), keeping the function and NativeModule.enable
usage to locate the change.
In `@app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx`:
- Line 37: Replace the readonly RefObject created with React.createRef() with a
mutable ref object so assignments to .current are type-safe: change the
declaration private scrollRef = React.createRef<any>(); to a MutableRefObject
like private scrollRef: React.MutableRefObject<any> = { current: null }; and
update the other similar refs (the ones referenced in the later block that
assign to .current) to use the same pattern or a callback ref; keep usages that
set scrollRef.current unchanged but ensure their ref declarations are
MutableRefObject types (referencing scrollRef and the other ref identifiers in
this file).
🪄 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: 3ccf1ed2-867a-4803-8928-cebf49d7f3ba
⛔ Files ignored due to path filters (6)
app/containers/Button/__snapshots__/Button.test.tsx.snapis excluded by!**/*.snapapp/containers/LoginServices/__snapshots__/LoginServices.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!**/*.snap
📒 Files selected for processing (22)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.javaandroid/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11ySpec.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/FocusUtils.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentViewManager.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollViewManager.javaandroid/app/src/main/res/values/ids.xmlapp/containers/Button/Button.test.tsxapp/containers/Button/index.tsxapp/containers/MessageComposer/interfaces.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/lib/methods/helpers/goRoom.tsapp/lib/native/KeyboardInversionA11yAndroid.tsapp/stacks/types.tsapp/views/RoomView/List/components/InvertedScrollView.tsxapp/views/RoomView/List/components/List.tsxapp/views/RoomView/List/components/RNLikeInvertedScrollView.tsxapp/views/RoomView/index.tsxapp/views/RoomsListView/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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:
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.javaandroid/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.javaapp/containers/RoomHeader/RoomHeader.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/Button/index.tsxapp/containers/RoomHeader/RoomHeader.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/RoomHeader/index.tsxapp/views/RoomView/index.tsx
🔇 Additional comments (25)
android/app/src/main/res/values/ids.xml (1)
1-5: LGTM!The resource ID declaration follows Android conventions and properly supports the native view tagging mechanism for inverted list detection.
app/containers/RoomHeader/RoomHeader.tsx (1)
138-174: LGTM!The
forwardRefconversion anduseImperativeHandleimplementation correctly expose an imperativefocus()method. The use offindNodeHandlewithAccessibilityInfo.setAccessibilityFocusis the appropriate pattern for programmatic accessibility focus in React Native.app/containers/RoomHeader/index.tsx (1)
27-104: LGTM!The
React.memo(React.forwardRef(...))composition is correctly implemented, properly forwarding the ref to the innerRoomHeadercomponent while maintaining memoization benefits.app/containers/MessageComposer/interfaces.ts (1)
5-10: LGTM!The
focus()method addition toIMessageComposerRefaligns with the existingIComposerInput.focus()contract and provides a consistent imperative API for programmatic focus control.app/views/RoomsListView/index.tsx (1)
76-76: LGTM!Passing
focusHeaderOnOpen: trueenables proper keyboard focus management when navigating to rooms. ThegoRoomhelper correctly applies this only in master-detail mode.app/views/RoomView/List/components/List.tsx (1)
47-49: LGTM!The
exitFocusNativeIdprop correctly configures the Android-specific focus exit target, enabling keyboard navigation to reach the message composer from the inverted message list.app/stacks/types.ts (1)
44-44: LGTM!The optional
focusHeaderOnOpenparameter correctly extends theRoomViewroute params type to support the keyboard focus navigation feature.app/lib/methods/helpers/goRoom.ts (2)
28-47: LGTM!The
focusHeaderOnOpenparameter integration correctly extends the navigation helper to support header focus in master-detail mode while preserving existing route param behavior.
94-104: LGTM!The type declaration properly documents the new optional
focusHeaderOnOpenparameter alongside existing navigation options.android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentViewManager.java (1)
27-30: LGTM!The
@ReactPropimplementation follows React Native conventions correctly, forwarding the boolean prop to the native view.android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollViewManager.java (1)
29-32: LGTM!The
@ReactPropforexitFocusNativeIdis correctly implemented with@Nullableannotation for the optional string prop.android/app/src/main/java/chat/rocket/reactnative/scroll/FocusUtils.java (1)
14-28: LGTM!The utility method is well-implemented with proper null safety and type checking. The parent chain traversal is safe and correctly terminates when reaching a non-View parent.
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.java (2)
39-53: Consider checking focusability in addition to visibility.In
onRequestFocusInDescendants, the loop checkschild.getVisibility() == VISIBLEbut doesn't verify if the child is actually focusable before callingrequestFocus(). WhilerequestFocus()will returnfalsefor non-focusable views, you could optimize by also checkingchild.isFocusable()or usechild.requestFocus()directly as you're doing (since it handles this internally).The current implementation is functionally correct, just noting this as a minor observation.
55-77: LGTM!The
addFocusablesoverride correctly reverses only the direct children's focusables in the list while preserving the order of focusables from nested descendants. The in-place swap algorithm is efficient.app/views/RoomView/index.tsx (2)
258-267: LGTM!The focus handling logic is well-structured:
- Uses
InteractionManager.runAfterInteractionsto avoid animation conflicts- Properly clears
focusHeaderOnOpenparam after focusing to prevent repeated focus- Correctly falls back to composer focus when header focus isn't needed
1574-1578: LGTM!The
nativeID='message-composer-exit-focus'correctly matches theexitFocusNativeIdprop passed to the inverted scroll component, enabling focus to exit the inverted list and land on the composer.android/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11ySpec.java (1)
9-30: LGTM!The TurboModule spec follows React Native conventions correctly with proper
@ReactMethodannotations and the asyncPromiseparameter forgetState.app/views/RoomView/List/components/InvertedScrollView.tsx (1)
1-10: LGTM!Clean re-export pattern with proper TypeScript typing. The
exitFocusNativeIdprop extension aligns with the native Android implementation.app/lib/native/KeyboardInversionA11yAndroid.ts (1)
21-25: LGTM!Clean TurboModule wrapper with proper type definitions.
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java (1)
95-136: LGTM!The helper methods
findContainingCellIndex,findExitTarget, andisDescendantOfare well-implemented. The navigation logic correctly handles the inverted view-tree order by reversing the iteration direction.app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx (3)
50-58: LGTM!Proper keyboard listener lifecycle management with cleanup in
componentWillUnmount.
172-203: LGTM!The render method correctly composes styles, forwards props including
exitFocusNativeId, and properly wraps children in the native content view withcollapsable={false}to prevent view flattening.
224-228: LGTM!Clean
forwardRefwrapper withdisplayNamefor easier debugging.app/containers/Button/Button.test.tsx (1)
67-67: Comment update is accurate and helpful.The revised note correctly documents the Jest/RNGestureHandlerButton mock behavior after migrating to
RectButton.app/containers/Button/index.tsx (1)
3-3:RectButtonprop-surface migration looks clean.Good move omitting
enabledfrom consumer props and mapping externaldisabledto internal enablement logic.Also applies to: 9-9, 20-20
android/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.java
Show resolved
Hide resolved
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java
Show resolved
Hide resolved
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/containers/MessageComposer/interfaces.ts (1)
5-10:⚠️ Potential issue | 🔴 CriticalAdd
focus()toMessageComposer'suseImperativeHandle.The interface now requires
focus()onIMessageComposerRef, butMessageComposer.tsxlines 55-59 do not expose it inuseImperativeHandle. The currently exposed methods are onlycloseEmojiKeyboardAndAction,getText, andsetInput. This will cause runtime failures when external code callscomposerRef.current.focus().Current implementation
useImperativeHandle(forwardedRef, () => ({ closeEmojiKeyboardAndAction, getText: composerInputComponentRef.current?.getText, setInput: composerInputComponentRef.current?.setInput }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/MessageComposer/interfaces.ts` around lines 5 - 10, The IMessageComposerRef now includes focus(), but MessageComposer.tsx's useImperativeHandle currently only exposes closeEmojiKeyboardAndAction, getText and setInput; update the useImperativeHandle(forwardedRef, ...) return object to also expose focus by wiring it to composerInputComponentRef.current?.focus (or the component's focus method), ensuring the forwardedRef provides a focus() implementation that delegates to the inner composer input's focus method and matches the IMessageComposerRef signature.app/views/RoomView/index.tsx (1)
222-266:⚠️ Potential issue | 🟠 MajorRead
focusHeaderOnOpenfrom live props, not the mount-timeroute.
routeis captured once incomponentDidMount, so this listener keeps reading the original params. After Line 262 clearsfocusHeaderOnOpen, the callback can still see the oldtrueand refocus the header every time the room regains focus.Suggested fix
- const { navigation, dispatch, isMasterDetail, route } = this.props; + const { navigation, dispatch, isMasterDetail } = this.props; ... this.unsubscribeFocus = navigation.addListener('focus', () => { InteractionManager.runAfterInteractions(() => { - if (isMasterDetail && route?.params?.focusHeaderOnOpen) { + if (isMasterDetail && this.props.route?.params?.focusHeaderOnOpen) { this.roomHeaderRef.current?.focus(); navigation.setParams({ focusHeaderOnOpen: undefined }); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomView/index.tsx` around lines 222 - 266, The focus listener currently closes over the mount-time route/navigation variables so it keeps reading the original route.params.focusHeaderOnOpen; update the navigation.addListener('focus', ...) callback (the code that references isMasterDetail, route?.params?.focusHeaderOnOpen, roomHeaderRef, messageComposerRef, and navigation.setParams) to read the live props instead (e.g. const focusHeaderOnOpen = this.props.route?.params?.focusHeaderOnOpen and use this.props.navigation.setParams) or use this.props.navigation.getParam, and similarly use this.props.navigation and this.props.route inside the callback so clearing focusHeaderOnOpen actually takes effect on subsequent focuses.
♻️ Duplicate comments (3)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt (1)
51-76:⚠️ Potential issue | 🟡 MinorRemap the matching
ACTION_UPevent too.Android sends paired down/up events. Right now the press is remapped but the release keeps the original keycode, so the framework sees mixed pairs for the same navigation step.
Suggested fix
- if (event.action == KeyEvent.ACTION_DOWN) { + if (event.action == KeyEvent.ACTION_DOWN || event.action == KeyEvent.ACTION_UP) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt` around lines 51 - 76, The current dispatchKeyEvent remaps only ACTION_DOWN events (uses keyCode -> mapped and builds invertedEvent) but leaves ACTION_UP unmapped causing mismatched pairs; update dispatchKeyEvent to also detect KeyEvent.ACTION_UP, compute the same mapped key (reuse the mapped logic for keyCode), construct an invertedEvent for ACTION_UP preserving event.downTime, event.eventTime, repeatCount, metaState, deviceId, scanCode, flags, source, and call return super.dispatchKeyEvent(invertedEvent) for the UP case as well so both press and release use the same mapped key; keep existing behavior for other actions.android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java (2)
69-69:⚠️ Potential issue | 🟡 MinorMissing type check before cast remains unaddressed.
The direct cast
(ViewGroup) getChildAt(0)can throwClassCastExceptionif the first child isn't aViewGroup. Add a defensive check as previously suggested.🛡️ Proposed defensive check
- ViewGroup contentView = (ViewGroup) getChildAt(0); + View child = getChildAt(0); + if (!(child instanceof ViewGroup)) { + return false; + } + ViewGroup contentView = (ViewGroup) child;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java` at line 69, In InvertedScrollView, avoid the unsafe cast to ViewGroup by checking the child first: call getChildAt(0) into a local (e.g., child), verify child != null and child instanceof ViewGroup before casting to contentView, and handle the else case (e.g., skip processing, return, or log a warning) so a ClassCastException cannot occur when the first child is not a ViewGroup.
85-92:⚠️ Potential issue | 🟡 MinorBoundary key consumption issue remains unaddressed.
When at the list boundary with no exit target found, returning
true(line 92) consumes the key event without moving focus anywhere. This traps keyboard users. Consider returningfalseto allow default system handling.💡 Proposed fix
View exitTarget = findExitTarget(exitDir); if (exitTarget != null) { exitTarget.requestFocus(); return true; } - return true; + return false; // Let system handle boundary case }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java` around lines 85 - 92, The key handler in InvertedScrollView currently returns true even when findExitTarget(exitDir) returns null, which consumes boundary key events and traps keyboard users; update the logic in the method handling directional focus (using exitDir and exitTarget) so that if exitTarget is null you return false (allowing default system handling) instead of true, while keeping the existing behavior of requesting focus and returning true when exitTarget != null.
🧹 Nitpick comments (2)
app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx (1)
36-48: Heavy reliance onanytypes reduces type safety.Multiple refs and handlers use
any, which bypasses TypeScript's type checking. While this is common with native components, consider using more specific types where possible:
scrollRefcould be typed asReact.RefObject<typeof NativeInvertedScrollView>if the native component exposes typingsStateinterface is well-defined, which is goodThis is acceptable for now given native component limitations, but worth revisiting if the native layer adds TypeScript definitions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx` around lines 36 - 48, The class RNLikeInvertedScrollView uses broad any types for scrollRef and keyboard subscription fields; replace them with more specific types (e.g., change scrollRef: React.createRef<any>() to React.createRef<NativeInvertedScrollView | null>() or React.RefObject<ScrollView | null> depending on the native component type you can import), and type the keyboard subscription fields as { remove(): void } | undefined instead of any; update the declarations for _subscriptionKeyboardDidShow, _subscriptionKeyboardDidHide, and _keyboardMetrics (e.g., { height: number } | null already ok) and adjust any handler params/signatures that currently use any to use the appropriate event/interface from React Native so TypeScript can check usages in RNLikeInvertedScrollView.app/lib/native/KeyboardInversionA11yAndroid.ts (1)
15-15: Consider adding a platform check for future safety, though currently unused.This module has no call sites in the codebase currently. However, since
TurboModuleRegistry.getEnforcingwill throw on iOS, consider adding a platform check at module load time to prevent crashes if this is imported from iOS code in the future:+import { Platform } from 'react-native'; + +if (Platform.OS !== 'android') { + throw new Error('KeyboardInversionA11yAndroid is only available on Android'); +} + const NativeModule = TurboModuleRegistry.getEnforcing<Spec>('KeyboardA11y');Alternatively, document the Android-only requirement in JSDoc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/native/KeyboardInversionA11yAndroid.ts` at line 15, The file currently calls TurboModuleRegistry.getEnforcing<Spec>('KeyboardA11y') at module load (const NativeModule = TurboModuleRegistry.getEnforcing<Spec>('KeyboardA11y')) which will throw on non-Android platforms; wrap this call in a Platform.OS === 'android' guard so the TurboModule is only resolved on Android and export a safe null/undefined fallback (or no-op shim) for other platforms, and add a short JSDoc note on the KeyboardInversionA11yAndroid export indicating it is Android-only; alternatively, if you prefer not to add runtime branching, add the Android-only JSDoc comment above the module to document the requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.java`:
- Around line 40-52: The onRequestFocusInDescendants method currently always
iterates children from last-to-first; when mIsInvertedContent is true you must
respect the direction parameter: if direction == View.FOCUS_UP iterate forward
from index 0 to getChildCount()-1 so the bottom-most child (index 0 in inverted
content) is tried first, otherwise iterate backward from getChildCount()-1 to 0;
update the loop in onRequestFocusInDescendants to choose the iteration order
based on the direction before calling getChildAt(...) and
child.requestFocus(...).
---
Outside diff comments:
In `@app/containers/MessageComposer/interfaces.ts`:
- Around line 5-10: The IMessageComposerRef now includes focus(), but
MessageComposer.tsx's useImperativeHandle currently only exposes
closeEmojiKeyboardAndAction, getText and setInput; update the
useImperativeHandle(forwardedRef, ...) return object to also expose focus by
wiring it to composerInputComponentRef.current?.focus (or the component's focus
method), ensuring the forwardedRef provides a focus() implementation that
delegates to the inner composer input's focus method and matches the
IMessageComposerRef signature.
In `@app/views/RoomView/index.tsx`:
- Around line 222-266: The focus listener currently closes over the mount-time
route/navigation variables so it keeps reading the original
route.params.focusHeaderOnOpen; update the navigation.addListener('focus', ...)
callback (the code that references isMasterDetail,
route?.params?.focusHeaderOnOpen, roomHeaderRef, messageComposerRef, and
navigation.setParams) to read the live props instead (e.g. const
focusHeaderOnOpen = this.props.route?.params?.focusHeaderOnOpen and use
this.props.navigation.setParams) or use this.props.navigation.getParam, and
similarly use this.props.navigation and this.props.route inside the callback so
clearing focusHeaderOnOpen actually takes effect on subsequent focuses.
---
Duplicate comments:
In `@android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt`:
- Around line 51-76: The current dispatchKeyEvent remaps only ACTION_DOWN events
(uses keyCode -> mapped and builds invertedEvent) but leaves ACTION_UP unmapped
causing mismatched pairs; update dispatchKeyEvent to also detect
KeyEvent.ACTION_UP, compute the same mapped key (reuse the mapped logic for
keyCode), construct an invertedEvent for ACTION_UP preserving event.downTime,
event.eventTime, repeatCount, metaState, deviceId, scanCode, flags, source, and
call return super.dispatchKeyEvent(invertedEvent) for the UP case as well so
both press and release use the same mapped key; keep existing behavior for other
actions.
In
`@android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java`:
- Line 69: In InvertedScrollView, avoid the unsafe cast to ViewGroup by checking
the child first: call getChildAt(0) into a local (e.g., child), verify child !=
null and child instanceof ViewGroup before casting to contentView, and handle
the else case (e.g., skip processing, return, or log a warning) so a
ClassCastException cannot occur when the first child is not a ViewGroup.
- Around line 85-92: The key handler in InvertedScrollView currently returns
true even when findExitTarget(exitDir) returns null, which consumes boundary key
events and traps keyboard users; update the logic in the method handling
directional focus (using exitDir and exitTarget) so that if exitTarget is null
you return false (allowing default system handling) instead of true, while
keeping the existing behavior of requesting focus and returning true when
exitTarget != null.
---
Nitpick comments:
In `@app/lib/native/KeyboardInversionA11yAndroid.ts`:
- Line 15: The file currently calls
TurboModuleRegistry.getEnforcing<Spec>('KeyboardA11y') at module load (const
NativeModule = TurboModuleRegistry.getEnforcing<Spec>('KeyboardA11y')) which
will throw on non-Android platforms; wrap this call in a Platform.OS ===
'android' guard so the TurboModule is only resolved on Android and export a safe
null/undefined fallback (or no-op shim) for other platforms, and add a short
JSDoc note on the KeyboardInversionA11yAndroid export indicating it is
Android-only; alternatively, if you prefer not to add runtime branching, add the
Android-only JSDoc comment above the module to document the requirement.
In `@app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx`:
- Around line 36-48: The class RNLikeInvertedScrollView uses broad any types for
scrollRef and keyboard subscription fields; replace them with more specific
types (e.g., change scrollRef: React.createRef<any>() to
React.createRef<NativeInvertedScrollView | null>() or React.RefObject<ScrollView
| null> depending on the native component type you can import), and type the
keyboard subscription fields as { remove(): void } | undefined instead of any;
update the declarations for _subscriptionKeyboardDidShow,
_subscriptionKeyboardDidHide, and _keyboardMetrics (e.g., { height: number } |
null already ok) and adjust any handler params/signatures that currently use any
to use the appropriate event/interface from React Native so TypeScript can check
usages in RNLikeInvertedScrollView.
🪄 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: 1c5841f3-c217-42c5-8fd1-fcfe793941e1
⛔ Files ignored due to path filters (6)
app/containers/Button/__snapshots__/Button.test.tsx.snapis excluded by!**/*.snapapp/containers/LoginServices/__snapshots__/LoginServices.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!**/*.snap
📒 Files selected for processing (22)
android/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.javaandroid/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11ySpec.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/FocusUtils.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentViewManager.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.javaandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollViewManager.javaandroid/app/src/main/res/values/ids.xmlapp/containers/Button/Button.test.tsxapp/containers/Button/index.tsxapp/containers/MessageComposer/interfaces.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/lib/methods/helpers/goRoom.tsapp/lib/native/KeyboardInversionA11yAndroid.tsapp/stacks/types.tsapp/views/RoomView/List/components/InvertedScrollView.tsxapp/views/RoomView/List/components/List.tsxapp/views/RoomView/List/components/RNLikeInvertedScrollView.tsxapp/views/RoomView/index.tsxapp/views/RoomsListView/index.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📚 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:
android/app/src/main/java/chat/rocket/reactnative/a11y/KeyboardA11yModule.javaapp/containers/Button/index.tsxandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.javaapp/containers/RoomHeader/RoomHeader.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/RoomHeader/index.tsxapp/views/RoomView/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/Button/index.tsxandroid/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.javaapp/containers/RoomHeader/RoomHeader.tsx
📚 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:
android/app/src/main/java/chat/rocket/reactnative/MainActivity.kt
🔇 Additional comments (17)
app/containers/Button/index.tsx (2)
91-101: MissingaccessibilityStatefor disabled button announcement.The
RectButtondoes not automatically expose disabled accessibility semantics whenenabled={false}. Screen readers (TalkBack on Android, VoiceOver on iOS) require explicitaccessibilityState={{ disabled: !!isDisabled }}to announce the disabled state.Suggested fix
<RectButton onPress={onPress} enabled={!isDisabled} + accessibilityState={{ disabled: !!isDisabled }} activeOpacity={0.2} underlayColor='transparent'
3-3: LGTM on the interface changes.The interface properly extends
RectButtonPropswhile omittingchildrenandenabledto avoid conflicts. Adding the explicitdisabled?: booleanprop maintains a clean API for consumers.Also applies to: 9-21
app/containers/Button/Button.test.tsx (1)
64-69: LGTM!The updated comment accurately documents the testing limitation with
RNGestureHandlerButtonin Jest. The assertion correctly validates theenabledprop for the newRectButtonimplementation.android/app/src/main/res/values/ids.xml (1)
1-5: LGTM!The resource ID declaration is correctly formatted and follows Android naming conventions. This ID will serve as a tag marker for inverted list detection in the focus navigation logic.
app/containers/RoomHeader/RoomHeader.tsx (1)
86-88: LGTM!The imperative focus API is correctly implemented using the standard React Native pattern with
findNodeHandleandAccessibilityInfo.setAccessibilityFocus. The null check onnodeHandleproperly guards against the case where the view hasn't mounted yet.Also applies to: 138-174
app/containers/RoomHeader/index.tsx (1)
27-107: LGTM!The ref forwarding is correctly implemented with
React.memo(React.forwardRef(...))pattern. TheIRoomHeaderReftype re-export enables proper typing at consumer sites.app/views/RoomView/List/components/List.tsx (1)
47-49: LGTM!The
exitFocusNativeIdprop is correctly conditionally passed only for non-iOS platforms, aligning with the Android-specific keyboard navigation fix. The hardcoded string'message-composer-exit-focus'should match thenativeIDset on the message composer wrapper inRoomView.app/views/RoomsListView/index.tsx (1)
74-77: LGTM!Adding
focusHeaderOnOpen: trueensures that when users navigate to a room from the rooms list (especially via keyboard), the accessibility focus moves to the room header. This is the correct behavior for keyboard navigation flow.app/stacks/types.ts (1)
44-44: LGTM!The optional
focusHeaderOnOpenparameter is correctly typed and positioned within theRoomViewroute params.android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollViewManager.java (1)
28-32: LGTM!The
@ReactPropregistration follows standard React Native view manager patterns. The nullable annotation correctly handles cases where the prop is not set from JavaScript.app/views/RoomView/List/components/InvertedScrollView.tsx (1)
1-10: Clean re-export pattern for the new inverted scroll implementation.The type cast approach correctly exposes
InvertedScrollViewPropswhile hiding the internalscrollViewRefprop that's managed by theforwardRefwrapper. TheexitFocusNativeIdprop is properly typed and flows through to the native layer.app/views/RoomView/List/components/RNLikeInvertedScrollView.tsx (3)
50-58: LGTM!Keyboard listener lifecycle is properly managed with setup in
componentDidMountand teardown incomponentWillUnmountusing optional chaining for safety.
68-75: Animation detection logic is well-designed.The
isAnimatingcheck using momentum scroll timing with a 16ms threshold (one frame at 60fps) effectively detects ongoing scroll animations. This prevents touch events from being stolen during smooth scrolling.
172-203: Render composition is correct.Destructuring
onContentSizeChangefromrestprevents passing it to the native scroll view (which doesn't support it), whilehandleContentOnLayouton the content view correctly emulates this behavior. The style composition and prop forwarding are well-structured.android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollView.java (3)
95-106: LGTM!
findContainingCellIndexcorrectly traverses the view hierarchy to locate the containing cell, with proper handling for the edge case where a parent isn't aView.
108-136: Well-structured exit target resolution.The two-phase approach (nativeID lookup then FocusFinder fallback) provides flexibility while maintaining system-like focus behavior. The
isDescendantOfcheck correctly prevents focus from cycling back into the scroll view.
35-57: Key event dispatch is correctly structured.The
mKeyConsumedflag pattern correctly handles the ACTION_DOWN/ACTION_UP pairing, processing navigation on ACTION_DOWN and returning the cached result for ACTION_UP. Direction mapping is correct for both DPAD and TAB keys.
android/app/src/main/java/chat/rocket/reactnative/scroll/InvertedScrollContentView.java
Show resolved
Hide resolved
|
Android Build Available Rocket.Chat Experimental 4.71.0.108432 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTtIwYdUnrVonpp1RfgQTAEX2ij3rh0R23ZYQhW3GqQEPZOmgSedjT5eSWj_xUeEZiNZq1RyP3wX_5QZ26W |
|
iOS Build Available Rocket.Chat Experimental 4.71.0.108433 |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/containers/Touch.tsx (3)
50-51: Outdated comment referencesRectButton.The comment still mentions "RectButton" but the component now uses
Pressable. Update for accuracy.📝 Suggested fix
- // The background color must be applied to the RectButton, not the View. - // If set on the View, the touch opacity animation won't work properly. + // The background color must be applied to the Pressable, not the View. + // If set on the View, the touch opacity animation won't work properly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/Touch.tsx` around lines 50 - 51, Update the outdated comment in Touch (app/containers/Touch.tsx) to refer to Pressable instead of RectButton: change the sentence that currently reads "The background color must be applied to the RectButton, not the View." to "The background color must be applied to the Pressable, not the View." so the comment accurately reflects the current component usage in the Touch component.
67-69: Outdated comment referencesRectButton.Same issue—update comment to reflect the switch to
Pressable.📝 Suggested fix
// The margin should be applied to the parent component. - // If set on the View, it will create an internal margin inside the RectButton. + // If set on the View, it will create an internal margin inside the Pressable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/Touch.tsx` around lines 67 - 69, The inline comment above the marginStyles constant still mentions RectButton even though the component now uses Pressable; update that comment in Touch.tsx to reference Pressable (or generically "Pressable/Touchable element") and clearly state that the margin must be applied to the parent to avoid internal spacing inside the Pressable; locate the comment immediately above const marginStyles and replace "RectButton" with "Pressable" and adjust phrasing to match the current implementation.
23-23: Consider renamingrectButtonStyleprop.This prop name references the now-removed
RectButton. Since this is a public interface and other components may depend on it, you could deprecate it with an alias or rename it to something generic likecontainerStylein a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/containers/Touch.tsx` at line 23, The public prop rectButtonStyle on the Touch component still references the removed RectButton; add a new generic prop (e.g., containerStyle) to the TouchProps interface and update the Touch component to accept both containerStyle and rectButtonStyle, mapping them to the actual underlying style prop (merge containerStyle and rectButtonStyle, with containerStyle taking precedence) so existing users don’t break; keep rectButtonStyle as a deprecated alias and emit a single console.warn when rectButtonStyle is passed (or both are passed) to guide users to containerStyle, and update any prop-type/TS definitions and the component’s props destructuring/usage to use the merged style for the rendered wrapper element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/containers/Touch.tsx`:
- Around line 50-51: Update the outdated comment in Touch
(app/containers/Touch.tsx) to refer to Pressable instead of RectButton: change
the sentence that currently reads "The background color must be applied to the
RectButton, not the View." to "The background color must be applied to the
Pressable, not the View." so the comment accurately reflects the current
component usage in the Touch component.
- Around line 67-69: The inline comment above the marginStyles constant still
mentions RectButton even though the component now uses Pressable; update that
comment in Touch.tsx to reference Pressable (or generically "Pressable/Touchable
element") and clearly state that the margin must be applied to the parent to
avoid internal spacing inside the Pressable; locate the comment immediately
above const marginStyles and replace "RectButton" with "Pressable" and adjust
phrasing to match the current implementation.
- Line 23: The public prop rectButtonStyle on the Touch component still
references the removed RectButton; add a new generic prop (e.g., containerStyle)
to the TouchProps interface and update the Touch component to accept both
containerStyle and rectButtonStyle, mapping them to the actual underlying style
prop (merge containerStyle and rectButtonStyle, with containerStyle taking
precedence) so existing users don’t break; keep rectButtonStyle as a deprecated
alias and emit a single console.warn when rectButtonStyle is passed (or both are
passed) to guide users to containerStyle, and update any prop-type/TS
definitions and the component’s props destructuring/usage to use the merged
style for the rendered wrapper element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3eb629a8-33d6-45b3-8abb-6a365995821d
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
app/containers/Touch.tsxapp/containers/message/Reactions.tsxapp/containers/message/Thread.tsxapp/containers/message/Touchable.tsxapp/containers/message/index.tsxpackage.json
✅ Files skipped from review due to trivial changes (2)
- package.json
- app/containers/message/Reactions.tsx
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📚 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/message/index.tsxapp/containers/message/Thread.tsxapp/containers/Touch.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/message/Touchable.tsxapp/containers/message/Thread.tsxapp/containers/Touch.tsx
🔇 Additional comments (6)
app/containers/message/Touchable.tsx (2)
24-25: Verify intentional fallback ofonHoverOuttoonLongPress.When no
onHoverOuthandler is provided, this falls back to triggeringonLongPresson hover-out. This means hovering away from the element will invoke the long-press action (typically showing message actions), which may cause unexpected behavior on desktop/mouse users.If this is intentional for accessibility or specific UX requirements, please add a comment explaining the rationale. Otherwise, consider using a neutral fallback like
undefined.
12-13: LGTM on keyboard focus integration.The use of
withKeyboardFocusHOC to wrapPressableand the addition of thefocusableprop correctly enables keyboard navigation for message items, aligning with the PR's objective to fix inverted keyboard navigation on Android.Also applies to: 21-28
app/containers/message/index.tsx (1)
444-444: LGTM on exposingonThreadPressvia context.This correctly adds
onThreadPressto theMessageContext.Providervalue, enablingThread.tsx(and potentially other child components) to invoke thread navigation via keyboard or press interaction. The wiring aligns with the existingonThreadPressmethod defined at lines 236-241.app/containers/message/Thread.tsx (1)
10-10: LGTM on Thread keyboard accessibility.The component correctly:
- Imports and uses the keyboard-focusable
Touchablewrapper- Consumes
onThreadPressfromMessageContext- Applies proper accessibility attributes (
accessibilityRole='button',accessibilityLabel)- Preserves the
testIDfor testingThis enables keyboard-driven thread navigation as intended by the PR.
Also applies to: 17-17, 28-35
app/containers/Touch.tsx (2)
80-106: LGTM on the Pressable implementation.The migration from
RectButtontoPressableis well-structured:
android_rippleprovides native feedback on Android- Dynamic
stylecallback handles pressed-state visuals- Accessibility props correctly applied to inner
Viewdisabledprop correctly wired toPressableThis aligns with the PR's goal of enabling proper keyboard navigation focus handling.
85-93: Dark theme press feedback is nearly imperceptible on iOS.With
activeOpacitydefaulting to1, iOS press feedback relies solely on backgroundColor shift fromsurfaceRoomtosurfaceNeutral. In light mode, this shift is adequate (#FFFFFF→#E4E7EA). However, in dark mode, the contrast is too subtle (#1F2329→#2D3039). Components without explicit backgroundColor—such asmessage/User(titleContainer),RoomView/LoadMore(button), andScreenLockedView(close)—show almost no visual feedback on press in dark theme.Consider setting
activeOpacityto a lower value (e.g.,0.8or0.7) as the default or for dark-theme variants to provide more obvious press feedback across iOS.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@patches/react-native-gesture-handler`+2.24.0.patch:
- Around line 14-21: The _isActivationKeyPress:(NSSet<UIPress *> *)presses
method only checks for UIKeyboardHIDUsageKeyboardSpacebar and
UIKeyboardHIDUsageKeyboardReturnOrEnter; update it to also treat
UIKeyboardHIDUsageKeyboardKeypadEnter as an activation key so external numpad
Enter is recognized. Locate the _isActivationKeyPress implementation and add a
condition that checks for press.key.keyCode ==
UIKeyboardHIDUsageKeyboardKeypadEnter alongside the existing ReturnOrEnter and
Spacebar checks, ensuring the method returns YES when any of these keyCodes are
present.
🪄 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: 1aa59a98-a548-48e6-a82d-7306c8555c65
📒 Files selected for processing (1)
patches/react-native-gesture-handler+2.24.0.patch
📜 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). (2)
- GitHub Check: format
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (1)
📚 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:
patches/react-native-gesture-handler+2.24.0.patch
🔇 Additional comments (2)
patches/react-native-gesture-handler+2.24.0.patch (2)
9-12: Good focusability guard for disabled/non-interactive states.This should keep keyboard focus off controls that are not actionable.
28-53: No additional verification needed. The patch correctly adds keyboard support (Space/Return) toRNGestureHandlerButtonby synthesizing a singleUIControlEventTouchUpInsideevent per key press. The nestedViewwithaccessibleandaccessibilityRole='button'in BaseButton.tsx is the correct accessibility pattern—it provides screen reader metadata for the BorderlessButton's interactive element, not a separate activation path. Keyboard and accessibility activation will fire theonPresshandler once as intended.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/views/SecurityPrivacyView.tsx (1)
98-109: Guard against duplicate side-effects when row press and switch change both fire.If both callbacks execute for one user action, this can double-log events and duplicate
AsyncStorage.setItemwrites. Consider making toggles idempotent with a quick equality guard.Proposed hardening diff
const toggleCrashReport = (value: boolean) => { + if (value === crashReportState) { + return; + } logEvent(events.SP_TOGGLE_CRASH_REPORT); AsyncStorage.setItem(CRASH_REPORT_KEY, JSON.stringify(value)); setCrashReportState(value); toggleCrashErrorsReport(value); }; const toggleAnalyticsEvents = (value: boolean) => { + if (value === analyticsEventsState) { + return; + } logEvent(events.SP_TOGGLE_ANALYTICS_EVENTS); AsyncStorage.setItem(ANALYTICS_EVENTS_KEY, JSON.stringify(value)); setAnalyticsEventsState(value); toggleAnalyticsEventsReport(value); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/views/SecurityPrivacyView.tsx` around lines 98 - 109, The row press and Switch onValueChange handlers (used with analytics toggles and crash report toggles) can both fire and cause duplicate side-effects; update the toggle handlers (toggleAnalyticsEvents and toggleCrashReport) to be idempotent by checking the incoming desired value against the current state (analyticsEventsState / crashReportState) and returning early if they are equal, so the AsyncStorage.setItem, logging, and other side-effects only run when the state actually changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/views/SecurityPrivacyView.tsx`:
- Around line 98-109: The row press and Switch onValueChange handlers (used with
analytics toggles and crash report toggles) can both fire and cause duplicate
side-effects; update the toggle handlers (toggleAnalyticsEvents and
toggleCrashReport) to be idempotent by checking the incoming desired value
against the current state (analyticsEventsState / crashReportState) and
returning early if they are equal, so the AsyncStorage.setItem, logging, and
other side-effects only run when the state actually changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb10760b-858b-411f-8128-0e7c978da7d7
📒 Files selected for processing (2)
app/views/DisplayPrefsView.tsxapp/views/SecurityPrivacyView.tsx
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📚 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/SecurityPrivacyView.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/views/SecurityPrivacyView.tsx
🔇 Additional comments (3)
app/views/DisplayPrefsView.tsx (1)
121-121: Nice improvement: row-level toggle for Avatars is now keyboard-friendly.Adding
onPress={toggleAvatar}on Line 121 improves activation from list-item focus and matches the intended accessibility behavior for this settings row.app/views/SecurityPrivacyView.tsx (2)
98-100: Good accessibility and interaction improvement for analytics toggle.Making the row pressable and setting the switch role is a solid UX/a11y improvement here.
107-109: Correct state source is now used for crash-report accessibility label.Switching
additionalAccessibilityLabeltocrashReportStatefixes the prior mismatch and keeps announcements aligned with actual state.
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
1 participant
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.
Proposed changes
Users should be able to navigate through the app using an external keyboard.
Issue(s)
https://rocketchat.atlassian.net/browse/MA-266
How to test or reproduce
What we should ensure:
1 - The RoomView message list must work as expected on both platforms;
2 - In master-detail, we must be able to focus on items inside the modal (Android issue);
3 - ActionSheet navigation must work as expected on both platforms.
What is not covered here:
1 - Keyboard navigation for room swipe actions.
PS: Must be tested on android, iOS, iPad and Android tablet.
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Accessibility
Bug Fixes
Refactor