Block Editor: update react-spring to 9.2.4 #30979
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
the
[Type] Bug
youknowriad
added
the
Browser Issues
|
Can we get some testing here. It would be cool to get this issue fixed for 5.8 |
|
I'm still able to reproduce the original issue. Tested on macOS Big Sur / Firefox 88 CleanShot.2021-05-05.at.15.12.42.mp4 |
|
Also looking at the tests, it seems this PR might not be building properly. |
gwwar
force-pushed
the
fix/ff-animations
branch
2 times, most recently
from
2e67823
to
6edb026
Compare
|
The |
|
So with FF + React Spring 9.1.2 animations do work, but it looks like there's quite a bit of jank going on when v9.1.2.mp4 |
|
Size Change: +7.81 kB (+1%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
|
@talldan how do we enable the block navigator movers UI (or is this no longer exposed?) #18014 @youknowriad do you know who'd be good to ping to double check what's going on with package.json and source map issues? |
youknowriad
reviewed
Jun 2, 2021
packages/block-editor/package.json
Outdated
| "@react-spring/core": "^9.1.2", | ||
| "@react-spring/shared": "^9.1.2", | ||
| "@react-spring/types": "^9.1.2", | ||
| "@react-spring/web": "^9.1.2", |
There was a problem hiding this comment.
Why do we need all these dependencies here since we're only using the "web" package directly?
There was a problem hiding this comment.
We probably don't. I was having trouble getting a plain react-spring import working vs pointing at the web package. I can revisit.
|
Sorry missed the comments here. I think @gziolo would be the best person for package and package-lock related things |
🙈 So when 'privacy.resistFingerprinting' is true FF will reduce the javascript time precision to 100 ms!
https://developer.mozilla.org/en-US/docs/Web/API/Performance/now#reduced_time_precision We might need to test if we need to add the following headers to enable higher precision:
|
gwwar
force-pushed
the
fix/ff-animations
branch
from
efec104
to
c32727d
Compare
gwwar
changed the title
|
This one is ready for review. Dusting this one off since it may be harder to replace the last two usages of React Spring in favor of framer at the moment. (We'll need other performance improvements to do so). |
gziolo
reviewed
Sep 13, 2021
| "redux-multi": "^0.1.12", | ||
| "rememo": "^3.0.0", | ||
| "tinycolor2": "^1.4.2", | ||
| "traverse": "^0.6.6" | ||
| }, | ||
| "dependencies": { |
There was a problem hiding this comment.
I'm not sure why it is here. It looks like it can be removed.
There was a problem hiding this comment.
Good catch, updated in 317f1d8.
The only dependencies for web should be the following:
Maybe it was a stray item from when I was testing other imports. To fix this, I removed the entry for "@react-spring/web": "^9.2.4", in package.json, npm install, then added it back with another npm install and it didn't appear to return.
gwwar
force-pushed
the
fix/ff-animations
branch
from
c32727d
to
317f1d8
Compare
gziolo
approved these changes
Sep 13, 2021
|
Thanks for the review @gziolo I'll land once tests are green |
Thanks, I ended up needing to restart almost all the jobs. I'd be curious to see what the bug turns out to be for the cb issue. It smells like a cache issue, but sounds like it may be on the network layer. Have folks reproduced this on a local docker env? |
The jobs that fail have npm cache primed, so it’s rather related to installation process. One thing we suspect is related to forked packages that use git urls. Anyway, it might be everything 😅 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #32398
This PR updates react-spring to 9.2.4. This should fix overlapping blocks in FF #19476 when
privacy.resistFingerprintingis set to true inabout:config. Note that whenprivacy.resistFingerprintingis enabled, animations will not be as smooth due to the reduced time precision 100ms or more (https://developer.mozilla.org/en-US/docs/Web/API/Performance/now#reduced_time_precision).There are a few breaking changes from React Spring 8 -> 9, most notably in the refactor of useTransition and rename of onFrame -> onChange. We'll want to verify that block moving animations still work in the list view and editor block list.
https://aleclarson.github.io/react-spring/v9/
https://aleclarson.github.io/react-spring/v9/breaking-changes/
Testing Instructions
CleanShot.2021-09-09.at.14.48.56.mp4