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
Comment | File | Size | Author |
---|---|---|---|
#43 | interdiff-41-43.txt | 1000 bytes | mherchel |
#43 | 3191716-43.patch | 7.31 KB | mherchel |
#41 | interdiff-35-41.patch | 1.2 KB | mherchel |
#41 | 3191716-40.patch | 7.31 KB | mherchel |
#35 | 3191716-35.patch | 7.23 KB | mherchel |
Comments
Comment #2
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedThis 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.
aria-expanded="false"
states. All the menus appear closed.li
which has the mouseover listener. The menu opens, and the button gets thearia-expanded="true"
state. Nothing has been clicked yet, and focus has not moved.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.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):
<a>
element, instead of theli
element?Comment #3
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedComment #4
mherchelTo 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.
Comment #5
mherchelSo... 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.
Comment #6
mherchelAfter a little debugging, the tap is triggering the
mouseover
event, which opens the menu. Then it triggers theclick
event, which closes the menu. This happens instantaneously, so the menu never appears.Comment #7
mherchelThis looks straightforward to resolve! https://developer.mozilla.org/en-US/docs/Web/API/Touch_events/Supporting...
Comment #8
mherchelPatch attached! Video coming shortly
Comment #9
mherchelVideo showing the correct functionality attached. This is also reproducible within Chrome Dev Tools mobile emulation.
Comment #10
mherchelUpdated patches that fix the prettier errors.
Comment #11
mherchelComment #12
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedApplied patch #10 and it works fine.Adding screenshots below.
Before patch;
After patch:
RTBC +1
Comment #13
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedI guessed it would come to this....
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.
Comment #14
mherchelInteresting. I did not know this.
From what I understand, the
touchend
event should satisfy the criteria, right?Comment #15
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedYep, 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
andtouchend
.)Scenario 1.
touchstart
fires on the button.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.
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.
Comment #16
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedRe. #4:
FWIW, my testing in #2 was using whatever was deployed on the lb.me/olivero demo site at the time.
Comment #17
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented@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.
Comment #18
mherchelIt 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, thee.target
was still the button. The same behavior happens withtouchmove
andpointerup
.This patch works around it by implementing
touchstart
, and within that, it will add ais-touch-event
CSS class. If this class is present, themouseover
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).
Comment #19
lokeshsahu CreditAttribution: lokeshsahu at OpenSense Labs commentedThis patch is not getting applied @mherchel.
I have attached the screenshot and video for ref.
Comment #20
mherchelNot 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.
Comment #21
mherchelFYI, 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?
Comment #22
mherchelI 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.
Comment #23
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedRe. #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.
Comment #24
mherchel@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?
Comment #25
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedYou'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!
Comment #26
mherchelComment #27
mherchelDuring 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.
Comment #28
mherchelFixed. 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 theis-touch-event
CSS classComment #29
mherchelUpdated tugboat: https://3191716-28-bonxnne2pn4shbxog7scqtpxln5ogntl.tugboat.qa/
Comment #30
lauriiiTested 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.
Comment #31
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented#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...
I think this could be a lot safer if we put the mouseover event on the top-level
<a>
rather than the top-level&tl;li>
. 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.
Comment #32
mherchel@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.
Comment #33
nod_Comment #34
mherchelI 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.
Comment #35
mherchelDuring 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 andmouseover
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, themouseover
event will not do anything.2. If the
touchstart
event doesn't trigger, themouseover
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)
Comment #36
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedIt 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?
Comment #37
mherchelI 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)
Comment #38
mherchelPatch is still good!
Here is a Tugboat preview with the changes, as well as changing the navigation change to 500px:
https://3191716-double-tap-500px-j2hfsttnfigrcwvg9woz8cyg2cqob6dq.tugboa...
Comment #39
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commentedThe 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:
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.
Comment #40
lauriiiBased 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.
Comment #41
mherchelGood catch! I didn't realize that. Updated patch attached.
Comment #42
katannshaw CreditAttribution: katannshaw at Lullabot commentedPatch #41 works great. Marking RTBC.
Comment #43
mherchelAfter testing, I updated the mouseover timeout to 500ms.
Comment #44
lauriiiThe 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!