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 dynamically added/removed animated styles on iOS/Android #5268

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

tjzel
Copy link
Collaborator

@tjzel tjzel commented Oct 18, 2023

Summary

Fixes #5296.

  1. Since this is potentially a JS-heavy fix, I'll gladly listen to suggestions and receive help with how to profile it.

  2. I personally dislike how those changes look like. I would much more prefer to simplify some logic present in createAnimatedComponent and its devil spawn, reducing some code, instead of building on top of it - although I'm not sure if we need it at the moment.

Just watch the video to understand what is happening:

BEFORE (buggy)AFTER (desired)
Screen.Recording.2023-10-18.at.19.42.02.mov
Screen.Recording.2023-10-18.at.19.35.24.mov

Test plan

💣

Note

JS-Reanimated (Web implementation) still has those issues and I will produce a separate solution for it in a finite amount of time.

@Latropos
Copy link
Contributor

Latropos commented Nov 6, 2023

Wanna add "closing #5296"?

app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Show resolved Hide resolved
app/src/examples/DynamicStylesExample.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@bartlomiejbloniarz bartlomiejbloniarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧨

src/createAnimatedComponent/PropsFilter.tsx Show resolved Hide resolved
@tjzel tjzel added this pull request to the merge queue Mar 19, 2024
Merged via the queue into main with commit 74985c3 Mar 19, 2024
7 checks passed
@tjzel tjzel deleted the @tjzel/fix-dynamic-styles-on-device branch March 19, 2024 10:01
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2024
## Summary

Unfortunately, turns out the PR with dynamic styles #5268 introduced
more problems than it solved. Thus, proposing a revert.
This revert most likely fixed:
-
#6203
-
#6102
-
#6037

as well as one more not-published issue and potentially a few more.

## Notes

Even if it gets approved, to be merged ONLY after @tjzel agrees with
everything

## Test plan

😢 😭
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.

Re-rendering component with inline SharedValues causes style loss
4 participants