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

fix(iOS): fix navigation item animation #2492

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Nov 12, 2024

Description

Fixing navigation item animation on iOS. It will use default EaseInOut curve, instead of spring.

(WIP paste a video)

Note

Possible improvements

  • Expose opportunity to customize header transition curve
  • Expose opportunity to customize animation completion curve (after gesture cancel)

Caution

It was noted by @hirbod that the back button flashes when clicked and go-back-animation starts.
Below you can see video of this bug.
This happens because:

  1. Back button, when clicked, fades out a bit to give haptic feedback,
  2. Back button, when clicked, triggers go-back-transition.
  3. Both animations add-up, resulting in this 'flickering effect'.
Screen.Recording.2024-11-12.at.14.19.10.mov

I wonder what should be the solution to above problem ^ (WIP tag people here). I see three paths:

  1. Throw away these changes and restore old header behaviour: B's navigation item fades out completely and just then A's navigation item starts fading in - this prevents this "bug",
  2. Go with this behaviour? (this is noticible though :/)
  3. Override the animation of the back button. Should be doable with obj-c: I could find playing animation on the layer, remove it & add custom one; this will require some debugging time though & I see couple
    of technical issues to pull this off.

Changes

Added RNSViewPropertyAnimatorCompositor which proxies and fans out system callbacks to series of animators
and exposes implicit animator (returned as interruptible animator from stack animator) (the system animates navigation items with curve of interruptible animator).

This is how it looks now:

Screen.Recording.2024-11-12.at.14.20.51.mov

Test code and steps to reproduce

Test1072.

Checklist

@WoLewicki
Copy link
Member

I'm very scared of the increasing complexity of the code regarding the animations 😅 I think at this point we need to have a test case where you can try all of the combinations of setting different screen transitions to be able to test all of the possible combinations. E.g. is the default animation of the header still working correctly with this PR?

@kkafar
Copy link
Member Author

kkafar commented Nov 13, 2024

I'll prepare an enhanced test case.

I'm very scared of the increasing complexity of the code regarding the animations

Partially agree, but there is more code because we want to achieve different effects - animate header and screen itself with different timing curves + synchronize these animations on gesture etc. There is no UIKit abstraction for this, had to come up with my own.

@kkafar kkafar force-pushed the @kkafar/fix-header-animation branch from e2e24ee to fccb540 Compare December 9, 2024 14:00
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.

2 participants