-
-
Notifications
You must be signed in to change notification settings - Fork 576
fix(Android,Paper): fix flicker during formsSheet enter transition on slow Android devices - not preloaded formSheet #2820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
kkafar
commented
Apr 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment for future me.
Comment on lines
+139
to
+152
fun Screen.requiresEnterTransitionPostponing(): Boolean { | ||
// On old architecture the content wrapper might not have received its frame yet, | ||
// which is required to determine height of the sheet after animation. Therefore | ||
// we delay the transition and trigger it after views receive the layout. | ||
// This is used only for formSheet presentation, because we use value animators | ||
// there. Tween animations have some magic way to make this work (maybe they | ||
// postpone the transition internally, dunno). | ||
|
||
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED || !this.usesFormSheetPresentation()) { | ||
return false | ||
} | ||
// Assumes that formSheet uses content wrapper | ||
return !this.isLaidOutOrHasCachedLayout() || this.contentWrapper?.isLaidOutOrHasCachedLayout() != true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails in case the formsheet has been preloaded. Described on internal board.
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.
Description
On old architecture we have "enter transition postponing" mechanism, however it has been applied only to formsheets with fit to contents. It turned out that on very low-end Android devices the issue affects also not
fitToContents
sheets.Changes
This PR extends the enter transition postponing mechanism to all
sheetAllowedDetents
values.Important
There still remains problem though, when the sheet is preloaded before navigating to it. The content wrapper receives the layout from RN, however the screen never receives the frame, because it is not initially attached to the container (and native layout triggered from screen stack does not reach it).
Then, when when the transition starts, content wrapper is already laid out, however the screen is not & FOR SOME WEIRD reasons it receives many frame updates during the transition - animated value updates are interleaved with
Screen.onLayout
calls - resulting in flicker effect.Test code and steps to reproduce
Was not able to reproduce the issue on our end :/
Checklist