Conversation
kkafar
commented
Apr 2, 2025
Member
Author
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
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 | ||
| } |
Member
Author
There was a problem hiding this comment.
This fails in case the formsheet has been preloaded. Described on internal board.
kkafar
changed the title
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.
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
fitToContentssheets.Changes
This PR extends the enter transition postponing mechanism to all
sheetAllowedDetentsvalues.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.onLayoutcalls - resulting in flicker effect.Test code and steps to reproduce
Was not able to reproduce the issue on our end :/
Checklist