Distraction Free: Unify the header animation #62167
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] Code Quality
|
Size Change: +2 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
youknowriad
commented
May 31, 2024
| @@ -292,37 +281,13 @@ export default function Editor( { isLoading } ) { | |||
| } | |||
| ) } | |||
| header={ | |||
| <AnimatePresence initial={ false }> | |||
| { canvasMode === 'edit' && ( | |||
| <motion.div | |||
There was a problem hiding this comment.
The main purpose of this PR is to move this animation within the "InterfaceSkeleton" component because the header is also animated there. So if these two animations are in two different places, they may cause conflicts (for instance the hover animation within the skeleton might trigger while the other animation is in progress...)
|
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. |
|
It's possible this is on my end, but I'm seeing an issue both in trunk, and this branch, where the top toolbar simply doesn't reappear when I move towards the top of the screen:
|
|
@jasmussen yes, that's the issue that the other PR solves #61579 but while solving that particular bug, we revealed another bug that was discovered on Firefox in the other PR by @draganescu So the current PR is actually a fix for that hidden Firefox bug but you can consider it a refactoring PR for now:
|
|
Oh yes, thanks for restating that. Still caffeeinating. Here's a before:
And after:
They are close, except in the "after", the actual content of the toolbar animates down after a bit of a delay. Can that happen at the same time as the fade? FWIW, I'd change both animations, so that the whole bar animates down, with no fade, rather than just the contents of the bar. |
|
I removed the delays for the child animations. It's still not 100% the same as before but feels close. |
|
Very close, animation looks good:
The bounce makes the contents of the toolbar briefly break the border, if you look closely. Easiest is probably to remove the bounce on the contents when they animate down. The height of those contents also need to be reduced by 1 or 2 px, probably related to @richtabor's recent PR to unify the borders: |
|
Ok, I removed the bouncing. I don't think I've touched the heights in this PR, so that's separate right? |
draganescu
approved these changes
May 31, 2024
|
You're right, the half-pixel height offset is an issue in trunk as well. No blocker. |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
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.
Related #52632
Extracted from #61579
Requirement for #62146
What?
This PR tweaks the DFM animation just a little bit in the post editor and also moves the appearance animation for the Header of the site editor to the InterfaceSkeleton component. The idea is that we now animate the header in a single place. (It's fine to keep its children animations though).
Testing Instructions
1- Check the animation of DFM in the post editor is good enough (the site editor is still broken like trunk)
2- Check that the header appears properly (outside DFM) when clicking the frame in the site editor.