Closed (fixed)
Project:
The Better Mega Menu
Version:
7.x-1.x-dev
Component:
User interface
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
19 Jul 2013 at 18:04 UTC
Updated:
28 May 2021 at 13:40 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
Corwin commentedYou guys really should remove this page from the demo: http://demo.themebrain.com/tb_sirate/blog/drupal-accessibility-statement
I can't tell if you guys have a twisted sense of humor or just have no clue.
Comment #2
Corwin commentedIn the demo, after tabbing to "Content Types", there is no way to open the menu. The arrow down key should do this.
Comment #3
andrewgearhart commentedComment #4
dsrikanth commentedI kind of did a jQuery workaround.. Still testing it but looks like its able to tab with keyboard now.
Comment #5
dshields commentedThis jQuery code is really helpful and should be included in the module!
Comment #6
mgiffordThis is a powerful module. Would be good if we could get this code in a future release.
Comment #7
gappleI've put together a patch that adds keyboard event handling, and also adds some ARIA attributes to make navigating nested lists more understandable to screen readers.
Comment #8
andremolnar commented#7 works as described.
Keyboard browsing working and Mac Screen Reader gave me a pretty good description on what was going on.
Comment #9
mgifford@andremolnar can you mark it RTBC?
Comment #10
lennyaspen commentedthere is a problem on Firefox.
When hitting Tab button it opens up submenu then it closes again. not going to any of the submenu items.
In chrome works ok.
Comment #11
gappleI was able to navigate submenus in FF on OSX 10.9
Comment #12
dureaghin commentedThank you for patch #7.
Will be great to have all keyboard interaction, not just Tab:
I found some useful JQuery code:
Comment #13
Jinghan Wang commented@lennyaspen Same here. In Chrome, it just works fine. But in Firefox, the submenu won't show. Have you already figure out a way to fix that?
Thanks.
Comment #14
spcbeck commentedMy organization was sued for accessibility directly related to this and other accessibility issues and lost. So be very careful when using TB mega menu.
edit: and it appears that patch no longer works, or at least it doesn't for me in Chrome OSX El Capitan.
Comment #15
spcbeck commentedMy organization was sued for accessibility directly related to this and other accessibility issues and lost. So be very careful when using TB mega menu.
edit: and it looks like the patch in #7 no longer works in the latest version of TB.
Comment #16
mediaformat commentedI agree with #12, if a11y is claimed here, then more keys codes should be supported.
Anyhow, I am just testing this module out, here is a re-roll of #7
Comment #17
CacheCache commentedThe re-roll provided at #16 does not appear to work for tabs. Will repost an updated patch if I can get it going.
Are there any plans to officially roll ADA compliance into the module?
Comment #18
ok_lyndsey commentedI'm interested in helping to test this when you repost your patch @CacheCache
Comment #19
christian le fournis commentedAny update on this issue?
Comment #20
niner94949 commentedAlso interested in this feature.
Comment #21
mgiffordI assume this is Needs work based on #17.
Comment #22
aubjr_drupal commentedTo help with ADA compliance, we came up with the attached patch (in JS) to deal with accessibility in that it made the menu forward and backwards with tabbing.
It may not catch all ADA issues, but it helps. Please consider adding this to the module.
Comment #23
mgiffordgo bots go.- sadly this doesn't work in Contrib.Comment #24
CacheCache commented@ok_lyndsey the one posted by aubjr_drupal in #22 was the one that I was discussing in #17. Do let us know if you see any issues!
Comment #25
ok_lyndsey commented@cachecache - I didn't notice your comment - sorry - let me test. But first I'm not sure what @mgifford means by sadly this doesn't work in contrib - does that mean #22 needs a change before testing? I'll ping you in Slack to fast track.
Comment #26
mgifford@ok_lyndsey I tried to test the patch like you do in Core by ensuring that the Status is marked as "Needs review". I wrote "sadly this doesn't work in contrib" because I couldn't actually engage the bots.
Comment #27
CacheCache commented@ok_lyndsey - No problem! We've been in the process of testing on our end as well and found an issue with the patch.
Some of the groups using the Megamenu have custom blocks added. If the custom blocks don't mirror the TB Megamenu HTML/CSS-classes then it creates issues for people using a mouse.
Haven't seen any problems with linear, nested, or multi-column menus, but it's definitely not a complete enough solution to merge into contrib. :(
Comment #28
mdgilardi@gmail.com commentedThis is a reroll of the patches from #7, #16 and #22, and a couple other changes - so far working for us.
Comment #29
andrewozoneWe are sizing up this effort and will provide an update on improving this version for keyboard accessibility.
Work has been done to make TB Mega Menu keyboard accessible for projects, but it was done as custom code. We are looking at this custom code and aiming to implement this into the module.
Comment #30
knaffles commentedThe attached patch combines some bits and pieces from previous patches, and also includes support for additional key codes.
Note that this patch has been applied against the latest commit in the 7.x-1.x branch. If it doesn't apply cleanly against your installation, you may need to first update to the latest commit in the 7.x-1.x branch.
To test keyboard navigation:
Comment #31
andrewmacpherson commentedPartial review of patch #30. (I haven't tested this manually, or studied the resulting DOM. I read the patch.)
This isn't necessary. The HTML
<ul>is already mapped to the list role by default. You don't need an explicit role here.The HTML
<li>is already mapped to the listitem role by default. You don't need an explicit role here.I'm unsure why you are setting the
aria-levelattribute here. It is allowed for list items, but what are you trying to achieve with it? Is the level unclear from the DOM structure of<ul>and<li>? Unless you've identified a particular need for it, I'd leave it to the user-agent to calculate the level.Is there a reason why you aren't using a
<nav>element here, rather than<div role="navigation">?Note that this navigation landmark doesn't have an accessible name. These are strongly recommended, especially where there's a likelihood of multiple navigation landmarks on the page.
The use of
aria-haspopuphere isn't appropriate, and it should be removed. It should only be used when the associated popup content is contained by certain roles; list isn't one of them.Comment #32
knaffles commentedThanks so much for your feedback @andrewmacpherson. Everything you said makes sense and I've updated accordingly. The only thing I didn't change is I did not update the
divintb-megamenu.tpl.phpto anavelement only because I'm concerned that could end up breaking related styles in some themes. So instead I'm just keeping thedivwithrole="navigation", which is effectively the same thing anyway.I've rerolled the patch, with Andrew's feedback incorporated, and also to account for recent dev updates.
Comment #33
knaffles commented(Updating credits).
Comment #34
quondam commentedReviewed and tested at length in collaboration with @andrewozone, who stepped through validating all accessibility concerns while I took a look at the updated codebase and confirmed that no new JS or PHP errors were being thrown by the changes introduced via patch #32.
Comment #35
andrewozoneIn addition to testing with @quondam, I have updated the documentation detailing how to keyboard navigate the mega menu. https://www.drupal.org/docs/contributed-modules/the-better-mega-menu/acc...
Comment #36
knaffles commentedComment #39
vmkazakoff commentedOh. But use
space keyis a really bad idea - I cant have input field in my menu (((Comment #40
knaffles commented@vmkazakoff, thanks for pointing out that issue with the space bar. I've created this ticket in response and assigned it to myself:
https://www.drupal.org/project/tb_megamenu/issues/3216250