Navigation: Try adding minimal animation to overlay. #43851
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
jasmussen
added
[Type] Enhancement
|
Size Change: +426 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
javierarce
approved these changes
Sep 5, 2022
|
It seems fine to me as well. |
|
🙏 |
|
Thanks. It looks great, but isn't the fade out option missing when the menu closes? |
|
That's a good idea. I'll create a followup PR to add that. |
|
I think in this animation it's better to use transformation This is just a suggestion, but I have CSS performance in mind. |
|
I'll keep that in mind for the PR! |
|
Alright, I created #44082 as a followup to this one to use translate. I did experiment with a fade-out animation, but of course this was not as trivial as I had thought it to be, because the way the modal closes at the moment, the entire overlay is gone before any animation has a chance to fire. So without a bigger refactor I couldn't immediately see a way to do that. @Gierand, if you have an idea for how to do it, or if you're even able to make a PR, I'd be happy to review it or follow up. |
|
@jasmussen, when there is |
|
Yes indeed, and from those options it sounds like JS would need to be the solution in order to maintain accessibility. I definitely think we'll get to that, but probably not in the near term. |
82 tasks
This was referenced Oct 5, 2022
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.
What?
Fixes #43524.
This PR does two things:
The animation is fast, 0.1s, in order for it to feel generic and actionable, but still intentional. It's hard to see in this GIF:
Here's a video variant with hopefully better framerate:
Screeny.Video.5.Sep.2022.at.12.29.45.mov
Testing Instructions
Insert a navigation menu, ensure it collapses to an overlay. Open the overlay and test that it animates upwards and fades in.