-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try adding text labels to block inspector icon buttons. #46299
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +133 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Thank you for your tireless efforts here! But I think it's important to keep the main goal of "show text labels" in mind: which is to increase legibility options. In some places, that means just replacing icons, but in other cases we have to consider the available space to avoid text getting cut off or overlapping, or respect important spacing and margin considerations affecting the context of a single segmented control that wasn't designed to fit text labels in the first place. In some cases, such as a segmented control, an appropriate replacement migth be a dropdown. In other cases, it can be necessary to rephrase and simplify labels, not just to fit but to balance the verbosity to consider dyslexics. For example, "Unlink" when shown in context of a "Radius" control almost certainly carries sufficient context, so the "radii" can be removed. Similarly, "Options" is also probably sufficient, when shown in context of the "Border" panel, so it doesn't have to say "Border options". Mostly, I think it's important we don't think of the above considerations as decorative, but actually as functionally crucial for the effort. Despite best intentions, I doubt we can ever consider #15311 fixed until those considerations are taken into account. Would it be more sensible to start with some of that functionality before we roll it out? |
Thanks for the feedback @jasmussen !
That would mean making segmented control display as a dropdown only when text labels are active, right? That should be fine, we did something similar when working out text labels for the top toolbar (though after a while the dropdown was removed as the controls changed around a bit).
I think we should split out any work that touches the components package (pretty much anything in the tools panels). I opened this PR mostly as a conversation starter, so we can look into what exactly needs to be done 😄 but we can potentially use it to look at the non-tools panel stuff (such as the block variations for Group block). |
Thanks for this PR @tellthemachines! Things are working as described. I've noticed a few items where text length affects presentation slightly, but I understand this is fairly unpredictable, especially taking into account i18n. One thing I noticed however is that the "Block Settings" button disappears in narrow viewport widths. This is an issue on trunk, so not related to this PR. I just thought I'd note it. Here's the icon to the right of "Publish" and to the left of "Options": The text equivalent however is not present: |
Correct.
The work is all valid, appreciated, and something we need to get to at some point. The only tricky thing is to make sure each step we merge is an improvement on what's there. This PR will be once we have components able to handle the change in appearance, which it seems they aren't quite ready for yet. |
For components in the We could then discuss how each component should handle this requirement, keeping into account layout constraints as @jasmussen mentioned. |
Why?
In order for #15311 to be fully fixed, the "Show button text labels" preference must replace all interactive icons in the interface with text. This PR tries to enable this functionality for the icon buttons and toggles in the block inspector.
How?
As a first approach, this PR blanket adds the CSS rule that hides SVG icons and adds their aria-label as text inside an
:after
pseudo-element. Visually, the results are not always great, as can be seen in the screenshots below. We need:[class*="isIconStyles"]
is only meant to give us a preview of the effects of this change - ideally we find a less brittle way to do this - cc @ciampo @mirka )Testing Instructions
Testing Instructions for Keyboard
Keyboard testing should work the same; Shift+Tab from the editor canvas when a block is selected should reach the "Options" button directly. Note that with text labels enabled there should be no difference for screen reader users in interacting with icon buttons (the text label being added is already their aria-label, and screen readers should ignore the text content of the pseudo element).
Screenshots or screencast