Skip to content
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

Stylebook: icon doesn't respect text-only buttons option #48051

Closed
joedolson opened this issue Feb 13, 2023 · 8 comments · Fixed by #48088
Closed

Stylebook: icon doesn't respect text-only buttons option #48051

joedolson opened this issue Feb 13, 2023 · 8 comments · Fixed by #48088
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@joedolson
Copy link
Contributor

joedolson commented Feb 13, 2023

Description

When the "Show button text labels" preference is enabled, the Stylebook icon remains an icon. This is particularly important because the StyleBook is a pretty unique feature and the icon has no common meaning.

@annezazu

Step-by-step reproduction instructions

  1. Go to Options > Preferences
  2. Check 'Show button text labels'
  3. Go to the Styles panel and see the Stylebook icon, not a text label.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@joedolson joedolson added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [a11y] Labelling labels Feb 13, 2023
@tomdevisser
Copy link
Member

Good point @joedolson. Seems like it does have an aria-label present, the only thing the button is missing is the ::after pseudo with content: attr(aria-label).

We can't add it for all the buttons in this div though, or you get this.
Screenshot 2023-02-14 at 6 13 41 PM

@joedolson
Copy link
Contributor Author

I do think that the "all or nothing" nature of the Show button text labels is a problem. The interface has different types of icons: there are some that are extremely common icons, and others that are very unique. Something like the Storybook icon really needs to appear in text mode; but I'm not sure it's a benefit to show all icons in text mode. That's probably a discussion for a different issue.

The aria-label values for these should probably be shortened to just "Stylebook" and "Close Styles", though. I'm not sure why they're so verbose.

@tomdevisser
Copy link
Member

tomdevisser commented Feb 14, 2023

@joedolson I understand how this might seem verbose, but I think buttons need to convey the action as well. Are you editing the stylebook? Are you deleting it? Opening it? This is valuable information in an aria label.

@joedolson
Copy link
Contributor Author

The button should already convey that information via state; there are aria-pressed or aria-expanded attributes on the buttons that indicates current state. Duplicating that information can be confusing. Right now, the 'Open stylebook' button will read "Open stylebook not pressed" when the stylebook is closed and "Close stylebook pressed" when the stylebook is open. The first announcement is clear, but the second is very much not.

When using aria-pressed, the label should not change; this creates confusing descriptions as above.

I'd be fine with either just 'Open stylebook' or 'Stylebook' with the "open" action implied. We aren't using open for the Styles panel, Options panel, or other standard interactions, and I don't really see why we need that here.

@tomdevisser
Copy link
Member

Ah, okay, thank you for clarifying @joedolson. I've been taught a different approach, but your explanation makes sense. In that case "Stylebook" would be great! 🙂

@annezazu
Copy link
Contributor

Calling on a crew to dive in and adding to the 6.2 board @tellthemachines @apeatling @andrewserong.

@andrewserong
Copy link
Contributor

Thanks for opening this issue, and for the discussion! I'll have a go at a fix for this one, so I've assigned myself.

@andrewserong
Copy link
Contributor

I've put up a PR to try to simplify the labels a little and expose them as text labels when the 'Show button text labels' feature is in use. Happy for any feedback over there: #48088

@andrewserong andrewserong moved this to 🏗️ In Progress in WordPress 6.2 Editor Tasks Feb 15, 2023
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.2 Editor Tasks Feb 16, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants