Pre-requisite: Don't commit this until after we've addressed the other problems in #3186349: Major accessibility problems with Olivero header show/hide feature.

Problem/Motivation

Opening desktop navigation dropdown menu using touch requires two taps.

After a bit of JavaScript debugging, it turns out that the first tap triggers the mouseover event, which opens the menu. Then that tap instantaneously triggers the click event, which then closes the menu. This all happens instantaneously so the menu just appears to not open.

Steps to reproduce

Use Chrome device emulator to replace mouse with touch (or use actual touch device)

  • Create menu with items that have at least 2 levels
  • On the front page using desktop, try to open menu item by pressing the arrow next to the menu item

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii created an issue. See original summary.

andrewmacpherson’s picture

Title: Opening dropdown menu using touch on desktop requires a double click » Opening dropdown menu using pointer on desktop requires a double click

This doesn't just affect touch

I replicated this without using a touchscreen device, or the device emulator in Chrome. I was using desktop Linux Firefox. I was using a hover-capable, fine-grained, single pointer (not multi-touch), equivalent to a mouse.

Steps to reproduce, with mouse pointer.

  1. At the start, the buttons are in the aria-expanded="false" states. All the menus appear closed.
  2. As you approach the button, the pointer enters the li which has the mouseover listener. The menu opens, and the button gets the aria-expanded="true" state. Nothing has been clicked yet, and focus has not moved.
  3. Now click the button. It gains focus (expected), and the button state changes to aria-expanded="false". This is actually expected, based on the button state at the end of the previous step; a button click changes the state from expanded to collapsed.
  4. A second click reveals the menu, and the button state changes to aria-expanded="false". Again, this is expected based on the state at the end of the previous step. A click changes the state from collapsed to expanded.

In the steps above, the button is behaving correctly, in that it always swaps collapsed for expanded.

I wonder what's happening with touch? We've seen that a second click results in expanded, so let's assume the first click set it to collapsed. Does the first touch fire a mouseover and click in rapid succession, and it opens and closes to rapidly to see?

We've been following the pattern at Link + Disclosure Widget Navigation fairly closely, but it doesn't anticipate mouse/hover or touch.

Some ideas to try (no idea whether they will be successful):

  • What if we put the mouseover listener on the <a> element, instead of the li element?
  • What if we only add the mouseover listener if the browser reports that a hover-capable pointer is present, via the "hover". I'm unsure about this one, because some touchscreen devices treat a long-press as hover (not sure which devices).
andrewmacpherson’s picture

mherchel’s picture

To me the current behavior with a pointer device is expected.

The question is behavior with large (touch screens). I'm going to experiment a bit.

mherchel’s picture

FileSize
29.65 MB

So... to test this, I modified my local copy of Olivero to bring up the desktop nav at 900px, so I could test on my iPad.

I found that the desktop menu does require a double tap to open the buttons. I'm attaching a video that I took of the issue.

This is a great bug.

mherchel’s picture

After a little debugging, the tap is triggering the mouseover event, which opens the menu. Then it triggers the click event, which closes the menu. This happens instantaneously, so the menu never appears.

mherchel’s picture

mherchel’s picture

Status: Active » Needs review
FileSize
4.82 KB

Patch attached! Video coming shortly

mherchel’s picture

FileSize
29.36 MB

Video showing the correct functionality attached. This is also reproducible within Chrome Dev Tools mobile emulation.

mherchel’s picture

Updated patches that fix the prettier errors.

mherchel’s picture

Title: Opening dropdown menu using pointer on desktop requires a double click » Opening desktop dropdown menu using touch device requires two taps
Issue summary: View changes
Abhijith S’s picture

FileSize
6.36 MB
27.22 MB

Applied patch #10 and it works fine.Adding screenshots below.

Before patch;
before

After patch:
after

RTBC +1

andrewmacpherson’s picture

Priority: Normal » Major
Status: Needs review » Needs work

I guessed it would come to this....



-    button.addEventListener('click', (e) => {
-      const topLevelMenuITem = e.currentTarget.parentNode;
-      toggleSubNav(topLevelMenuITem);
+    button.addEventListener('touchstart', (e) => {
+      e.preventDefault();
+      toggleSubNav(el);
     });


Close, but no cigar.

We can't do that, because it introduces a WCAG level A failure :-(

See WCAG SC 2.5.2 Pointer Cancellation.

Then go back to the MDN article and read what it says about cancelling the touch event.

Then read F101: Failure of Success Criterion 2.5.2 due to activating a control on the down-event

The proposed fix toggles the sub-nav on touchstart. So it isn't possible to cancel opening the sub-nav on a touchscreen.

Bumping to major, since we've run into a level A success criterion.

mherchel’s picture

Interesting. I did not know this.

From what I understand, the touchend event should satisfy the criteria, right?

andrewmacpherson’s picture

Yep, it's that simple!

Manual testing tips.... pretend you don't have accurate fingers. You're simulating a user with Parkinson's tremors.

(To understand why the Pointer Cancellation success criterion matters, compare these manual steps using touchstart and touchend.)

Scenario 1.

  1. Start with no sub-nav open.
  2. Put your finger on a sub-nav button, but don't lift up. touchstart fires on the button.
  3. You didn't mean to hit the sub-nav button. Slide your finger so it isn't near the sub-nav button. Maybe you intended to hit a top-level link, or the search button, so try sliding your finger there.
  4. Lift your finger. touchend fires, but the target isn't the sub-nav button because that's not where your finger was when you lifted it. Hopefully, no sub-nav appears. If you moved your finger to a top-level link, or the search button, that's what should activate.

Scenario 2.

  1. Open a sub-nav.
  2. Put your finger on the screen, outside of the sub-nav, but don't lift up. Hopefully the sub-nav doesn't disappear.
  3. Slide your finger so it is over the a link in the sub-nav.
  4. Lift your finger. touchend fires on the sub-nav link. Hopefully the link activates?

BTW, it isn't just about touch. You should do this manual testing with a mouse too.

If you have an iOS device handy, try using the slow pointer accessibility function. (I forget what it's called.) It gives you more time before registering touch events.

andrewmacpherson’s picture

Re. #4:

To me the current behavior with a pointer device is expected.

FWIW, my testing in #2 was using whatever was deployed on the lb.me/olivero demo site at the time.

andrewmacpherson’s picture

Title: Opening desktop dropdown menu using touch device requires two taps » Opening desktop dropdown menu using pointer device requires two taps/clicks

@mherchel - Can you clarify #4 please? Specifically, can you replicate my steps in #2?

I think the general pointer behaviour in #2 is just as much a problem as the touchscreen behaviour. In particular, I'm thinking of pointers which don't need to be "moved" across the space between 2 clicks. Some USB-HID devices pointers make use of absolute coordinates, as do various assistive tech and macro/script tools.

mherchel’s picture

Status: Needs work » Needs review
FileSize
2.6 KB
5.85 KB

It wasn't quite that simple, but I got it done.

I couldn't use the touchend event, because even if i started the touch, and moved it to outside the button, the e.target was still the button. The same behavior happens with touchmove and pointerup.

This patch works around it by implementing touchstart, and within that, it will add a is-touch-event CSS class. If this class is present, the mouseover event will not trigger the menu toggle. This leaves the menu toggling to the regular ol' click event, which works as you would hope it to.

This patch should be good for the initial issue. I'll digest @andrewmacpherson's comments in #15, #16, and #17 and respond tomorrow (as well as post a video).

lokeshsahu’s picture

Status: Needs review » Needs work
FileSize
35.45 KB

This patch is not getting applied @mherchel.
I have attached the screenshot and video for ref.

mherchel’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: +Accessibility
FileSize
41.99 MB
889 bytes
5.88 KB

Not sure why the patch wouldn't apply for you. I only see the screenshot, and not the video. Be sure you're trying to apply it on the latest 9.2.x branch. The automated tests were able to apply the patch, so it should be working.

I'm updating a code comment from the previous patch, fixing prettier errors, and attaching a video of this working properly.

mherchel’s picture

FYI, I played around with scenario 2 in @andrewmacpherson comment in #15

When placing the finger on the menu item, and then sliding it out, the menu does not appear (yay!)

However, if we then slide it back to the menu, it still does not appear (boo). However, this is the same behavior of all buttons that only have a click event. I tested this out on the mobile menu open button, too. So, I'm pretty sure we don't want to modify the default click() behavior of the browser, correct?

mherchel’s picture

I created a tugboat preview to test this at https://3191716-double-tap-qkwltsptphlryh1t3lwvzgwmyi4aipjs.tugboat.qa/

Note that I modified the CSS within this preview (which isn't in the submitted patch) so the desktop menu will load at 1000px, so you can test it in iPad's landscape orientation.

andrewmacpherson’s picture

Issue summary: View changes
Status: Needs review » Postponed

Re. #18 - ah, my mistake. I need to study touch events more.

I think it's worth postponing this until we've implemented the new plan in #3186349-9: Major accessibility problems with Olivero header show/hide feature. The sticky header behaviour is partly to blame for the problem here; specifically, the way the sticky behaviour kicks in without the user asking for it.

Feel free to to keep experimenting here though! We'll still need to make this robust enough for the situation when the user has opted-in to a sticky header.

But let's not commit anything here until the other issue has been addressed. Noting this in the issue summary.

EDIT: I commented on the wrong issue.

mherchel’s picture

@andrewmacpherson I want to make sure you replied to the correct issue here with your previous comment.

I don't think the sticky header issue has any bearing on this issue. Regardless if the header is sticky, you would still require two taps to open the menus.

Thoughts?

andrewmacpherson’s picture

Status: Postponed » Active

You're right. I commented on the wrong issue. Most of #23 was meant for #3190120: Olivero: Focused level-2 nav items should always be in viewport during keyboard navigation..

Browser tabs, pah!

mherchel’s picture

Status: Active » Needs review
mherchel’s picture

Status: Needs review » Needs work

During code sprint with @lauriii, he discovered that if you tap on the link (not the drop down button) the mouseover event will activate the menu right before it navigates away.

mherchel’s picture

Status: Needs work » Needs review
FileSize
5.88 KB

Fixed. There's a minor issue where if you first toggle the button, and then immediately tap the link, it will still open the dropdown before navigating. This is fairly edge case, and is caused by the mouseover event not triggering to add the is-touch-event CSS class

mherchel’s picture

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Tested manually with Chrome, Firefox and Safari. I didn't have an environment that could emulate touch events and runs Edge. I also couldn't find device running Android that would be large enough to use the desktop layout. I was mostly concerned about how consistent the order of triggering the events would be across different browsers. I didn't find any inconsistencies and based on https://www.html5rocks.com/en/mobile/touchandmouse/#toc-1 , it seems like the order should be consistent.

andrewmacpherson’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs accessibility review

#30 says which browsers were used for manual testing, but I'm more interested in what kinds of pointer were used. Has this been tested using any pointers other than a finger on a touchscreen?

In particular, has this been tested with...

  • A hover-capable stylus? Example: a Samsung Galaxy S-pen.
  • Pointer emulation by assistive tech? Examples: a mouse-grid tool ("show grid" in speech control); or the 2-step cross-hair pointer mode in switch access scanning.

I think this could be a lot safer if we put the mouseover event on the top-level &lt;a&gt; rather than the top-level &tl;li&gt;. The mouseout behaviour could remain on the list item, so long as #3192903: Mouseout event should not close navigation sub-menu if focus is inside the sub-menu was addressed.

Covid lockdown is making it awkward to get hold of actual devices. In ordinary times, if I wanted to test on an iPad or S-pen or whatever, I'd go and annoy the staff in a shop.

mherchel’s picture

@andrewmacpherson would you be okay with committing this and opening up followup issues to test with devices that you suggested? They can be stable blockers if necessary.

nod_’s picture

Issue tags: +JavaScript
mherchel’s picture

Status: Needs review » Needs work

I spoke to @andrewmacpherson about this yesterday (among many other things) and the main issue that we're worried about is users of "mouse grid tools" instantaneously triggering the mouseover and click events without triggering the touchstart event.

The best solution that we can think of for this is to introduce a short timer (250ms) after mouseover. If the click event happens within that time, it will not trigger the menu toggle.

mherchel’s picture

Status: Needs work » Needs review
FileSize
2.68 KB
7.23 KB

During testing of the timer, I found that the action is not cancelable as mentioned by @andrewmacpherson in #15.

However, I found that a combination of a touchstart event and mouseover timer satisfies the conditions in what I believe to be a somewhat robust way.

The logic of this new patch is

1. Check for a touchstart event. If this happens, the mouseover event will not do anything.
2. If the touchstart event doesn't trigger, the mouseover event triggers the toggling of the navigation, and inserts a 250ms timer.
3. The click event fires, if the timer is still active, it will not do anything.

Tugboat preview

https://3191716-double-tap-2-6mhkkiinwbq1g2fqng7bcno0ox9ora5x.tugboat.qa/
Note that this preview has the styling modified so it will load the desktop menu at 1000px (enabling us to easily test on an iPad)

andrewmacpherson’s picture

Note that this preview has the styling modified so it will load the desktop menu at 1000px (enabling us to easily test on an iPad)

It would be even better if it used the desktop menu at 500px. I'd be able to test it with an ipod touch (it's the cheapest iOS device) and an Android phone.

I wonder if the Olivero theme settings could provide this. Something like "Enable the mobile menu at these widths: never (not recommended in production websites) | >800px | >1200px | always". Worth a separate issue?

mherchel’s picture

It would be even better if it used the desktop menu at 500px

I can modify the preview.

As far as the theme setting, I'd say it would take a good deal of work because of the core config process (@lauriii warned me these were somewhat arduous)

mherchel’s picture

Patch is still good!

It would be even better if it used the desktop menu at 500px

Here is a Tugboat preview with the changes, as well as changing the navigation change to 500px:

https://3191716-double-tap-500px-j2hfsttnfigrcwvg9woz8cyg2cqob6dq.tugboa...

andrewmacpherson’s picture

Status: Needs review » Reviewed & tested by the community

The approach we came up with in #35 is probably a good way to satisfy the greatest number of users.

Thanks for setting up the Tugboat preview with a breakpoint of 500px in #38. That's allowed me to manually test approach #35 in the following ways:

  1. Using a touch screen device with the wide-breakpoint navigation layout. A single finger-tap on a sub-nav button opens it, which is the original bug report we're trying to solve here. I repeated this with Android/Chrome, Android/Firefox, iOS/Safari, and iOS/Firefox.
  2. Also on these touch screen device/browser combinations, I'm satisfied that we'll pass WCAG SC 2.5.2 Pointer Cancellation. Touching a sub-nav button with a finger, then moving the finger away before lifting, does NOT open it. That's good.
  3. Also on thes touch screen device/browser combintations, I was able to operate the sub-nav buttons using the pointer-mode scanning tools in Android and iOS switch access. This is the method where the user performs a mouse-click by positioning the X and Y coordinates in turn, then confirms that they want a tap there. This worked well.
  4. Based on the foregoing tests, I'm reasonably optimistic (touch wood...) that this will be robust enough for any USB/HID pointer device which uses absolute coordinates.
  5. Using a mouse pointer (hover capable) on desktop Windows Firefox and Chrome. The sub-nav appears on hovering over the top-level list item, and seems quite snappy; I tried very hard to do a mouseover and then click the button before the menu appeared, but never succeeded. The sub-nav opening has an animation, and I think that disguises the 250ms delay before opening on hover. This feels nice for the mouse user without any assistive tech.

The 250ms period was suggested by me. It's a similiar magnitude to that used for palm-detection on laptop touch pads to avoid moving/clicking a pointer while typing, and the filter-keys accessibility feature for avoiding accidental double key presses. It's also a similar period to the debounce timing in some specialized keyboard firmware (such as the tap/hold dual-function key behaviours in QMK firmware).

(Note, we have another issue to disable animations for prefers-reduced-motion but the 250ms delay will still be in place for the mouseover opening. That might feel noticable to some users, but the functionality will still be there.)

So RTBC for this approach. Excellent work, I think, it's pretty darn robust.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/themes/olivero/js/second-level-navigation.es6.js
@@ -108,8 +129,24 @@
+    if (e.key === 'Escape' && isDesktopNav()) {

Based on https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key, this doesn't work in IE 11 because they use different key for the escape key.

mherchel’s picture

Status: Needs work » Needs review
FileSize
7.31 KB
1.2 KB

Good catch! I didn't realize that. Updated patch attached.

katannshaw’s picture

Status: Needs review » Reviewed & tested by the community

Patch #41 works great. Marking RTBC.

mherchel’s picture

After testing, I updated the mouseover timeout to 500ms.

lauriii’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs accessibility review

The accessibility review was done in #39.

Tested this manually with @mherchel using MacOS switch control. On #43 it was working as expected.

Committed 5a389e3 and pushed to 9.2.x. Thanks!

  • lauriii committed 5a389e3 on 9.2.x
    Issue #3191716 by mherchel, Abhijith S, lauriii, andrewmacpherson,...

Status: Fixed » Closed (fixed)

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