fix(android): correct accessibility focus order in inverted FlatList#6934
fix(android): correct accessibility focus order in inverted FlatList#6934OtavioStasiak wants to merge 29 commits intodevelopfrom
Conversation
WalkthroughAdds Android-native inverted scroll views, view managers, and a React Native package; introduces an Android-only TSX wrapper and wires FlatList to use the native inverted scroll on non-iOS platforms. Accessibility traversal is reversed to match the visual newest-first order. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as React (FlatList)
participant RN as React Native Bridge
participant Native as InvertedScrollView / InvertedScrollContentView
participant A11y as Android TalkBack
JS->>RN: renderScrollComponent -> InvertedScrollView
RN->>Native: createViewInstance(ThemedReactContext)
Native->>Native: mount children in inverted newest-first order
A11y->>Native: request accessibility children traversal
Native-->>A11y: return reversed children list to match visual order
A11y-->>JS: user navigates (TalkBack follows visual order)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Android Build Available Rocket.Chat Experimental 4.69.0.108196 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTlK_aaCydNoR8tL0LU9ha91x4SUn6F8LQ_l7193fkFssxP1kKCqvtZvuPhoyE9s7WolKu01pF8dITARNJC |
|
Android Build Available Rocket.Chat Experimental 4.69.0.108208 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRkEKjr0nv7NcI6xLOU1Hf33CN6fGAImxz2dxN5rwCG938UBKpFBFtPdDKYUQ0167PNBAipwJsTvABYyrAj |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/views/RoomView/List/components/InvertedScrollView.tsx`:
- Around line 106-114: The ref callback setRef assigns the external ref (casting
node to InvertedScrollViewRef) but the ScrollableMethods are attached later in
useLayoutEffect, so synchronous calls to methods like scrollTo from a consumer's
ref callback may fail; update the file to add a short inline comment near setRef
(and reference InvertedScrollViewRef, setRef, and
useLayoutEffect/ScrollableMethods) explaining that methods are attached in
useLayoutEffect and thus consumers should not call instance methods
synchronously in the ref callback, and recommend accessing the methods from an
effect or event handler instead.
- Around line 55-103: The useLayoutEffect in InvertedScrollView is missing a
dependency array causing repeated re-patching of methods; update the
useLayoutEffect call so it only runs when the internal node changes (e.g., add a
dependency array such as [internalRef.current] or [] per desired behavior) to
avoid reassigning patchedNode.scrollTo, scrollToEnd, flashScrollIndicators,
getScrollRef and setNativeProps on every render; ensure the dependency array is
added to the useLayoutEffect invocation that wraps the patching logic.
- Around line 157-158: The InvertedScrollView component is destructuring
maintainVisibleContentPosition, snapToAlignment, and stickyHeaderIndices but not
passing them to the rendered ScrollView, so their native behaviors are lost;
update the return in InvertedScrollView (the JSX that renders <ScrollView ...
/>) to forward these props (maintainVisibleContentPosition, snapToAlignment,
stickyHeaderIndices) along with restWithoutStyle/style/ref/horizontal so the
native view receives them (also ensure TypeScript types/interfaces for
InvertedScrollView accept these props if needed).
7daddeb to
d7a0a5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/views/RoomView/List/components/InvertedScrollView.tsx`:
- Around line 77-83: The fallback for node.setNativeProps is recursive because
it overwrites the method and then calls the same property; fix this in
InvertedScrollView.tsx by first capturing the original method (e.g., const
origSetNativeProps = (node as any).setNativeProps) before assigning the
fallback, then assign node.setNativeProps to a wrapper that calls
origSetNativeProps if it's a function, otherwise becomes a safe no-op; ensure
you reference node.setNativeProps and origSetNativeProps (or similar) so the
wrapper never invokes itself.
|
Android Build Available Rocket.Chat Experimental 4.69.0.108209 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNT-qSf1sj6yCWFc7EBUnCX7872g8JTAnZzbGkVwQo0KMctVry7qMTkO4_EipaKXVvueHGOoPpO8rG9jHbly |
Proposed changes
create a native module to use a native ScrollView component that uses a11y children in the right order.
Issue(s)
https://rocketchat.atlassian.net/browse/MA-96
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Bug Fixes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.