adobe-accessibility / Accessible-Mega-Menu

A demonstration of how to implement a keyboard and screen reader accessible mega menu as a jQuery plugin.
Apache License 2.0
605 stars 199 forks source link

jquery-accessibleMegaMenu.js in this repo breaks the example #72

Closed symbiotic closed 2 years ago

symbiotic commented 2 years ago

i'm trying to use the example menu at the top of the homepage, as a starting point: https://adobe-accessibility.github.io/Accessible-Mega-Menu/

homepage (screenshot):

mega menu

homepage but with jquery-accessibleMegaMenu.js from this repo (screenshot):

mega menu (messed up)

so the formatting gets messed up (and the final HTML is actually also different).

the file jquery-accessibleMegaMenu.js on the homepage has some differences like less comments and less code.

symbiotic commented 2 years ago

upon further investigation, the homepage menu broke after the changes made by @maedi #66. it looks like all these changes were not tested with this menu.

for example the settings.panelClass is set after it is found, which does not seem to make sense. see the bold code below:

topnavitemlink = topnavitem.find("a").first();
topnavitempanel = topnavitem.find('.' + settings.panelClass);
_addUniqueId.call(that, topnavitemlink);
// When sub nav exists.
if (topnavitempanel.length) {
    _addUniqueId.call(that, topnavitempanel);
    // Add attributes to top level link.
    topnavitemlink.attr({
        "role": "button",
        "aria-controls": topnavitempanel.attr("id"),
        "aria-expanded": false,
        "tabindex": 0
    });
    // Add attributes to sub nav.
    topnavitempanel.attr({
        "role": "region",
        "aria-hidden": true
    })
    .addClass(settings.panelClass)
    .not("[aria-labelledby]")
    .attr("aria-labelledby", topnavitemlink.attr("id"));
}
majornista commented 2 years ago

Thanks for pointing this out. This should be fixed with https://github.com/adobe-accessibility/Accessible-Mega-Menu/commit/ee1d2e83d38cb7032aa160f1bbc3a4d1eb54196b and https://github.com/adobe-accessibility/Accessible-Mega-Menu/commit/3810000cf87ee36686721bde7e0f11db490c337e.

symbiotic commented 2 years ago

@majornista There is still a problem, that the menu items on the homepage are not expanding anymore. maybe because 3810000 is missing, when i do a clone. Thanks for the fast response

symbiotic commented 2 years ago

@majornista awesome! thank you