maintainVisibleContentPosition fixes on Android#46247
Open
janicduplessis wants to merge 1 commit intofacebook:mainfrom
Open
maintainVisibleContentPosition fixes on Android#46247janicduplessis wants to merge 1 commit intofacebook:mainfrom
janicduplessis wants to merge 1 commit intofacebook:mainfrom
Conversation
52 tasks
|
I have this same problem on Android. |
Collaborator
|
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
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
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:
We've noticed some cases of content jumping when using maintainVisibleContentPosition on Android when used for bidirectional pagination.
This makes some improvements to the maintainVisibleContent position implementation on Android to reduce cases of content jumping.
When using z-index Fabric re-orders the views so we can no longer rely on the ordering of the views to find the first visible view, we must go through all views and find the one that is bigger than the scroll position, but also has the smallest position.
This changes the approach to calculating the first visible view. Previously this was done in the Fabric
willMountItemslifecycle, but there were cases where it would result in an incorrect update to the first visible view. Instead this changes the calculation to happen on scroll, which is actually when the first visible view can change.This also does minor refactoring of the method names in MaintainVisibleScrollPositionHelper.java to better reflect the Fabric lifecycle names and what Android view lifecycle events they are called from.
Changelog:
[ANDROID] [FIXED] - maintainVisibleContentPosition fixes on Android
Test Plan:
Tested in the Expensify app to make sure this reduces cases of content jumping in the chats while scrolling and items are added at the start of the list on Android.