Components: Remove Framer Motion from DropZone
#62044
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
fullofcaffeine
changed the title
DropZoneDropZone (WIP)
29 tasks
|
Size Change: +29 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 7a3a19a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9262608301
|
fullofcaffeine
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
2 times, most recently
from
52a38a4 to
4ef5979
Compare
|
This was trickier than I initially thought. :P After experimenting a bit further, I managed to make the animation based on the mount/unmount cycle of the inner
I'll experiment with animating the foreground separately tomorrow. As a related note, and It might be because I'm not very experienced with CSS animations, but one advantage I see with |
|
I think this is ready for a first review pass. I managed to implement an animation with CSS that is very close to the original, notice how the inner element (the up arrow and the label) animates in sequence and a bit later than the background: cssanimaton-latest.movIt also handles the reduced motion scenario just like before: reducedmotion-latest.movHere's the original animation when it was implemented with Framer Motion, for comparison: framer.movLet me know what you think! |
fullofcaffeine
changed the title
DropZone (WIP)DropZone
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
fullofcaffeine
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
from
b352016 to
80ccc35
Compare
fullofcaffeine
added
[Package] Components
fullofcaffeine
commented
Jun 1, 2024
fullofcaffeine
commented
Jun 3, 2024
fullofcaffeine
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
2 times, most recently
from
c838812 to
533fff5
Compare
fullofcaffeine
commented
Jun 4, 2024
tyxla
reviewed
Jun 6, 2024
There was a problem hiding this comment.
Thanks for working on it @fullofcaffeine 👍
I think this can be greatly simplified if we:
- Use CSS transitions instead of
@keyframes - Support reduced motion preference with CSS
Left some more detailed comments, let me know what you think.
fullofcaffeine
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
2 times, most recently
from
1c1eb1a to
f079dbf
Compare
fullofcaffeine
commented
Jun 8, 2024
|
@DaniGuardiola @tyxla I've changed it to use CSS transition and a media query to handle the reduced motion scenario. I also further simplified it by removing the Here's how it looks (and a screencast of the version in New implementationcss-transition-css-media-query.movTrunktrunk.mov |
fullofcaffeine
commented
Jun 8, 2024
| ( ( type === 'file' && onFilesDrop ) || | ||
| ( type === 'html' && onHTMLDrop ) || | ||
| ( type === 'default' && onDrop ) ), | ||
| 'is-dragging-over-document': isDraggingOverDocument, |
There was a problem hiding this comment.
I've removed these classes as I couldn't find where they were used. They don't seem to be needed for the component to function. If you know they are used somewhere, let me know, and I'll add them back.
There was a problem hiding this comment.
Seems like there is some 3rd party plugin usage in one of BuddyPress's component plugins:
There was a problem hiding this comment.
Added back those classnames
There was a problem hiding this comment.
is-dragging-over-document could possibly be unused, I didn't spot any usage when I was doing my wpdirectory.net search. Sounds good as a follow-up cleanup too.
tyxla
reviewed
Jun 17, 2024
| ( ( type === 'file' && onFilesDrop ) || | ||
| ( type === 'html' && onHTMLDrop ) || | ||
| ( type === 'default' && onDrop ) ), | ||
| 'is-dragging-over-document': isDraggingOverDocument, |
There was a problem hiding this comment.
Seems like there is some 3rd party plugin usage in one of BuddyPress's component plugins:
…ns later if used across components
…MaybeFade` animation orchestrator
ciampo
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
from
f079dbf to
f2f556e
Compare
ciampo
requested review from
DaniGuardiola and
tyxla
and removed request for
DaniGuardiola
|
@tyxla I updated this PR, tackling the outstanding comments and pushing some more changes with the goal of minimizing the diff with This PR should be ready for a new round of review |
ciampo
force-pushed
the
remove/framer-motion-from-components-dropzone
branch
from
1f13745 to
45b2d01
Compare
tyxla
approved these changes
Jun 19, 2024
There was a problem hiding this comment.
Looks and works great 👍 🚀
Thanks, @fullofcaffeine for the original work and @ciampo and @DaniGuardiola for finishing this one up 🙌
| ( ( type === 'file' && onFilesDrop ) || | ||
| ( type === 'html' && onHTMLDrop ) || | ||
| ( type === 'default' && onDrop ) ), | ||
| 'is-dragging-over-document': isDraggingOverDocument, |
There was a problem hiding this comment.
is-dragging-over-document could possibly be unused, I didn't spot any usage when I was doing my wpdirectory.net search. Sounds good as a follow-up cleanup too.
|
Apologies for not including the list of contributors to the the commit message — it seems like this change was introduced while I was away. I'll make sure I'll do so from now on |
ajlende
added a commit
that referenced
this pull request
Oct 1, 2024
ajlende
added a commit
that referenced
this pull request
Oct 2, 2024
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.
Project: #60975.
What
Remove Framer Motion from the component and implement the previous animation using pure CSS.
Why?
See the full list of reasons in #60975. TL;DR is better runtime performance, bundle size, and following web standards, which are often leaner and simpler to maintain.
How?
@wordpress/components.It might be acceptable to simplify he animation if it makes sense, provided everyone agrees with it.
Testing Instructions
Screenshots or screencast
I'll expand this section later with screencasts comparing the before / after. TBD