Fix PostPublishButton toggle #13194
Fix PostPublishButton toggle #13194
Conversation
|
Not sure what milestone should I use for this bugfix. |
|
With the changes here, the button would still be focusable by tabbing, which would be unexpected for a disabled element. |
|
I think a similar fix landed? The fact that the button stays tabbable when it's "disabled" property changes is expected to avoid focus loss. All the buttons in the toolbar behaves this way IIRC. |
Which focus loss? A disabled button is not meant to be tabbable, so I don't find the behavior to be expected. If it's specifically about the focus return behavior for the publish panel, then (a) isn't the button not being disabled a prerequisite for the panel to have been opened in the first place, and (b) it's an implementation detail which shouldn't impact the expected interaction of disabled/enabled button tabbing order. |
|
It's about the button becoming disabled when it's currently focused. Say, you're current on the publish button and for some reason (ajax call or something), the post is not saveable anymore, if we disable the button, the focus will be lost and moved to the body forcing people to tab all over again. That's how I understood it at least. cc @afercia |
|
Correct. Note that modern browsers try to keep focus "in place". That's what, for example, Chrome has implemented in version 50 and called https://bugs.chromium.org/p/chromium/issues/detail?id=454172. However, don't let that fool you: focus is actually lost, as the previously focused element just became unfocusable. Also, there's no guarantee browsers are still able to keep focus in the sequential order when a screen reader or other assistive technology is used. I've seen that failing often. Besides technical details, the ARIA Authoring Practices allow for focusability of disabled controls in some cases, see https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_disabled_controls. Seems to me the most important recommendation they do is to adopt a consistent set of conventions. |
Interesting, thanks for sharing. It's a bit unclear to me from the text whether tabbability and focusability are meant to be interchangeable, especially considering their example is one which uses arrow keys to navigate disabled items. As far as it impacts discoverability, I guess I'd agree that having these disabled elements be focusable would be a benefit. I think consistency will be the challenge though, since it requires more conscious consideration in deciding whether a disabled element qualifies to be focusable. |
|
Yep, consider the ARIA Authoring Practices refer mainly to ARIA widgets (toolbars, and so on) but the general principle of keeping "disabled" element focusable is applicable in other cases too, when it's a benefit.
Absolutely. It should be evaluated on a case by case basis. |
Because we're using aria-disabled instead of the disabled property the button is clickable. We need to back off from firing the onToggle event if the toggle is disabled.
oandregal
force-pushed
the
fix/publish-button
branch
from
4f25a7f to
390ccf3
Compare
|
I saw there was some reorganization since I created this PR, and e2e test were failing. Rebased from |
Because we're using aria-disabled instead of the disabled property the button is clickable. We need to back off from firing the onToggle event if the toggle is disabled.
Because we're using aria-disabled instead of the disabled property the button is clickable. We need to back off from firing the onToggle event if the toggle is disabled.
Fixes #13014
Complements #12885
Test
The current behavior is that the publish sidebar will be opened, but it shouldn't be.