accessible-menu
accessible-menu copied to clipboard
NickDJM
Feat: Disclosure Navigation Menu with Top-Level Links
Summary
W3C has created a new example for Disclosure Navigation Menus with Top-Level Links. This is something I have been waiting to support, so I'd like to implement this asap.
I haven't done a whole lot of looking in to the new example, but I would assume this is going to need its own subclass. DisclosureNavigationTopLevelLinks seems a little wordy but would be descriptive. If a new subclass is required then we'll go with that for now and possibly change it to something else if it still gets the point across.
Solution
Add a new subclass (possibly DisclosureNavigationTopLevelLinks) that supports the new structure.
Alternatives
If the structure and changes required don't differ too much from regular Disclosure Navigation Menus its possible we can just add it into the existing subclass, however it doesn't feel "right" compared to a new subclass.
David MacDonald calls them "shadow button" menus: https://www.davidmacd.com/widgets/menu/wcag-tutorial/index.html
Perhaps the class can be called ShadowDisclosureMenu. It sounds a little better than the class name above.
Implementation of this menu variant would be really great! :+1:
Side note in regards to the shadow buttons: https://www.fh-krems.ac.at/ has those, but as already mentioned in https://github.com/NickDJM/accessible-menu/issues/161#issuecomment-1030595557 a solution for big touch displays is needed. The desktop menu is not usable on big touch screens, because the sub menu can't be opened.
@NickDJM In our initial testing of the W3C script, we have noticed that the logic does not support a third-level menu dropdown. That is, the behavior is glitchy and somewhat unpredictable when you add a menu with a third level. We are interested in adopting this for our site building platform and are wondering what your intentions are regarding support of the third-level menu use case?
@pyrello Just to clarify when you say a third-level menu dropdown do you mean something like this:
If so, the current menu's should support it just fine, and this new one would as well. Theoretically accessible-menu should support as many nested dropdowns as you throw at it, though I can't say I have tested more than 2-3 levels.
@NickDJM Yes, that is what I am talking about. In our initial testing of using the JS at https://www.w3.org/TR/wai-aria-practices-1.2/examples/disclosure/disclosure-navigation-hybrid.html, triggering the down arrow button next to # 2 in your example screenshot above causes the parent item (# 1) to close.
@NickDJM Do you know your timeframe for getting this included in the library? We are under some pressure to get some kind of implementation done fairly soon and we would like to our efforts to align with what is ultimately going to end up here. If it is helpful, we could open a PR and make an attempt at adding this new class.
@pyrello I can make some time this weekend to get a plan sorted for this class, however PRs are always welcome!
As a note: I think TopLinkDisclosureNavigation is probably the best class name. It describes what it is without being overly wordy.
So the main thing we'll need account for with the new TopLinkDisclosureMenu class will be adding a new argument for topLinkSubmenuToggleSelector which should default to button. This is pretty simple- technically if you use classes instead of tags you could do that with the existing DisclosureMenu class- however keybindings is where the biggest changes will have to be made...
Currently, we have:
https://github.com/NickDJM/accessible-menu/blob/48f077849e921f3d7967542f28754f8c33af260b/src/disclosureMenu.js#L321-L329
This only looks at the menu item since there was never a need to worry about where in the item you were. There will need to be distinction between focus on the link or the button.
To update this issue instead of just posting everything in the MR: Functionality-wise I believe this is ready. All that's left is to write tests and documentation on it.
This is now merged in to the 4.x branch.
I'll look into making a beta release soon. I'd like to wait for #188 to be merged into 3.x and then I can move that forward and get the beta 4 and stable 3 released at the same time.
@pyrello I know you were integrating this at a pretty early stage, so I would double check you're still good with the latest build once the beta releases.