Fix crash with nested FlatLists and fix edge case with nested views#50827
Closed
jorge-cab wants to merge 5 commits into
Closed
Fix crash with nested FlatLists and fix edge case with nested views#50827jorge-cab wants to merge 5 commits into
jorge-cab wants to merge 5 commits into
Conversation
Summary: Introduce a trait to be able to tell if a ShadowNode is focusable by keyboard. This will be used for focus ordering that delegates the work to the shadow tree when Native platforms don't have enough information to define the next focusable node Changelog: [Internal] Reviewed By: joevilches Differential Revision: D73114986
Summary: Pull Request resolved: facebook#50196 Currently when `removeClippedSubviews` is enabled on Android keyboard navigation breaks and we can never focus the elements that are clipped. iOS has a similar issue but not as drastic, it only happens when elements on the FlatList have a lot of margin between them. This algorithm aims to find the next focusable view and return it to native so that we can prevent the clipping of the view on the view clipping algorithm and hence fix keyboard navigation. For more information see D71324219 Fabric algorithm to find the next focusable view given: `parentTag`: Top most relevant parent of the focused view `focusedTag`: Tag of the currently focused view `direction`: Direction in which focus is moving Changelog: [Internal] Reviewed By: joevilches Differential Revision: D73115104
…nt of a child view (facebook#50404) Summary: Pull Request resolved: facebook#50404 Add another function to fabric to get the topmost stacking context parent given a root and a child. This is to be used on focus searching algorithm in the case where the next focusable child is deeper in the hierarchy meaning we need to find the top most parent in the Android hierarchy and lay that out as well before transferring focus. If we don't lay out the parent as well as the next focusable view: - The next focusable view might lack context given by the parent - If the parent is a scrollview and has removeClippedSubviews enabled then laying out the next focusable view will not work - If the view is deeper in the android hierarchy in some cases it won't be layed out unless the parent is Changelog: [Internal] Reviewed By: joevilches Differential Revision: D73114933
…bled (facebook#50105) Summary: Pull Request resolved: facebook#50105 Pull Request resolved: facebook#49543 When using `ReactScrollView` or `ReactHorizontalScrollView` Views with `removeClippedSubviews` keyboard navigation didn't work. This is because keyboard navigation relies on Android's View hierarchy to find the next focusable element. With `removeClippedSubviews` the next View might've been removed from the hierarchy. With this change we delegate the job of figuring out the next focusable element to the Shadow Tree, which will always contain layout information of the next element of the ScrollView. We then prevent the clipping of the topmost parent of the next focusable view to lay out the entire containing element in case we have some necessary context in the parent Changelog: [Android][Fixed] - Fix keyboard navigation on lists with `removeClippedSubviews` enabled Differential Revision: D73114782
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D73213775 |
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D73213775 |
jorge-cab
force-pushed
the
export-D73213775
branch
from
473d8fd to
d409b8a
Compare
…acebook#50827) Summary: Pull Request resolved: facebook#50827 This diff addresses a crash caused by view duplication in React Native Android. The issue occurred when a view was not already clipped and was laid out again, resulting in duplicated views. This problem was particularly noticeable when using nested FlatLists, which triggered a custom focus search with an incomplete and buggy duplicated FlatList container view. The fix involves preventing the duplication of views by checking if a view is clipped already before laying it out again. Additionally, this diff includes two other improvements: - Preventing clipping issues: When a view is nested within a non-ReactClippingViewGroup ancestor, focus searching would fail due to the needUpdateClippingRecursive logic only running on instances of ReactClippingViewGroup. By excluding these ancestors, we ensure that the next focusable view can be properly excluded from being clipped. - Minor fix: A minor fix was made to prevent potential issues in deeply nested cases. - Add a Kill switch with a feature flag and mobile config combo. For Facebook we can kill through MC and for all other apps we can kill with the feature flag default value Changelog: [Internal] Reviewed By: joevilches Differential Revision: D73213775
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D73213775 |
jorge-cab
force-pushed
the
export-D73213775
branch
from
d409b8a to
1cd4006
Compare
Contributor
|
This pull request has been merged in 5a9023f. |
Contributor
|
This pull request has been reverted by 4038183. |
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
2 participants
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.
Summary:
This diff addresses a crash caused by view duplication in React Native Android. The issue occurred when a view was not already clipped and was laid out again, resulting in duplicated views.
This problem was particularly noticeable when using nested FlatLists, which triggered a custom focus search with an incomplete and buggy duplicated FlatList container view.
The fix involves preventing the duplication of views by checking if a view is clipped already before laying it out again. Additionally, this diff includes two other improvements:
Differential Revision: D73213775