-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
fix(Android): separate transitions of sheet and dimming view #2542
Merged
kkafar
merged 48 commits into
main
from
@kkafar/separate-transition-for-sheet-and-dimming-view
Jan 31, 2025
Merged
fix(Android): separate transitions of sheet and dimming view #2542
kkafar
merged 48 commits into
main
from
@kkafar/separate-transition-for-sheet-and-dimming-view
Jan 31, 2025
Conversation
This file contains 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
Transition progress is not yet supported!
container This code was necessary when I tried to use Android Transition API. Currently, with custom animators this is not required.
I simplified the implementation. It now relies on `onViewAnimationStart` / `onViewAnimationEnd` callbacks of the fragment wrappers.
kkafar
commented
Jan 31, 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.
Okay, the code changes look good now. I'm gonna drop this in beta of 4.7.0
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
This PR allows the form sheet on Android to be dismissed using
slide-down
animation. Currently the sheet fades-out together with the dimming view due to using regular Tween animations, which apply animation whole view hierarchy from the fragment's root view down - this does not allow for separate animations for the sheet and dimming view.Transition API
was considered as an implementation measure, however due to numerous encountered problems, most prominent example being "disappearing shadows during pop transitions", I rejected it.The implementation settled on using custom value / object evaluators. This approach comes with its own series of issues, with the most prominent one:
Caution
This PR changes current default animation for formsheets on Android. This is potentially a breaking change. I haven't decided yet whether to treat is as a fix or hide this new animation behind some prop.
WIP recordings of the changes.
Changes
setCustomTransition
) defined inScreenStack.onUpdate
.DimmingFragment
as there is no longer "nested fragment strcuture". Dimming view is now attached directly into hierarchy (under coordinator layout) w/o hosting it in separate fragment.Note
During testing I've noticed that for some reason the form sheet screens do not receive
onWill(dis)appear
events on navigator. This is something to investigate, however it seems that this is not a regression.Test code and steps to reproduce
TestFormSheet
/TestAndroidTransitions
examplesChecklist