Twenty Twenty: with horizontal menu, submenu should be dismissible

[lcarevic]

Hi there!

I've tried adding a submenu in the horizontal menu but there is some issue regarding accessibility when I tested it.

It fails the Success Criterion 1.4.13 Content on Hover or Focus of WCAG : ​https://www.w3.org/TR/WCAG21/#content-on-hover-or-focus

The submenu cannot be dismissed. If there are many links in the submenu, keyboard navigation is tedious since the person will have to tab all of the content of the submenu before reaching the next item of the menu.

Expected fix:

The submenu should be dismissable with the Esc key for example.

Here is a menu with a submenu that implements the SC 1.4.13 : ​https://www.eesc.europa.eu/

It's my first ticket, so I hope I'v done it properly.

Regards,

Luce

[rcreators]

Updated CSS file and JS file code. Keyboard tabbing will open submenu and esc will close submenu as per accessibility standard.

[rcreators]

Patch added. Need Testing. Style.css, Style-rtl.css and index.js files updated. Tabbing will open a sub-menu and the user can go through it. If the user presses the ESC key while the sub-menu is opened, it will close the submenu and the user can move to the next menu item.

abbing will open a sub-menu and the user can go through it. If the user presses the ESC key while the sub-menu is opened, it will close the submenu and the user can move to the next menu item. Also updated index.js as per slack discussion to make parent a focus when closing sub menu with esc.

Trac ticket: https://core.trac.wordpress.org/ticket/49950

[rcreators]

@joedolson as discussed over Slack, updated the code and submitted PR.

[joedolson]

Thanks, @rcreators! I think it's too late in the 6.5 cycle to add this in, but I'm going to slate it for 6.6, and plan on getting it in early.

[poena]

Hi @rcreators
I tested PR 6150 using Chrome on macOS, and the update breaks the menu.

---

When I navigate to the menu using the tab key, the visible focus is on the first parent menu item, and the submenu shows.
Pressing tab again, I am expecting to be able to use the submenu, but the submenu is no longer visible, and I can't see where the focus is.

---

I can only use the escape key once to close a submenu.
When I navigate to the menu using the tab key, the visible focus is on the first parent menu item, and the submenu shows.
When I press escape, the submenu no longer shows.
if I tab forwards from there, the visible focus os on parent menu item two, and its submenu shows.
When I press escape, nothing happens.

---

If I navigate with they keyboard past the menu and navigate backwards with Shift + Tab, focus moves to the wrong element. It moves to the parent menu item, not the the last item in the submenu.

[joedolson]

I tested this as well, but I don't see quite the same experience as @poena

Pressing tab again, I am expecting to be able to use the submenu, but the submenu is no longer visible, and I can't see where the focus is.

1) I can't replicate this issue.

I can only use the escape key once to close a submenu.

2) I replicated this issue.

If I navigate with they keyboard past the menu and navigate backwards with Shift + Tab, focus moves to the wrong element. It moves to the parent menu item, not the the last item in the submenu.

3) In my opinion, this change is an improvement, since it allows the navigation of the menu to be sped up both forward and backwards. However, if it's not a necessary change, it could be eliminated. I'm not sure the toggling of visibility is actually necessary to this change.

I also found an additional issue - the menu no longer closes when the link loses focus by means *other* than a keypress. E.g., if you tab onto the parent menu then click on the page, the menu stays open. A loss of focus by click should also be considered intent to close the menu.

[rcreators]

Hello @joedolson and @poena,

So I found the issue that @poena talked about. It happens when there is a multi-level sub-menu. I adjusted the code to make it work in that situation as well.

Apart from that, if you move back with the shift + Tab, as the sub-menu is already out of focus, it will focus on the parent menu item and then you can move to the sub-menu item with tabbing. Let me know if you want to change that function. It can be just updated with visibility CSS.

I also added a focusout event for the menu. So if the user clicks or does any event outside of the menu, the sub-menu will close down as well.

Please test this out and let me know if any further changes are required.

[joedolson]

Even though I actually think it's an improvement, I actually am inclined to think that we should not have reverse tabbing skip the submenus. On older core themes, we generally try only to fix bugs, and not actually make changes to behavior beyond that, which this would be.

@poena, if you have an opinion on this, let us know.

[karmatosed]

My recommendation would be to move this to close due to the reversing. I am going to gently propose this with the keyword but we can always revisit. I would love to have feedback from @poena and others that might counter this.

[joedolson]

@karmatosed I'm not following your reasoning for closing this ticket. The reverse tabbing is not a change we should make in this ticket, but that doesn't have any relationship to the need to be able to close submenus using the esc key. My argument is that we should limit the scope of the patch to the reported issue, and not make other unneeded interface changes at the same time.

[karmatosed]

Apologies @joedolson I misunderstood. Thank you for course correcting. This is another good reason we have that keyword and let things sit for a bit, so this can be checked, thanks for doing that.

[rcreators]

@joedolson and @poena Reverse Tabbing to the sub-menu is amended and working fine with the rest of the functionality. Please test and confirm. It is ready to commit.

[joedolson]

Requested changes on the PR.

[sabernhardt]

Moving to 6.7 for more change(s) and testing

[poena]

Is anyone able to clarify what changes are still requested? So that this can be addressed.

[joedolson]

The changes requested are fairly clearly laid out in the PR; are you asking me to repeat them here, @poena?

See: ​https://github.com/WordPress/wordpress-develop/pull/6150

[poena]

@joedolson No, I did not see the comments since they are not copied from GitHub to Trac, explaining where the requests are is enough.

[chaion07]

Thanks @lcarevic for reporting this. We reviewed this Ticket during a recent bug-scrub session. Here's a summary of the feedback received:

1. There is a new commit on PR that marks the requested changes as outdated
2. Considering the possibility of moving this Ticket to the next milestone in case it doesn't get closed

Thank you.

Props to @pratiklondhe for these suggestions.

Cheers!

[stoyangeorgiev]

This ticket was discussed at a bug-scrub session. With Beta 3 in just a few hours, will move this one to 6.8. Some refreshment of the PR may be needed.