Editor: Always render the 'Switch to Draft' button to avoid focus loss #54722
Conversation
Mamaduka
added
[Focus] Accessibility (a11y)
|
Tricky one, obviously needs to be solved so thank you for the PR. It surfaces a shortcoming in this design, definitely — because it isn't ideal that "switch to draft" as a big honking always present button is visible on draft posts when it's entirely non functional. I suspect we need to revisit this entire panel and keeping this focus loss in mind so we can find a new design that covers all those bases. In the mean time, this does seem like the simplest path forward. The bigger issue here is the lack of a border on the disabled state of the secondary button style. Can we/should we update that to include the border, can even be lighter if need be. This is mainly to maintain the box grid. |
|
Size Change: -5 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
|
100% - It's not the prettiest solution, but it's a good intermediate one while working out the new design. The editor has other actions with the same issues, e.g., the "block toolbar" unlock button. It would be great to have a general design guide on handling this tricky a11y and design challenges. I also noticed a lack of focus styles. Do we have CSS/styles that I can reuse for this? |
|
The block toolbar is a difficult challenge I'd keep separate, there's a lot of nuance there.
There are a couple of mixins, but I'm curious why the focus style doesn't work on its own? Ideally it should be built into the button component itself. |
|
The Screenshot
|
|
That seems a bug in the core component, one to fix there, no? |
|
Yes, it's unrelated to the changes in this PR, so we can fix it separately and add a changelog entry to the components package. |
|
Its a design challenge to solve Gutenberg-wide. There are 2 basic rules.
The second is less obvious but still a WCAG failure. The first can lead to focus loss as this PR shows. What someone should figure out how to do is code some type of focus loss handler that always returns focus back to a constant point. Tricky part would be figuring out if its a true focus loss vs. the user moving away. At least until design and functionality learn to work together a little better in the UI. I tried it once but could never figure out how to tell if focus was totally lost or not. I have an active PR where I try to implement this for writing flow and I almost get a headache from how messy the code is. Real solution, do whatever you've got to do visually but never change the DOM to the point where React can't reconcile. |
|
Would it be feasible to use the status selection UI from the site editor here? |
|
@jameskoster, can you share the screenshot? |
|
This one: 
To be clear, I'm not suggesting we do that in this PR. Just noting that the site editor UI effectively solves this issue by omitting the "Switch to draft" button altogether. And since that UI already exists, perhaps it can be reused? |
|
Thanks, @jameskoster! The post editor already has a similar UI, so we should consolidate Status control. Does anyone remember the historical reasons why we had a separate button for the "Switch to Draft" action? Screenshot
|
The site editor 'status selection' UI actually mixes together Status and Visibility, which historically are two separate logical concepts. I'm not sure why they were grouped together to start with. Seems to me that's less than ideal and i'd recommend to keep those concepts logically and visually separated. Also, the status selection UI introduced a pretty visible inconsistency with the post editor, see #52643. I'm not sure how this kind of UI onconsistencies can be of any help for users. There is a proposal to better align some edit features in the Site and Post editors, see #52632 but still I'm not sure that mixing together status and visibility is the right way to go. |
|
Whether status and visibility are separate or not, the point was to use the same UI in both locations. But clearly that is distracting from this PR, apologies @Mamaduka!
I do not, but I remember it used to live in the main top bar, and was moved to reduce prominence: #50217. Perhaps we pursue this PR as-is, then follow-up to try and better align post & site editor document inspectors. I'd be happy to work on designs for that. |
Sounds good to me! Let's solve the immediate a11y issue; ensure the new design pattern we implement has a11y reviews before merging. If there are no other blockers for this PR, I would appreciate approval 🙇 |
|
The blue outline on hover looks strange. 
Should we fix that here or in a follow-up that targets the button component? It doesn't appear if the button is disabled rather than aria-disabled. |
|
@jameskoster, the |
|
@jameskoster, @jasmussen, do you have a mockup of how the disabled secondary button's focus/hover styles should look like? I can create a follow-up PR this week. |
|
Try settings I think @jasmussen said that the disabled button still should have a border - #54722 (comment). Screenshot
|
I don't have a strong feeling about that, but I will say that with a gray border / text the button begins to resemble an input field which isn't ideal: 
If we want to maintain the boxed shape then a background might work better. This more closely resembles the default browser style for disabled buttons too. 
Is If that's required then I expect the standard outline makes sense: 
|
Not via this prop. The component uses I've created #54978, which matches the rules for other buttons. |
|
If the button is disabled, the border does not need to meet any contrast ratios, it just needs to be focusable. So this is what I was thinking: The gray background lends prominence in this context which, wouldn't you agree, is unintentional for a disabled button? 
|
@jameskoster, basically use the same grey border when a disabled button is focused, right? |
|
The light border could work. Worth double-checking about the text colour though. Do disabled button labels still need to meet contrast guidelines? |
If I had to guess, I would say yes 😅 But I would defer to @afercia, @alexstine, and @joedolson on this one. |
|
No, disabled buttons do not need to meet contrast ratios. I'm happy to be educated if I'm wrong:
Emphasis mine. |
|
Looks like not.
https://www.w3.org/WAI/WCAG21/Understanding/non-text-contrast.html |
|
Thanks, everyone! @jameskoster, which light border/box shadow can I use here? Any existing example would be helpful. |
|
You can use the one I used, which is $gray-500 text and $gray-200 border. It would be good to do that too, this currently feels like it's floating a bit:
|
Was coming to report the same; looks a bit broken. |
|
I will push fix for this today. Just want to clarify one thing. We want secondary disable button to always have a border not only when it’s focused. Is this correct assumption? |
|
Secondary buttons should always have borders, whether focused or not, disabled or not. |
|
I'm a bit more confused now 😅 Do you mind if I leave this follow-up to you folks? |
What?
Fixes #51901.
PR updates the
PostSwitchToDraftButtoncomponent always to render to button, but using a disabled state when switching to draft isn't possible.Why?
See #51901.
How?
aria-disabledinstead ofdisabledprop.onClickcallback when the button is disabled.Testing Instructions
Testing Instructions for Keyboard
Same
Screenshots or screencast
CleanShot.2023-09-22.at.13.48.20.mp4