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): restore behaviour of RNSScreenStackAnimationNone #2565

Merged
merged 6 commits into from
Dec 16, 2024

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Dec 10, 2024

Description

There is still some header animation noticeable for some reason. <-- This is because we use fade transition with duration 0 and do not override interruptible animator! To prevent the animation we could either return nil interruptible animator (but overriding the method comes with it's own set of problems, see #2563 and other related) or handle the none animation much earlier, when calling showViewControllers:animated: in updateContainer (pass animated: NO).

PS: If we would want to pass animated: NO I wonder what would happen to dismiss prevention - we implemented it at the stage of the animation start... We need to think this through.

Note: Must be implemented with old animation API, because UIViewPropertyAnimator does not allow for 0 duration (it uses default if the specified animation duration is below some undocumented treshold).

This regression was introduced with #2477

Changes

We now use old API for animation: none & still rely on fade animation to implement it. Note the points made above ☝🏻 - we should refactor this code to make advantage of animated: parameter of the showViewControllers:animated:.

Test code and steps to reproduce

TestAnimation - set stack presentation to none - it works as prior to v4.

WIP VIDEO

Checklist

@matinzd
Copy link

matinzd commented Dec 11, 2024

Is this a fix for #2536?

@hirbod
Copy link
Contributor

hirbod commented Dec 12, 2024

@matinzd no, this is for supporting animation: none again, which is broken since v4, as it always fades.
Try to install react-native-screens 4.4.0-rc.0 to see if #2536 is fixed.

@kkafar kkafar force-pushed the @kkafar/fix-none-animation-ios branch from df7f1a4 to 567242b Compare December 13, 2024 10:17
@hirbod
Copy link
Contributor

hirbod commented Dec 16, 2024

Can we get the fixed none behavior shipped with another rc? Or do I have to patch?

@kkafar
Copy link
Member Author

kkafar commented Dec 16, 2024

I'll release it today's evening

@kkafar
Copy link
Member Author

kkafar commented Dec 16, 2024

I'm tracking now the "header animation", when the animation: none is set, but it seems that this behaviour was also present before v4 & must be fixed separately.

animation-none-bc-hr.mov

@hirbod
Copy link
Contributor

hirbod commented Dec 16, 2024

Yeah, that is great. Thanks for fixing the header mess, was always very unfortunate

@kkafar kkafar merged commit 178d94d into main Dec 16, 2024
5 checks passed
@kkafar kkafar deleted the @kkafar/fix-none-animation-ios branch December 16, 2024 12:28
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.

3 participants