Ensure keyboard navigation works with the drawer

Created on 26 March 2024, 24 days ago
Updated 17 April 2024, 2 days ago

Problem/Motivation

In ๐Ÿ“Œ Implement drawer functionality Needs review the initial work to have the drawer working has implemented the basic functionality. This issue is to verify it works with keyboard and otherwise work on the remaining tasks.

Key Points:

  • Arrow Keys. inner menu items
  • Tab/Shift Tab
  • Escape
  • Home / End

https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-naviga...

Note: extended scope: 'character'.

WCAG success criteria

2.1.1: Keyboard (Level A)

Proposed resolution

TBD

Remaining tasks

N/A

๐Ÿ“Œ Task
Status

Fixed

Version

1.0

Component

Code

Created by

๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

Live updates comments and jobs are added and updated live.
  • Accessibility

    It affects the ability of people with disabilities or special needs (such as blindness or color-blindness) to use Drupal.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ckrina
  • Status changed to Postponed 24 days ago
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    I forgot to postpone it.

  • Status changed to Active 22 days ago
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    Un-postponing since ๐Ÿ“Œ Implement drawer functionality Needs review has been merged in. This is ready to start the work.

  • Assigned to bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Merge request !215Fix JavaScript error โ†’ (Closed) created by bronzehedwick
  • Pipeline finished with Success
    18 days ago
    Total: 257s
    #134607
  • Issue was unassigned.
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Assigned to bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Status changed to Needs review 16 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia ehsann_95

    The Keyboard navigation is working as expected. Attaching the screen capture for the reference

  • Status changed to Needs work 16 days ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    One detail in the context of keyboard navigation i wonder if it would make sense to add is the ability to close the drawer by pressing the esc key. at the moment you have to tab through the entire sub menu, the configuration sub menu requires quite a few tabs to get out.

    On a side note not directly related to keyboard navigation but i've noticed while testing that several menu items miss focus outlines. without any patches applied to the 1.x-dev branch of the navigation module the logo, sub sub menus (for example the menu items underneath configuration -> system), and the sub menu items (view profile, edit profile and log out) for the logged in user are not visible. the username isnt tab-able at all, only clickable by mouse.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    @rkoller Agreed on Esc. I'm following the W3C guidelines, which also includes patterns for arrow key navigation, Home and End.

    Regarding the items that are not keyboard navigable, thanks for spotting! I'll look into that.

  • ๐Ÿ‡ท๐Ÿ‡ดRomania silviu.serdaru

    Silviu S. โ†’ made their first commit to this issueโ€™s fork.

  • ๐Ÿ‡ท๐Ÿ‡ดRomania silviu.serdaru

    I added ESC to close the sub-menu.

  • Status changed to Needs review 16 days ago
  • Status changed to Needs work 16 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Thanks for the suggestion @silviu-s. As mentioned previously in the comment thread, I am working on this, and will open an MR to review with everything listed. As a point of process, an issue should not transition to "Needs Review" until all aspects of the scope are addressed.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT

    I think we need some clear guide for when Arrow Keys Vs Tabbing & Escape should be working as the collection of actions that need to be this ticket.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Thanks for adding that @skaught! That table is what I'm going on.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    One change from the table above I think should be made is reversing the Up/Down and Left/Right arrow functions. The W3C example is a horizontal menu, and this is a vertical menu.

  • Pipeline finished with Success
    16 days ago
    #136701
  • Pipeline finished with Success
    16 days ago
    Total: 221s
    #136753
  • Issue was unassigned.
  • Status changed to Needs review 16 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    I added code to handle keyboard navigation, patterned off the W3C guidelines outlined in the ticket description.

  • Pipeline finished with Failed
    16 days ago
    Total: 180s
    #136763
  • Assigned to rkoller
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    based on #23 ๐Ÿ“Œ Ensure keyboard navigation works with the drawer Active i've updated the issue summary

    Menu bar:

    space/enter :
    behaves correctly

    down arrow:
    fails - the down arrow jumps not to the next top level menu items instead it jumps to the next top level menu item only that has a submenu and is expandable. if you tab to a top level menu item that has no sub menu item the down button has no effect and is unresponsive
    works - if i reach the username at the bottom and press the down key another time the shortcuts top level menu item is in focus next

    up arrow:
    fails - the up arrow jumps not to the next top level menu items instead it jumps to the next top level menu item only that has a submenu and is expandable. if you tab to a top level menu item that has no sub menu item the up button has no effect and is unresponsive
    works - if i reach shortcuts at the top and press the up key another time the username top level menu item is in focus next

    right arrow:
    fail - the drawer expands but the first sub menu item is not getting into focus

    fail in addition try the following. get the reports top level menu item into focus, press space, the drawer for reports expands, press now the up key and configuration gets into focus, press now the right arrow, the configuration drawer opens underneath the still open reports drawer

    left arrow
    fail(?) - if the top level menu item is collapsed and you press the left arrow nothing happens, while if the drawer is expanded and you press left the drawer closes. but the desired if you press the left arrow and the drawer opens and you get the last sub menu item into focus doesnt happen here.

    home
    works - ctrl fn left jumps to shortscuts on the mac

    end
    works - ctrl fn right jumps to the username on the mac

    character
    fails - i have content in focus and press m as well as M, no reaction. ahhhh just noticed. it has the same problem like the up and down button the character applies only to top level menu items that have a submenu. therefor m had no effect. if i type s i first jump to shortcuts if i press is another time i jumps to structure.

    Submenu:

    Space/Enter:
    fails - if you press space or return und a parent the focus remains on the parent instead move to the first item in the submenu
    works - pressing space or return on menu items without a submenu forwards to the linked page

    esc
    works - closes drawer and moves focus to the parent top level menu item that opened the collapsed drawer

    Right arrow:
    fails: within the configuration submenu for example when the focus is on system and you press the right button nothing happens
    fails: fi you are in the structure submenu and you have the media types menu item in focus and press the right button nothing happens

    Left arrow:
    works (?): if block types menu item is in focus in structure and you press the left button structure gets into focus

    but the focus isnt moving to the previous to level menu item in the menu bar

    fail (?) if a sub sub menu item is in focus and you press the left key not the parent sub menu item gets into focus but the parent top level menu item
    down arrow:
    fail - it only works for the configuration submenu, which is the only that consists of menu items with sub menus. that way i am able to move down and after workflow i get back up to people

    up arrow
    fail - it only work for the configuration submenu, which is the only that consists of menu items with sub menus. that way i am able to move up and after people i get back down to workflow

    home
    works - ctrl fn left jumps to shortscuts on the mac

    end
    works - ctrl fn right jumps to the username on the mac

    character
    fails - in submenus characters dont work at all not even for sub menu items that contain another submenu

    a few more details i've noticed:
    the menu item for shorcuts in the drawer is not reachable by keyboard but only by the mouse cursor. same for the username menu item in the username drawer (plus view profile, edit profile, and logout are missing the focus outline). same for the sub sub menu items they are also missing the focus outline.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany
  • Issue was unassigned.
  • Status changed to Needs work 16 days ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada mgifford Ottawa, Ontario

    Just tagging it here.

  • Assigned to bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Pipeline finished with Success
    9 days ago
    Total: 220s
    #143207
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Thanks for the review @rkoller. I pushed an update that should address the following.

    1. Up and down arrow keys moving focus to both expandable menu items and links.
    2. Up and down arrow keys moving focus as above inside popovers when opened.
    3. Typing a character jumping to either expandable menus or links, as above.
    4. Left and right arrows should open and close popovers consistently.

    There's some issues left, below.

    1. Focus immediately moving to the first popover item when opened. Currently the first down arrow press after the popover is opened will focus on the first popover menu item.
    2. The shortcuts popover menu items are still inaccessible from the keyboard.
    3. Mixing navigation with tab and arrow keys can result in unexpected jumps in focus.

    Since this MR does improve the keyboard navigation, and fixing the issues above won't be insignificant, I propose we split the remaining items into a new ticket.

  • Issue was unassigned.
  • Status changed to Needs review 9 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Here's the follow up ticket https://www.drupal.org/project/navigation/issues/3440047 ๐Ÿ“Œ Improv keyboard navigation inside drawer Active

  • Status changed to RTBC 9 days ago
  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT

    Have manually tested this. Is certainly worthy proof of concept and introduces needed javascript work that would be great for continued work around in beta phase, and for other to be able to also continue related work around in the active dev line!

    note: some oddness in 3rd level menus between 'what i think i should be doing' between switching between needing to tab into 3rd item. This maybe what you are describing in 'leftover issue point1'.

  • Status changed to Needs work 9 days ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    thank you! I've quickly tested against the four points that got fixed in #32 ๐Ÿ“Œ Ensure keyboard navigation works with the drawer Active . in "general" they work but i've found some odd behavior:

    1&2) yes in it general it works but the problems are:

    • in safari moving by the arrow keys through the top level items sometimes just stops working and i am unable to move up and down anymore (in edge and firefox i was unable to reproduce that)
    • there is some sort of active trail for the top level menu items (see safari.mp4 - reproducible in edge and firefox)

    3) for top level menu items moving by character works for sub menus it does now
    4) works - the only odd thing pattern wise, similar to the mix of hover for top level menu items and click for sub menus is the fact that with left and right you are able to collapse and expand the drawer but within a drawer left and right have no effect and you have to press space on a sub menu.

    update: oh @skaught already set it to rtbc while i was writing up my comment. due to the active trail shown in the video i would set it back to needs work. if you would move the work on that newly introduce behavior to the follow up as well then put the status back to rtbc.

  • ๐Ÿ‡จ๐Ÿ‡ฆCanada SKAUGHT

    no worry. pardon my excitement!

  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    Thanks all for the reviews! We discussed yesterday in the call (that's why @SKAUGHT jumped in to help,- thanks!-) to try to get something here in soon and open issues for follow-ups, otherwise this MR will take too much time to get in. Reason 1: this is quite too big of a big change to have while we have the MR opened for core (hopefully tomorrow), and 2. this MR changes are already better than having nothing. So let's try to move from the classic Drupal mindset "things get in when they are perfect" and not try to solve all the things in this issue: any idea of which feedback could be moved into a follow-up? :)

  • Assigned to bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Pipeline finished with Success
    8 days ago
    Total: 220s
    #144026
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    in safari moving by the arrow keys through the top level items sometimes just stops working and i am unable to move up and down anymore (in edge and firefox i was unable to reproduce that)

    I pushed a fix for this. I think the issue is pressing right arrow on menu items that don't have popovers, which pushes focus to a hidden element, preventing other arrow movement until left arrow is pressed.

  • Pipeline finished with Success
    8 days ago
    Total: 208s
    #144068
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    Gonna review now

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    thank you for the update @bronzehedwick. i've applied the latest changes but somehow i am still able to reproduce being stuck. somehow i am able to trigger the behavior faster when i hover and or click a top level menu so a drawer expands and collapses and after that i go on navigating up and down through the top level menu items. for the video i've activated the keyboard viewer so you are able to see my keyboard input. and in addition i see the following type error in the console:

    [Error] TypeError: undefined is not an object (evaluating 'focusableElements[focusedIndex-1].focus')
    	(anonymous function) (js_Ez2GMQcuGAO5zipRlKWSB0V0jT7lLqDrF3Fi0L4o6dY.js:1970:3557)

    hope that helps.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    forgot to add the video. apologies :/

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Looks like the problem here is the hover. The hover effect is controlled in JavaScript, and has a delay on toggling (by design). The hover off is triggered when you hover over another menu item. That means that if you quickly hover over a few items, then move your mouse outside the menu drawer, the hovered items will stay hovered. This blocks arrow movement, unless you press arrow left to "dismiss" the (hidden) popover.

    I haven't been able to reproduce the JS error @rkoller and @finnsky are seeing yet.

    This is an architectural issue, and will probably require some larger refactoring to resolve.

    I think this and the other remaining issues should be moved into the follow up ticket ๐Ÿ“Œ Improv keyboard navigation inside drawer Active , to reduce churn while the core MR is being made. Does that make sense with what you are thinking @ckrina?

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Pushed a fix for the inaccessible Shortcuts menu via arrow keys. Simple oversight, just needed to add tabindex.

  • Pipeline finished with Success
    8 days ago
    Total: 247s
    #144228
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    I think this and the other remaining issues should be moved into the follow up ticket, to reduce churn while the core MR is being made. Does that make sense with what you are thinking @ckrina?

    It doesn't have to be all at the same issue, but yeah we'll open the MR to core hopefully today and if we don't get this in before it'll be a nightmare to try to solve the conflicts later on and we might have to pause it for 2 weeks. :)

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Issue was unassigned.
  • Status changed to Needs review 7 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Moving to NR, given @ckrina's guidance, above. The follow up issues are below.

    1. Improv keyboard navigation focus inside drawer ๐Ÿ“Œ Improv keyboard navigation inside drawer Active
    2. Hover effect can cause problems with keyboard focus ๐Ÿ“Œ Hover effect can cause problems with keyboard focus Active
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    I added an approach that, in my opinion, fits better here. Some problems that I tried to solve:

    1. All keyboard events are controlled in one place. They are bubble so we can control them at the sidebar level using the current target.
    2. I used https://www.npmjs.com/package/tabbable which is part of the Drupal library
    3. I used the https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inert attribute to hide the submenu from focus
    4. I used data-index-text in twig to reduce client calculation and search time
    5. All menus at all levels use the same logic. Branching occurs only where it is necessary.
    6. The logo and bottom button are also false to participate in keyboard navigation, even if they are not part of the menu.

    Currently we need to add a description of how the arrows behave when the third menu level has focus. Now the focus falls into a trap from which you can escape only to the first level

  • Merge request !246Keyboard navigation - #3436130 โ†’ (Merged) created by finnsky
  • Pipeline finished with Success
    5 days ago
    Total: 210s
    #146464
  • Pipeline finished with Success
    5 days ago
    Total: 219s
    #146465
  • Pipeline finished with Success
    5 days ago
    #146478
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Hey @finnsky, thanks for this! Some things I noticed.

    • When closing a popover with left arrow, focus moves up to the previous menu item, instead of moving back to the popover's button.
    • Pressing right arrow inside an open popover, the the popover is dismissed and focus is moved to the next main menu item below the current item. I would expect nothing to happen.
    • When inside a third level menu (accordion), there's no way to move out of it.
      • Pressing either arrow left or right dismiss the entire popover.
      • Pressing Space scrolls the page.

      I would expect pressing arrow left (and maybe arrow right?) or Space to collapse the accordion, moving focus back to the top level of the accordion.

    • After initially opening a popover, pressing up arrow dismisses the popover. I would expect the focus to move to the last menu item in the popover.
    • When focus is on a collapsed third level accordion, pressing left arrow does nothing. I would expect the popover to be dismissed.
  • Status changed to Needs work 4 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Status changed to Needs review 4 days ago
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    @bronzehedwick
    Thank you for review! You are right!

    I've managed initially how it requested in ticket. But seems logic which is fair for horizontal menu not really obvious here.

    Now i added more realistic and obvious logic.
    Also fixed focus trap on lower levels.

    Please review.

  • Pipeline finished with Success
    4 days ago
    Total: 230s
    #147350
  • Pipeline finished with Success
    4 days ago
    Total: 242s
    #147351
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    Thanks @finnsky! Your updates addressed all my feedback.

    There is one small, new regression, I think from the 1.x rebase. There's extra padding and grey bar below each third level accordion.

    This seems to be caused by the recently merged additional padding MR ๐Ÿ› Bottom padding needs to be increased along gray container box of grandchildren menu items Fixed running up against the new styles.

    One solution would be to change line 38 of toolbar-menu.pcss.css from:

    .toolbar-menu--level-2 {
    

    to:

    .toolbar-menu--level-2:not([inert]) {
    
  • Status changed to Needs work 4 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick
  • Status changed to Needs review 4 days ago
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    reverted height. it was experiment anyway.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States katannshaw

    Added WCAG 2.2 success criteria.

  • Pipeline finished with Success
    4 days ago
    Total: 231s
    #147374
  • Status changed to RTBC 4 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    @finnsky that last commit fixed the issue. Thanks!

    This approach also resolves the follow up issues, listed below.

    Moving to RTBC.

  • Status changed to Needs work 4 days ago
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    Moving to Needs work per the above feedback.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany rkoller Nรผrnberg, Germany

    One minor detail I've noticed, you are able to bypass the constraint to expand only a single submenu with a combination of arrow and tab key:

    - expand the configuration submenu
    - move to the people menu item and press space
    - tab until you reach user interface
    - press the right arrow
    - tab until you reach media
    - press the right arrow

    That way you are able to have three sub menus expanded at the same time. if you try the same with only tab and space, only a single sub menu remains open. while when just use the four arrow key you are only able to move inbetween the sub sub menu items with arrow up and down of for example the expanded media submenu.

  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    @rkoller i agree that this should be covered somehow.

    But this one is good candidate for follow up.

  • Status changed to Needs review 4 days ago
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    @ckrina removed that not used code.

  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    I also think we need to rework current events system.

    They should be better documented and all iterations click/hovers/keys should go in same manner.
    Since only events cares globally about multiple things. EG

    - Switch expand/collapse text
    - Switch classes
    - Controls sibling elements open/close

  • Pipeline finished with Success
    4 days ago
    Total: 227s
    #147412
  • Status changed to RTBC 3 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick

    I also think we need to rework current events system.

    They should be better documented and all iterations click/hovers/keys should go in same manner.
    Since only events cares globally about multiple things. EG

    - Switch expand/collapse text
    - Switch classes
    - Controls sibling elements open/close

    @finnsky would this be appropriate for a follow up ticket?

  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    RE #65

    Thank you!

    for a follow up ticket?

    Yes sure. Now all works.
    But i guess all system needs some refactoring.
    We have all features onboard. And globally nothing should changed. But we can improve code readability and documentation.

  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    ckrina โ†’ changed the visibility of the branch 3436130-keyboard-navigation to hidden.

  • Pipeline finished with Skipped
    2 days ago
    #149286
  • Status changed to Fixed 2 days ago
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    Merged! Thanks all, navigating via keyboard is a great improvement for everybody.

Production build https://api.contrib.social 0.62.1