@camilledavis

Closing as duplicate of https://www.drupal.org/project/drupal/issues/3360837 🐛 Users with permissions for toolbar but not administration tray get "Uncaught TypeError: $orientationToggleButton[0] is undefined" in console Needs work

Added a draft change record.

Just tested with Gin, it doesn't seem to be affected due to not using Claro's page.html.twig

Voiceover announces the row (see screenshot), so I removed the reference to screen readers from the description.

@Charles Belov I agree with you, but based on the previous discussion such a compromise may be necessary to get the feature merged...

Sure, where should it go? I'm guessing Nightwatch/Tests/ and make a file called dropbuttonTest.js?

Separated Olivero issue into its own issue.

camilledavis → created an issue.

I believe aria-current it was added in this issue: https://www.drupal.org/project/drupal/issues/3038523 📌 Add aria-current attribute to navigation items RTBC

camilledavis → created an issue.

Made MR based on the following feedback from @maxstarkenburg in #accessibility Slack:

1. Remove the ["Expand/Collapse" text] because it's redundant with information a screen reader would already announce via aria-expanded.
2. Depending on who you ask, the button's accessible name (potentially via aria-labelledby) ought to be "[sibling link text]" or "[sibling link text] sub-menu" or "More [sibling link text]".
3. Also, personally, I would recommend not letting the effort to add aria-controls block this issue (perhaps could be made into a separate issue?), since adding aria-expanded and removing the incorrect state would both be easier wins, adding value sooner. aria-controls seems to have limited AT support and/or be semi-"controversial" in its usefulness, see e.g. https://heydonworks.com/article/aria-controls-is-poop/ and https://github.com/w3c/aria/issues/995

camilledavis → created an issue.

I'm testing 3821 (in Firefox) and had a few issues:

1. If I set the menu to vertical orientation, then back to horizontal, the arrow icons disappear and there is no more focus state

2. I find it unintuitive that the vertical and horizontal modes use different keyboard navigation patterns. I also wasn't able to navigate the horizontal toolbar using my assistive software (Vimium browser addon which allows you to access any interactive element on the page using a 2-letter sequence) because the chevrons in the horizontal mode are not a separate element that open/closes the menus (the way they are in vertical mode).

Though not every keyboard-only user uses Vimium, this kind of functionality is pretty common in assistive software (for example Voice Control, ShortCat and VimMotion provide this functionality for macOs). Having the collapse/expand button be its own element would make it easier in general for assistive software to interact with.

For this reason I think the chevrons should be their own element and allow keyboard users to open/close the associated menu by pressing "Enter" or "Space" while under focus. This could be implemented in addition to the "arrow keys" functionality (and in general I think it's a good accessibility principle to provide multiple ways to do the same thing)

Just ran into an issue when adding this patch to a site that has Claro as the admin theme, but has a different theme set for non-admin pages.

In the patch I’ve updated 2 toolbar.theme.css files — one in Claro, using design system variables, and one in the toolbar module, using hex codes as a fallback.

However, when Claro is only set as the admin theme, if you are logged in and visit a non-admin page, the Claro version of toolbar.theme.css still gets loaded but the variables don’t — so the module fallback doesn’t work. This is a core issue I think.

Just ran into an issue when adding this patch to a site that has Claro as the admin theme, but has a different theme set for non-admin pages.

In the patch I’ve updated 2 toolbar.theme.css file — one in Claro, using the design system variable, and one in the toolbar module, using #004eff as a fallback.

However, when Claro is only set as the admin theme, if you are logged in and visit a non-admin page, the Claro version of toolbar.theme.css still gets loaded but the variables don’t — so the module fallback doesn’t work. This is a core issue I think.

In the meantime I could try adding both to the toolbar.theme.css file in Claro:
background: #004eff;
background: var(--color-blue-500);

So that if it can't access the variable there's still a fallback.

@smustgrave I can't reproduce the bug, is it with a specific theme?

Added fallback for people not using Claro

camilledavis → made their first commit to this issue’s fork.

@mherchel thanks for your feedback!

Re: the following -

Buttons should toggle aria-expanded to indicate their state to screen readers. Ideally this would be handled in core's toolbar.menu.js, but there's already a mutation observer set up, so this should be super straightforward to add.

If I add aria-expanded attributes I'd also have to add aria-controls attributes to the buttons and a unique id for all the submenus. This feels a bit messy to do here since I am just pulling keyboard functionality from Core's mobile/vertical toolbar mode. Since the expanded/collapsed state is already indicated by the visually hidden label changing, I think it would make more sense to make a core issue for this.

No need to flip chevron on click. This is weird because the hover state doesn't flip the chevron, but the click action does.

The "opened on hover" and "opened by clicking" states are slightly different though - if a dropdown is opened on hover, it'll close on focusout, but if it's opened by clicking it will stay open until clicked again. So the chevron rotating could indicate that the menu will now stay open until clicked closed.
If we want it consistent though I think it would make more sense to flip the chevron on hover as well.

@kostyashupenko The stylelint from my package.json doesn't flag line breaks... I have 15.10.1

Looks like the same issue still happens in https://www.drupal.org/project/drupal/issues/3310075 🐛 Toolbar js errors when you remove the administration menu Active which hasn't been fixed

Error seems to have been fixed in 10!

Just realized an additional issue, the dropbuttons close automatically half a second after losing focus. This seems confusing for screen readers / magnifiers, even after fixing the aria attributes to match the state, you wouldn't necessarily expect it to change. Also the buttons don't open on focus/hover, so it seems inconsistent to have them close automatically on focusout

Created MR for 11.x, just for Claro though. Wondering what to do about Olivero since the skiplink design is different. Would be good to have some input from designers...

Just created MR for 11.x

Note that accessible keyboard navigation is being added to the admin_toolbar module in this issue: https://www.drupal.org/project/admin_toolbar/issues/3286466 ✨ Tabbing order does not satisfy 508 accessibility requirements Needs review

Maybe the same pattern can be used here

Weird, looks like it treats the text differently from the icons. Think the easy fix here would be to not have the arrows turn black on focus

@rkoller Thanks for testing this!

For Claro I agree that a focus outline would be better, Claro has a white-in-green outline on most stuff and I created a patch to add this to the toolbar here: https://www.drupal.org/project/drupal/issues/3270230 🐛 Toolbar focus styles are not sufficiently obvious to know where your focus is Needs work

Added a config option to remove keyboard accessibility, if this option is checked the toolbar will fall back on the previous behavior (force keyboard users to tab through all child elements.)

I've re-included this previous keyboard navigation code in lines 103-118 of admin_toolbar.js but one thing I find annoying is that it gets run 5 times (every time drupal.behaviors is triggered)

In the rest of the code I've prevented things from running multiple times by putting it in a once() function. However the previous keyboard navigation code passes a 'context' variable to jQuery, which the once() function would break. I can't figure out what this 'context' variable is for. Usually it's for running jQuery on updated DOM but I don't know of any module/theme that adds toolbar items dynamically. If such a module/theme is in use, the code I've added may not work - to fix it I'd have to take it out of the once() and pass the 'context' variable as well. I'm reluctant to do this though unless someone can point me to an actually theme/module that would cause this issue

@saschaeggi Here's how it would impact Gin:

I'm also wrapping up a commit that would allow the keyboard navigation arrows to be disabled. They'd be enabled by default, but users could disable them temporarily (I hope!) while the dependent theme/modules are updated.

New MR looks like this:

I tested it with Claro and Oliver 9 and 10 - any suggestions for other themes/modules to test against?

Also tested with Gin which will need to be updated

Here are a few options to make the arrows less visually intrusive, any thoughts?

Using the chevron from core level-2 dropdowns:

Using the chevron from Gin - vertical placement could vary too:

camilledavis → created an issue.

@kvantstudio Strongly disagree that there should be an extra step to enable accessibility features. Accessibility should be the default. Disabling the feature is what should require an extra step (and even then I'd be careful about disabling it for all users).

Agree with @rkoller in https://www.drupal.org/project/admin_toolbar/issues/3357166 🐛 Upset theme since update to 3.3.1 Closed: duplicate and @itmaybejj in this thread https://drupal.slack.com/archives/C2ANFUGGG/p1680208825935249 that the arrow buttons could be made less visually intrusive.

One way could be to make the arrows not be in a circle (the way the arrows on the sub-menus are), and/or make them smaller (but keep enough padding for an adequate touch target.) Would be good to have more input from designers

This MR:

• Puts the secondary actions in a sublist
• Adds aria-expanded and aria-controls attributes to the toggle button
• Changes the toggle button text to "Show additional actions" or "Hide additional actions"

Working on MR, I think it will be necessary to put the secondary actions in their own

element so it can be marked as aria-expanded.

Noticed an additional inconsistency (at least in 9.5): hovering over the dropdown toggle button doesn't open it (you need to click), but hovering out closes it automatically. I think it should be the same action to open and close, and my preference would be clicking (so the open state does not depend on where you cursor is). So if a user with a hand tremor or other mobility impairment opens the menu and accidentally moves the cursor away, they can still access it.

Additional accessibility issue: it should be possible to dismiss the dropdown by pressing "Esc" ( reference - https://www.w3.org/WAI/WCAG21/Understanding/content-on-hover-or-focus.html )

Updated the active background to --color-blue-500.

However I couldn't find --color-green-500 in Claro 9.5, to include it I think I'd have to add new variables to the MR.

Even if I did, the green background on focus isn't an accessible state change by itself, the button would still require the white-in-green focus ring from https://www.drupal.org/project/drupal/issues/3270230 🐛 Toolbar focus styles are not sufficiently obvious to know where your focus is Needs work to be accessible

camille.davis@civicactions.com → made their first commit to this issue’s fork.

Oh hold on I think I grabbed the wrong blue color from @saschaeggi's png... now I'm trying to grab the color from https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/%F0%9F%92%A7-Drupal-... but getting a few different values

Does anyone have the exact hex/rgb code handy?

I don't think so because the focus ring has 2 color rings in it with 3:1+ contrast between the colors

Here's a screenshot of the focus state with the white-in-green:

And here it is desaturated:

Still need to make a right-aligned version for RTL

camille.davis@civicactions.com → created an issue.

Here's a before/after for option 1

(The different options are just for focus ring style, in all 4 options the link is on the top left)

Before:

After:

I believe it's a pass since the focus ring is a UI component / graphical object

I wonder if this is related to a weird issue I'm getting in 9.5 where if I change the active toolbar toggle button ("Manage", etc) to blue to address https://www.drupal.org/project/drupal/issues/3097907 🐛 Active toolbar tray has weak affordance and fails WCAG color criteria Needs work , a few white pixels flicker on and off to the left or right of the button when resizing the window in Firefox.

Issue appears to have been fixed in 10 but not sure how

Here's an MR for the blue active tab in Claro 9.5

It currently has no visible focus state (aside from the underline). At first I tried with this css:

.toolbar .toolbar-bar .toolbar-tab > .toolbar-item:hover,
.toolbar .toolbar-bar .toolbar-tab > .toolbar-item:focus {
  background-image: -webkit-linear-gradient(rgba(255, 255, 255, 0.125) 20%, transparent 200%);
  background-image: linear-gradient(rgba(255, 255, 255, 0.125) 20%, transparent 200%);
}
.toolbar .toolbar-bar .toolbar-tab > .toolbar-item.is-active {
  background-color: #1d7aff;
}

So that the active tab turned slightly lighter while under focus, as it did previously. However this causes it to fail WCAG AA contrast requirements for small text. So I ended up changing the style to just background: #1d7aff so the transparent overlay doesn't get applied on focus.

Looks like this:

With the focus styles from https://www.drupal.org/project/drupal/issues/3270230 🐛 Toolbar focus styles are not sufficiently obvious to know where your focus is Needs work it will look like this:

And here's a MR using option 1

Here are some options for Claro 9.5 using the theme's white-in-green focus ring:

1. Partial focus ring with round edge:
   
2. Partial focus ring with square edge:
   
3. Full focus ring with round edge:
   
4. Full focus ring with square edge:
   

@saschaeggi @dww no prob! Just updated the MR and copied the focus ring css to the toolbar module.

@dww For sure!

1. Here's a commit without the newlines - they get added when I run yarn run build:css, I'm guessing that's not normal?

2. Should I remove the styles from Claro and add them to the toolbar module?

Maybe since the green is Claro-specific I could use some sort of "default" color in the module, and overwrite it as green in Claro? Not sure what the "default" color would be though.

Opened an MR for the focus ring on 9.5.x on this issue 3270230 - Toolbar focus styles are not sufficiently obvious to know where your focus is 🐛 Toolbar focus styles are not sufficiently obvious to know where your focus is Needs work

The issues are related, but solving #2784311 would not solve this issue. However the fix for #2784311 could be incorporated into the fix for this issue.

@saschaeggi Got the focus ring to cover the left and right border on the 1st level tray items, but I have to remove the 1px amount from the left and right of both insets:

.toolbar-tray-horizontal .toolbar-menu-administration > .toolbar-menu > .menu-item > a:focus {
box-shadow: inset 2px 3px 0 0 #26a769, inset -2px -3px 0 0 #26a769, inset 4px 5px 0 0 #fff, inset -4px -5px 0 0 #fff, 1px 0 0 0px #26a769, -1px 0 0 0px #26a769;
}

Is there a way to make the css simpler? After I replace the colors and px values with focus variables (some of which are calculations of 2 more variables) it gets pretty unreadable.

And that's just picking the variables that are equal to the px size... it's actually worse if you consider that for example, the first "2px" value is actually the link's --focus-border-size (which is set to 3px), minus its parent's border size (1px). So to account for the --focus-border-size variable changing, that 2px value would have to be written as calc(var(--focus-border-size) - 1px). And so on...

camilledavis → created an issue.

Cross-posted from slack and issue 3097907 - Active toolbar tray has weak affordance and fails WCAG color criteria 🐛 Active toolbar tray has weak affordance and fails WCAG color criteria Needs work :

Re: adding focus ring to the toolbar

@saschaeggi: the focus ring we use in Claro has a 2px white inset/offset so it passes the contrast.

@camilledavis: Is there somewhere I can grab that css from?

@saschaeggi: https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/themes/clar...

Note:
/**
* Default focus styles for focused elements.
*
* This is applied globally to all interactive elements except Toolbar and
* Settings Tray since they have their own styles.
*/

so that’s maybe what we want to change cc @ckrina

@camilledavis: thanks! So if that was applied to the toolbar it could go two ways, not sure which is best. Inside, you could see the whole focus ring. Outside, it would match the other focus styles.

@ckrina: +1 to this change as long as this visual language pattern is limited to the existing Toolbar.

And to answer @camilledavis, I would go with the inside focus ring, but just for the Toolbar and its specific placement.

I hope this solves the accessibility concerns.

@camilledavis: Should the focus ring replace the current focus state (underline, slight change in background color) entirely? Or appear in addition to it?

@saschaeggi: it should be additional to underline & bg color

@camilledavis: What about the tray -- should the focus ring cover the gray border on the left and right?

And here's what it looks like with admin_toolbar module enabled... should the focus ring cover the borders on all sides here?

What about the tray -- should the focus ring cover the gray border on the left and right?

And here's what it looks like with admin_toolbar module enabled... should the focus ring cover the borders on all sides here?

@ckrina Should the focus ring replace the current focus state (slight change in color) entirely? Or appear in addition to it?

camilledavis → made their first commit to this issue’s fork.