Open tanu08 opened 6 years ago
@majornista Sending this PR on top of gh-pages branch. The testing of the touchscreen related fixes can be done by navigating to https://adobe-accessibility.github.io/Accessible-Mega-Menu/ on Chrome browser.
Hi @tanu08 - looks good on many counts. Unfortunately there's a regression in desktop versions. Here's a bug report (not exhaustive):
testing accessible mega menu updates 2018-03-09 issue: mouseclick behavior inconsistent across browsers and platforms
unexpected behavior across most desktop browsers: description: single mouse click on parent menu items does not open subpanel; subsequent clicks highlight a parent menu item but have no further effect.
safari desktop: description: single mouse click on parent menu items does not open subpanels; subsequent clicks have no further effect. browser/os: safari 11.0.3 (latest version) on Mac High Sierra (latest version) & Sierra (penultimate version)
@tanu08 The rationale for supporting a dual behavior for links as top level menu items, where the menu displays on hover and the clicking the menu item with the menu expanded navigates through to the specified href
, was to use the principal of progressive enhancement to add behavior to the menu.
With touch browsers, the behavior of differs mainly to ensure that a screen reader user on a touch device can toggle the menu using a click action on the menu item. Clicking an area outside of the menu isn't a viable option for a screen reader user on a touch device.
If maintaining support for navigation to the top-level menu items is a requirement on touch devices, one simple solution would be to add the link within an <h*>
tag at the top of the menu panel on touch devices.
@majornista You describe the notion of navigating by clicking a menu item with the menu expanded. Do you have any insights on how well the notion has stood the test of time? I don't believe I've come across it for a while. (@tanu08 comments that adobe.com also went a different direction).
That question aside, can you weigh in on the notion of a patch to this library that supports the behavior @tanu08 proposes (i.e. to have this library work the way it does on adobe.com, with no hyperlinks in top-level nav)? There seem to be several related issues in the queue, and several patches that don't fix the issue with android devices. I wonder if this simplification is a reasonable solution, or at least a stop-gap.
@care4kittens It is probably better practice to toggle submenus with a click, as we do on touch devices, rather than rely on hover. In which case, I'd recommend a separate link within the sub-menu to navigate to any url specified in the top-level header link's href.
@majornista @care4kittens Thanks for weighing in with your thoughts. I also agree that, navigation of top-level menu links by click(in expanded state) and opening their menus on hover, though works fine on mouse-based interaction, it goes counterproductive on touch-based gestures. Also, I looked at a few other sites apart from adobe.com, and they all exhibit the behaviour I feel is more intuitive: to have consistent experience across browser and touchscreen devices - on click of top nav items open their menus, and hover is not doing menu toggle.
@majornista If the above design principle seems fair, then you can take a look at this PR for merging it into main trunk. Let me know your feedback on the patch and if any suggestions to the fix, are also welcome.
@care4kittens I have fixed the issues you mentioned above re: inconsistency in click behaviour across browsers and the issue with Safari Menu panels not opening. @majornista I'll try to update this PR itself with the new set of changes, or send in a new one with all the changes done. Let me know if that will work with you.
@majornista @care4kittens I have pushed another commit to gh-pages
branch of my fork, so this PR reflects the updated changes (2 commits till now). With this commit, we have all browsers on desktop and Mobile devices working consistently. A click on desktop browsers opens the menu panel, a touch/tap does that on touchscreen devices.
@tanu08 I just committed a few fixes that seek to address the concerns you have been trying to solve with this PR. Thank you for your contributions.
The behaviour of mouse-clicks versus touch/tap, is now made consistent.
A detailed analysis of touchscreen and mouse click/hover behaviour across browser/OS environments is captured in a long comment here: https://github.com/adobe-accessibility/Accessible-Mega-Menu/issues/32