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

feat: remove system animations on Android #1213

Merged
merged 23 commits into from
Jan 26, 2022

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Nov 17, 2021

Description

It looks like react-native v0.66.0 updated their Android appCompat library to 1.3.0, it also upgraded fragment library to 1.3.4, which in turn removed the behavior of dispatching onAnimationStart and onAnimationEnd in Screen.kt for system animations (default, fade, none). Since the custom animations work correctly, we added the proper xmls for system animations and completely removed the code managing the previous ones.

We should check if the newly added xmls behave in the same way as the former system animations, especially the default one, since it is the most advanced.

stackAnimation: 'none' xml has the duration set to 20ms since it helped with the correct dispatching of events in nested stacks. See https://github.com/software-mansion/react-native-screens/pull/1213/files#diff-7be7173f0a5459770b0c152de49f96ef4ee70923a7b223a039b921529f2396d4R275 for a little more info.

Changes

  • Added xmls reacreating system animations
  • Removed code managing applying system animations
  • Removed code managing appearance events for system animations

Test code and steps to reproduce

Test593.tsx for check if events still work properly.

Checklist

  • Included code example that can be used to test this change
  • Ensured that CI passes

@Guuz
Copy link

Guuz commented Nov 22, 2021

This sounds good! the current default screen transition on android is not as it should be. it also doesn't follow the animation scale/speed you can change in the OS. does this also fix that?

@hirbod
Copy link
Contributor

hirbod commented Nov 24, 2021

This sounds good! the current default screen transition on android is not as it should be. it also doesn't follow the animation scale/speed you can change in the OS. does this also fix that?

I don't think that OS parameters are considered here. As far as I understand the code, the XMLS files are used (with fixed values). So it won't reflect any system OS variables like reduced motion or speed.

@kacperkapusciak kacperkapusciak linked an issue Nov 29, 2021 that may be closed by this pull request
7 tasks
@elan
Copy link

elan commented Dec 25, 2021

FWIW I gave this is a try on Android with React Native 0.64.2 and native stack navigator encountered a weird glitch where it seemed like 50% of the time, the transition didn't end properly, and stayed in a dimmed out state, as if perhaps the alpha level didn't properly end up as 1.0.

@WoLewicki
Copy link
Member Author

@elan can you provide a simple repo with it and maybe a video where it happens? It would greatly help us test it.

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

Successfully merging this pull request may close these issues.

Zoom animation in Android feels too quick and elastic
5 participants