Problem/Motivation
From #3618906: [Meta] Forms sidebar accessibility issues
The toggle control shouldn't be rendered as a link with role="button".
If it's meant to function as a link (IOW, it's supposed to go somewhere usable) without JavaScript:
Render it as a standard link.Either change it to a button or add role="button" in the JavaScript when the toggle behavior is added.
If it's not meant to function as a link without JavaScript, render it as a button.
The toggle control should be operable with the SPACE key, probably on keyup.
See https://www.w3.org/WAI/ARIA/apg/patterns/disclosure/examples/disclosure-..., https://adrianroselli.com/2022/04/brief-note-on-buttons-enter-and-space.....
The accessible name (& probably the title attribute) of the toggle control probably shouldn't change according to the state. See #3093378: Use ARIA disclosure pattern for submenu buttons in vertical toolbar orientation for @andrewmacphereson's explanation of why.
Similar to the toggle control, the close button shouldn't be rendered as a link with role="button".
I can't imagine that is supposed to function as a link in any circumstance, so it should probably just be a button element. Probably not really an accessibility issue, now that I think about it. Just bad practice.
Proposed resolution
This may or may not be a bug, since it does have the correct ARIA attribute. However, this is really low hanging fruit, and should be really be a <button>
The SPACE key should also operate as expected and fire a click event. We shouldn't have to mess with keyup or anything.
For the close button, I'm fairly confident that we can just remove it, since the toggle button exists. As far as closing with the ESC key, that key is also used for the Navigation module. We need to take that into account.
Issue fork drupal-3619154
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- sidebar-toggle-fixes
changes, plain diff MR !16911
- 3619154-accessibility-fixes-for
compare
Comments
Comment #2
mherchelCrediting @kentr in issue credit due do discovery in #3618906: [Meta] Forms sidebar accessibility issues
Comment #3
kentr commentedI think the color in forced-colors is currently wrong. IIRC it’s the same as color as
CanvasText.If so, that part is a bug. Using a
buttonelement should fix that, though.Comment #4
mherchelComment #5
mgiffordThis is a good best practice. Buttons should be buttons, links should be links.
Comment #6
kentr commentedJust discovered that when the Navigation module is disabled, the sidebar obscures the toggle button.
As long as the sidebar is a modal, clicking the overlay does close the it even when you can't access the toggle control. But I don't think that's great UX.
Comment #7
kentr commented@mgiffod You think the forced-color problems are minor also?
Comment #8
mgiffordIf I used forced-colors then absolutely it wouldn't be minor to me. I'm just thinking compared to the things like just meeting color contrast which tend to affect more people. I am hoping that we can get to this though.
I do think that this AI generated approach is at least one way to deal with that button:
https://www.drupal.org/project/drupal/issues/3619387#comment-16750667
Although as you've mentioned before @KentR it is wise not to remove something until you know why it was put there.
I've got enough open issues at the moment that I may be confusing things. If so, sorry.
Comment #9
kentr commented@mgifford, makes sense.
I'm just thinking that the semantic forced-color color problem can be broken out into a separate issue that has a different priority than this one.
Or, it could be addressed in #3619155: Admin theme's toggle button disappears on hover/focus in forced colors, which I think would be working with the same piece of CSS.
Comment #10
kentr commentedCommented about fixing the semantic color problem in #3619155-3: Admin theme's toggle button disappears on hover/focus in forced colors.
Comment #13
mherchelPutting a bit of work into this on a boring Sunday.
I'm moving the toggle into a SDC, and going to try to simplify the code while we fix these a11y issues.
Comment #14
kentr commentedRemoved the bit about toggle button coloring. Handled in #3619155: Forced-colors problems with admin theme's sidebar toggle button.
Comment #15
mherchel