Comments

Corwin’s picture

You 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.

Corwin’s picture

In the demo, after tabbing to "Content Types", there is no way to open the menu. The arrow down key should do this.

andrewgearhart’s picture

Priority: Normal » Major
Issue tags: +Accessibility
dsrikanth’s picture

Issue summary: View changes

I kind of did a jQuery workaround.. Still testing it but looks like its able to tab with keyboard now.

(function($) {
    Drupal.behaviors.drupaldeveloper = {
        attach: function (context) {
            /*$('a.dropdown-toggle').focusout(function() {
                $(this).parent('li').removeClass('open');
            })*/
            $('.nav > li, li.mega').focusin(function(event) {
                $(this).addClass('open');
            })
            $('.nav > li, li.mega').focusout(function(event) {
                $(this).removeClass('open');
            })
            $('.nav > li, li.mega').keydown(function(event) {
                $(this).addClass('open');
            })
        }
    }
})(jQuery);
dshields’s picture

This jQuery code is really helpful and should be included in the module!

mgifford’s picture

This is a powerful module. Would be good if we could get this code in a future release.

gapple’s picture

Version: 7.x-1.0-alpha3 » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new6.98 KB

I'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.

andremolnar’s picture

#7 works as described.
Keyboard browsing working and Mac Screen Reader gave me a pretty good description on what was going on.

mgifford’s picture

@andremolnar can you mark it RTBC?

lennyaspen’s picture

there 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.

gapple’s picture

I was able to navigate submenus in FF on OSX 10.9

dureaghin’s picture

Thank you for patch #7.

Will be great to have all keyboard interaction, not just Tab:

  • If a menu bar item has focus and the menu is not open, then:
    • Enter, Space, and the up down arrow keys opens the menu and places focus on the first menu item in the opened menu or child menu bar.
    • Left or right arrow keys move focus to the adjacent menu bar item.
  • When a menu is open and focus is on a menu item in that open menu, then
    • Enter or Space invokes that menu action (which may be to open a submenu).
    • Up Arrow or Down Arrow keys cycle focus through the items in that menu.
    • Escape closes the open menu or submenu and returns focus to the parent menu item.
    • If the menu item with focus has a submenu, pressing Enter, Space, or the right arrow key opens the submenu and puts focus on the first submenu item.
  • When a submenu is open and focus is on a menu item in that submenu:
    • Up Arrow or Down Arrow keys cycle through the submenu items (behaves the same as open menu).
    • Escape or the Left Arrow key closes the submenu and returns focus to the parent menu item.
  • Typing a letter (printable character) key moves focus to the next instance of a visible node whose title begins with that printable letter.
  • First item in menu bar should be in the tab order (tabindex=0).
  • Disabled menu items receive focus but have no action when Enter or Left Arrow/Right Arrow is pressed. It is important that the state of the menu item be clearly communicated to the user.
  • Tabbing out of the menu component closes any open menus.
  • With focus on a menu item and a sub menu opened via mouse behavior, pressing down arrow moves focus to the first item in the sub menu.
  • With focus on a menu item and a sub menu opened via mouse behavior, pressing up arrow moves focus to the last item in the sub menu.
  • With focus on a submenu item, the user must use arrows or the Escape key to progressively close submenus and move up to the parent menu item(s).
  • At the top level, Escape key closes any sub menus and keeps focus at the top level menu.

I found some useful JQuery code:

$(document).keypress(function(e){
    switch((e.keyCode ? e.keyCode : e.which)){
        case 13: // Enter
            // do something
            break;
        case 27: // Esc
            // do something
            break;
        case 32: // Space
            // do something
            break;
        case 37:   // Left Arrow
            // do something
            break;
        case 38: // Up Arrow
            // do something
            break;
        case 39:   // Right Arrow
            // do something
            break;
        case 40: // Down Arrow
            // do something
            break;
    }
});
Jinghan Wang’s picture

@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.

spcbeck’s picture

My 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.

spcbeck’s picture

My 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.

mediaformat’s picture

StatusFileSize
new7.75 KB

I 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

CacheCache’s picture

The 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?

ok_lyndsey’s picture

I'm interested in helping to test this when you repost your patch @CacheCache

christian le fournis’s picture

Any update on this issue?

niner94949’s picture

Also interested in this feature.

mgifford’s picture

Status: Needs review » Needs work

I assume this is Needs work based on #17.

aubjr_drupal’s picture

StatusFileSize
new3.08 KB

To 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.

mgifford’s picture

Status: Needs work » Needs review

go bots go. - sadly this doesn't work in Contrib.

CacheCache’s picture

@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!

ok_lyndsey’s picture

@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.

mgifford’s picture

@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.

CacheCache’s picture

@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. :(

mdgilardi@gmail.com’s picture

StatusFileSize
new11.99 KB

This is a reroll of the patches from #7, #16 and #22, and a couple other changes - so far working for us.

andrewozone’s picture

Category: Feature request » Task
Priority: Major » Critical

We 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.

knaffles’s picture

StatusFileSize
new20.8 KB

The attached patch combines some bits and pieces from previous patches, and also includes support for additional key codes.

  • Adds keyboard support for tab, return, esc, up, down, left, right, home and end.
  • Adds support for aria-expanded and aria-haspopup.
  • Adds aria roles to templates where needed.

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:

  • Build a megamenu with at least a couple of top level links and at least a couple of second level links below each top level.
  • Verify that navigating by keyboard works as expected:
    • Tab - When focus is at the top level, navigate across top level links; when focus is on a submenu, cycle through all submenu links
    • Left/right - When focus is at the top level, navigate across top level links; when focus is on a submenu, navigate across columns within the submenu
    • Return - visit the link that has focus
    • Esc - close the mega menu
    • Up/down - cycle through all the links within a submenu
    • Home - When focus is at the top level, navigate to the first top level link; when focus is on a submenu, navigate to the first link in the submenu.
    • End - When focus is at the top level, navigate to the last top level link; when focus is on a submenu, navigate to the last link in the submenu.
andrewmacpherson’s picture

Partial review of patch #30. (I haven't tested this manually, or studied the resulting DOM. I read the patch.)

-<ul <?php print $attributes;?> class="<?php print $classes;?>">
+<ul <?php print $attributes;?> class="<?php print $classes;?>" role="list">

This isn't necessary. The HTML <ul> is already mapped to the list role by default. You don't need an explicit role here.

-<li <?php print $attributes;?> class="<?php print $classes;?>">

... more lines

+<li <?php print $attributes;?> class="<?php print $classes;?>" role="listitem" aria-level="<?php print $level; ?>">

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-level attribute 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.

-<div <?php print $attributes;?> class="<?php print $classes;?>">
+<div <?php print $attributes;?> class="<?php print $classes;?>" role="navigation">

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-haspopup here 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.

knaffles’s picture

StatusFileSize
new19.93 KB

Thanks 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 div in tb-megamenu.tpl.php to a nav element only because I'm concerned that could end up breaking related styles in some themes. So instead I'm just keeping the div with role="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.

knaffles’s picture

(Updating credits).

quondam’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed 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.

andrewozone’s picture

In 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...

knaffles’s picture

Status: Reviewed & tested by the community » Fixed

  • knaffles committed 7d7e7e9 on 7.x-1.x
    Issue #2046067 by knaffles, gapple, aubjr_drupal, MediaFormat, mdgilardi...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

vmkazakoff’s picture

Oh. But use space key is a really bad idea - I cant have input field in my menu (((

knaffles’s picture

@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