Tabs: add vertical indicator animation #62879
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
|
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. |
DaniGuardiola
added
[Type] Bug
|
Size Change: -268 B (-0.02%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 3fc6096. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9682244307
|
jasmussen
approved these changes
Jun 27, 2024
There was a problem hiding this comment.
Impressive amount of red code changes considering what an improvement this is. Works great:
I checked and validated that this animation respects the operating system set "reduce motion", it does, good.
I don't know how much more of a code review this needs, it works well and appears safe. Nice work!
|
Nice to see this one! |
andrewserong
added
[Feature] UI Components
|
@DaniGuardiola Thanks again for this PR. It seems like there was an unintended side-effect, however! This Inserter > Patterns list of tabs is apparently also using the tablist component, meaning the gray backdrop is now present behind the light blue style it's meant to have. What would be a good way to address this? Should the custom styles for the inserter-usage of TabList be refactored to set a light blue background on the backdrop, instead of on the button item itself? LMK, happy to help. |
|
Thanks for the heads up @jasmussen. In line with the best practices we're trying to achieve regarding the components package (mostly aimed at reducing custom style overrides that cause breakages and high maintenance costs down the line), I would suggest considering the following in order of preference:
If none of these three options work, we can always address this as a regression and tweak the custom styles as you propose, though I think it's a good chance to address it at a more fundamental level. Of course, that doesn't merit blocking a fix so happy to take the simpler route in the short term. Let me know what you think from a design standpoint :) |
|
I think there might be a regression to fix for the immediate color disparity, the gray + blue overlay is poor. However your point is well made. I wonder, would the light-blue style work for the preferences modal @WordPress/gutenberg-design ? |
|
As Dani points out, these regressions are actually a good opportunity to see if we can make our UI more coherent across Gutenberg, avoiding overrides and sticking as much as possible with the default component's look! |
|
I see no reason for the active tab styles to differ between the Preferences modal and the Patterns Inserter, or any other (vertical) tabs for that matter. I'd love to see more coherence across. The light blue works for me. Thinking about #62020 (comment) I could even go a step further and suggest that the orientation shouldn't affect the styling, but that's a separate discussion. |
|
Excellent conversation. We want the gray backdrop fixed for 6.6 as it's a really poor regression (my bad!). So I wonder: what's the best way to get that fixed? I assume that actually fixing this at the component level is not going to be possible. Another option is to revert the animated backdrop for the 6.6 branch, and fix this for the 6.7 branch. Any thoughts? |
|
If the patterns inserter is the only exception, we could also add some temporary, targeted style overrides that disable the gray backdrop only for that part of the UI (to be cherry-picked for 6.6) and in the meantime agree + implement a universal backdrop style that works for all tabs across the editor (to be merged in |
|
Sounds fine if we believe that is okay to backport. We're pretty late in the RC phase, there are special rules. Perhaps @Mamaduka might know? |
|
If the bug was introduced during the WP 6.6 release cycle, it could be fixed in RCs, but I don't think there are any more scheduled RC releases. The target for the WordPress 6.6 release is next Tuesday, July 16, 2024.. But we can target the next minor release. I'm unsure if it's related, but vertical tabs animations can be a bit "junky" when navigating pattern categories. ScreencastCleanShot.2024-07-10.at.11.18.19.mp4 |
I noticed that too, and my hunch is that the jankiness is caused by the pattern previews being quite intensive to render (and not to the |
This was referenced Jul 10, 2024
|
Disabled the indicator animation in the patterns inserted in this PR #63352 Let's start thinking about the next steps too since ideally we shouldn't be keeping that custom style override around:
|
|
@Mamaduka depending on the technical details of this, we might be able to solve that with a react transition in the right place. Either in the Tabs component, as a generic solution (ideal, but a bit unlikely) or in this specific instance (a bit more likely to work). |
|
A couple of updates, a PR I'm working on (#64371) fixes:
|
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?
Added an animated vertical tabs indicator (related: #60560 (comment)). I also removed the style overrides from one instance (block editor preferences dialog) where it was a complete match. Tested other overrides and they look correct to me.
How?
Using the existing animation system.
Testing Instructions
Check out the Storybook story for vertical tabs, or instances of vertical tabs in Gutenberg.
Testing Instructions for Keyboard
Focus it and use up/down arrow keys.
Screenshots or screencast
tabs.mp4
Preferences dialog - before:
before-tabs.mp4
Preferences dialog - after:
after-tabs.mp4
With
prefers-reduced-motion:tabs-reduce-motion.mp4