Skip to content
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

Shared Element Transitions for Modals #6647

Closed
5 of 7 tasks
mrousavy opened this issue Oct 6, 2020 · 8 comments
Closed
5 of 7 tasks

Shared Element Transitions for Modals #6647

mrousavy opened this issue Oct 6, 2020 · 8 comments

Comments

@mrousavy
Copy link
Collaborator

mrousavy commented Oct 6, 2020

Intro

Currently Shared Element Transitions (SETs) only work for Screens by using push and pop commands. This is a bit weird, since there's a lot of issues with using custom screen animations, such as the top bar flickering and the bottom tabs sliding out. With Modals, we don't have those issues anymore while providing extra flexibility, such as showing the parent screen beneath, blurring the parent screen, etc.

TODO

  1. Also create Shared Element Transitions for showModal and dismissModal commands for iOS (almost done in Shared Element Transitions for Modals! #6625 thanks to @yogevbd)
    • Fix new animations not being applied when passing them using mergeOptions in dismissModal
    • Fix initial scale/transforms not being applied causing noticeable flickering (see this demo)
    • Fix borderRadius animations not being applied (also see this demo)
    • Fix last tick in SET being a bit too far, maybe the initial from scale/transform is being applied but has been reset to 1 in the meantime (see this demo)
  2. Also create Shared Element Transitions for showModal and dismissModal commands for Android (done in Support shared elements transition in show/dismiss modals #6716)
  3. Update JS API (types) to add support for sharedElements object in showModal and dismissModal (done in Shared Element Transitions for Modals! #6625)

Demo

Here's a smooth demo to showcase what it could look like:

@mrousavy
Copy link
Collaborator Author

Here are some demos of the current implementation:

  1. imgur
  2. imgur

What do you think @guyca @yogevbd ? ;)

In some cases, the drag down animation still doesn't work 100%, since it shortly flickers back to it's original position, instead of the scaled down one. Other than that, it's perfectly smooth on iOS.

That leaves us with TODOs:

  • Android
  • that one flicker bug on iOS
  • and mergeOptions not working for modal dismissal. (I'll look into that one)

@guyca
Copy link
Collaborator

guyca commented Oct 22, 2020

hmmm how often does the drag-flicker reproduce?

@mrousavy
Copy link
Collaborator Author

@guyca on iOS about 1 out of 3 times... 😥

@guyca
Copy link
Collaborator

guyca commented Oct 22, 2020

And reproduces in the Playground, right?

@mrousavy
Copy link
Collaborator Author

Last time I checked it did, I'll check again in a sec 👍

@mrousavy
Copy link
Collaborator Author

mrousavy commented Oct 22, 2020

@guyca @yogevbd I noticed another bug while playing around in the playground: When the to view has no border radius (border radius 0) it somehow bugs out. Two scenarios:

  • Using CarStoryScreen.tsx as it is in master: demo (imgur)
  • Using CarStoryScreen.tsx as it is in master, but add borderRadius: 1 to the background style: demo (imgur) (also, here's another demo of that, but you can see the flicker issue)

I've tried debugging it as far as I can, and I noticed that it never adds the CornerRadiusAnimator.h to the SET animators. So at some point, either the view.location.fromCornerRadius or view.location.toCornerRadius is calculated wrong.

@guyca
Copy link
Collaborator

guyca commented Nov 11, 2020

@mrousavy shared element transition in modals is supported on iOS in 7.20 and Android support was introduced in 7.3.0. Are we good to close this issue?

@mrousavy
Copy link
Collaborator Author

@guyca oh yeah of course! the flickering bug is tracked in another issue, so this can be closed. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants