Submenus can't be dismissed on mobile.

[obenland]

When the site-name submenu pane is active, it covers the viewport and there is no way to dismiss it. All tapping will lead to a screen in the admin.

First reported in: ​https://make.wordpress.org/flow/2014/07/07/the-guises-of-bar-nexus-5-4-0-alpha-20140706/screenshot_2014-07-07-07-41-58/

[dancameron]

Correctly position dropdown allowing for scrolling. Disable the dash-icon link back to admin.

[dancameron]

Here's an example of the issue I found when working on this...since the drop was set to fixed the user couldn't scroll to unseen menu items.

[dancameron]

When working on this I noticed that the .ab-sub-wrapper is set to position: fixed;, this will cause the menu to essentially take over any small screens and on somewhat larger screens (see example above) the menu would stick and break away from the header menu. My patch attached will set it to position: absolute;, which will fix the weirdness but will also provide a way for small screens to get out of the drop-down submenu (e.g. scroll down and touch the content).

Something not necessary in the patch is to disable the dash-icon that links to the dashboard on these smaller screens. Seems redundant and unnecessary and if the user expects the icon to close the drop-down they're now not unexpectedly sent to the admin.

I wanted to ask two things before proceeding:

1. Should we disable that icon?
2. Should we go further and allow for that icon to close the submenu on smaller screens and/or enable a touch to the other areas of the admin bar close it?

IMO # 1 is warranted and a UX argument could be had. As for # 2 I'm mixed since IMO the current patch fixes the UX and UI bug.

[dancameron]

I just noticed #29905, which fixed the sub-menu scroll issue weeks ago. Anyway...

[pento]

#30637 was marked as a duplicate.

[ryan]

1. Should we disable that icon?
2. Should we go further and allow for that icon to close the submenu on smaller screens and/or enable a touch to the other areas of the admin bar close it?

Any toolbar icon that has a submenu should open and close that submenu and do nothing else. Closing open toolbar menus by tapping on empty areas of the toolbar would be nice. G+ does this.

[DrewAPicture]

1. Should we disable that icon?
2. Should we go further and allow for that icon to close the submenu on smaller screens and/or enable a touch to the other areas of the admin bar close it?

Any toolbar icon that has a submenu should open and close that submenu and do nothing else. Closing open toolbar menus by tapping on empty areas of the toolbar would be nice. G+ does this.

[seanchayes]

I've created a basic patch 29906-2.diff that emulates the desired functionality in the original ticket request.

With the site-name submenu active and the patch in place a touch event on a blank location within the admin bar will remove the displayed site-name submenu and the user will remain on the current screen/view.

Additionally, during the same touch event the if the touch is in another area of the adminbar occupied by an icon the following actions will take place dependent on the touch location:

• trigger the action on the admin bar if the touch is on the comment icon - there is no dropdown associated with this menu item
• trigger the new content dropdown menu if the touch is on the wp-admin-bar-new-content icon (+)
• trigger the my account dropdown menu if the touch is on the wp-admin-bar-my-account icon

Any testing / feedback welcome

[coreymcollins]

Tested this and it's working great for me. Clicking anywhere in the empty space of the toolbar once the Dashboard menu is expanded closes the menu and does not redirect the user to the Dashboard.

[ryan]

Tapping a toolbar icon when the submenu is open should close the submenu, not follow a link. Tap Dash icon to open the submenu, tap the dash icon again to close the submenu. This is a common interaction and what I expect on mobile. The first item in all of our submenus duplicates the link used in the toolbar icon, so losing the links in the icons won't close any roads.

29906-2.diff works well for me. Add in the above and our toolbar will behave like other mobile sites and apps with toolbars.

​https://make.wordpress.org/flow/2014/09/30/toolbar-menus-on-touch-devices-nexus-5-4-1-alpha-20140930/

[ryan]

The toolbar was designed for menus that fly open on mouse over. The home icon has a submenu with just one item, Visit Site. This single item menu was added way back when to help document that the site name was a view site link. This was before the Home icon existed, IIRC, and folks had trouble finding view/visit site without some help. So, the site name toolbar item gained a menu with a lonely Visit Site link that would flash up when you ran your mouse over the toolbar. Discoverability was somewhat improved. Given that mouse over doesn't apply to touch devices and that we now have a big touchable home icon, perhaps Visit Site could be retired from the home menu, at least for touch devices. The home icon would become single tap, like the comment icon.

[seanchayes]

I have addressed the comments from @ryan in comment:15 and have wrapped it into another patch that is an evolution of 29906-2.diff. I've called it 29906-3-comment15.diff highlighting the association with both the original ticket and the additional focus this ticket could benefit from and suggested by @ryan.

I've tried and tested it but really welcome further testing from anyone who can and see it it hits what comment:15 alludes to.

[SergeyBiryukov]

Please use tabs rather than spaces for indentation (per ​WordPress Coding Standards).

[seanchayes]

Updated patch - tabs not spaces - and with Multisite navigation / submenu support. 29906-5-comment15.diff

Please test / improve.

[ryan]

Patch needs refresh.

patching file src/wp-includes/css/admin-bar.css
Hunk #1 FAILED at 1053.
1 out of 1 hunk FAILED -- saving rejects to file src/wp-includes/css/admin-bar.css.rej

[seanchayes]

If I've understood correctly I need to supply an updated / refreshed patch.

I have attached a refreshed 29906-6-comment15.diff

[ryan]

I gave 29906-6-comment15.diff a quick run through on an iPhone 6+. If you tap a blank area of the admin bar to dismiss a menu, your next tap on the admin bar gets eaten. Example: Tap the Dashboard icon. Tap an empty area of the admin bar to dismiss Dashboard. Tap +. The tap causes a brief blink of the active color on the icon but otherwise does nothing. A second tap on + brings up the menu. Other than that, looks good on iPhone 6+.

Removing the links from the icons on desktop needs more discussion. Removing the links is good behavior on mobile, but our desktop behavior has long been to use the admin bar icons as both menu actuators (on mouse entry) and action buttons (when clicked). This should be a conscious change, if we do it. If we made desktop behave like mobile, I'd probably want to lose the Visit Site menu from the Home icon (wp-admin-bar-site-name) so that the home icon could be used like a button (like the Comments icon). If we don't want to match mobile and desktop right now, can we support both behaviors conditionally?

[seanchayes]

I am struggling to duplicate the exact scenario you describe in paragraph 1. But I will happily continue to see where an issue like that is manifesting itself.

For the second point I concur about removing the links.

I was thinking about that when I did it but with your feedback perhaps there is a way the urls/links can be retained for the non mobile / desktop experience.

I'll try and restore the urls/links for non-mobile and provide another version of the patch. But if the feedback is for a consistent experience whether mobile or not then we can do that instead and make the other menu adjustments you suggest.

[samuelsidler]

No movement for a few weeks. This ticket is at risk of getting punted.

[seanchayes]

Better version, now 29906-8-comment15.diff. A long shot but it might be there.

I have restored some of the links I'd removed by placing wp_is_mobile around them so the desired effect is achieved in both worlds. #comment:23

I think the eaten touch/click/event has been resolved #comment:23

I still welcome review and testing.

[ryan]

I tested with Macnchrome and an iPhone 6+. Both behave as expected. I can still reproduce the eaten touch event after tapping empty space in the bar. I could live with that though to get the rest. Or we could lose that feature and try again in 4.3.

[ryan]

I tried an iPhone 5 with the same results as on the 6+.

On a Nexus 5 with Android 5.1, tapping toolbar icons to close open menus doesn't always work. Home works reliably, but Dashboard and + visit the link instead of closing the menu. Tapping in empty space in the toolbar to dismiss works and doesn't have the tap eating bug that iOS has.

[seanchayes]

As far as I can tell it could be based a little on what I politely term as "fat finger" syndrome. I too can replicate the issues on those navigation items.

My test was with careful and repeated taps on "+" and the "Dashboard" navigation items and I found I could easily trigger the menu action of the first item in the drop down if my finger was a little below the "optimum" placement for the respective icon.

So, to help mitigate this a little, I adjusted the markup for the Dashboard and "+" navigation toggles with out affecting their main purpose nor their markup and appearance on non-mobile displays. I felt the chances were reduced but can't be 100% certain.

If anyone has fingers available to help testing that would be great.

Patch 29906-9-comment15 is attached to this ticket with these latest modifications.

Feedback welcome.

I also included a fix for #31778 that will also benefit from review / testing.

[ryan]

Whoops, already out-of-date.

patching file wp-admin/js/common.js
Hunk #1 FAILED at 721.
1 out of 1 hunk FAILED -- saving rejects to file wp-admin/js/common.js.rej

[seanchayes]

Darn. And it was the additional line included for #31778.

I have refreshed and attached the patch as 29906-10-comment15.diff

[ryan]

Looking good on an iPhone 6+. Double tap after tapping empty toolbar to dismiss is all that remains. Not worried about it. I'll test on my usual five device array later today.

[ryan]

Still having problems on Nexus 5. Tapping to dismiss still sometimes follows the link, especially when tapping + from the front page of the site.

[seanchayes]

Good and bad. I have a Nexus 5.

What's the "double tap" issue? I can't recall. Steps to re-create would be great (if poss).

[ryan]

Replying to seanchayes:

Good and bad. I have a Nexus 5.

What's the "double tap" issue? I can't recall. Steps to re-create would be great (if poss).

When you tap an empty part of the toolbar to dismiss on open menu, the next tap on a toolbar icon gets eaten. Two taps on an icon are required. This is specific to iOS.

[helen]

Realizing that this is a late juncture - would this perhaps be a place where we'd be better off taking a look at handling the capability rather than size/type of device? Which could be an approach that detects touch capability in JS and removes/doesn't activate the top level links for items with a submenu but toggles the submenus instead.

[DrewAPicture]

After talking this over with @helen, I agree that simply swapping out the link using wp_is_mobile() is not the best solution. I also agree that we would be far better served to develop a solution based on actual device capability, re: comment:41. Punting.

[ryan]

This hurts flow through the toolbar on touch devices. That's a big deal. Moving this to 4.3 so we can up our mobile competency.

[stephdau]

So, I've been looking into detecting device capabilities in regards to touch, and after an easy path (touch events) last week, I basically spent yesterday screaming at the web and my laptop (touch screens)...

At the core of the issue is: browser APIs will let us detect touch events, but not the presence of a touch screen in and of itself.

Detecting touch events is all well and fine in the mobile context, but we're at a loss to detect a device that runs a standard desktop OS in combination with touch capabilities (think MS Surface, for example). This is okay in most contexts, but such a device with no other pointing device will basically be out of luck... And there's nothing we can seem to do about it via capability detection.

Modernizr, for example, used to document their touch detection as detecting touch (screens),but they have since moved to clarifying that they also only detect touch events (countless discussions online documenting the same reason as us as to why), and more: see comments at ​https://github.com/Modernizr/Modernizr/blob/master/feature-detects/touchevents.js#L19

For the record, we also can't complement touch events detection with other parameters when the former fails, since we can't even deduct the presence of a touch screen through the user-agent string, or the like...

So, in conclusion, we can most definitely do touch event detection (where I'd even go as far as making it globally accessible to all WP JS, and base our menu behavior based on that, and be mostly successful at it. But some devices will be left out.

[stephdau]

EG of elaborate (but not complex) touch event detection: ​https://github.com/Modernizr/Modernizr/blob/master/feature-detects/touchevents.js#L38

Detailed article on the state of detecting a touch screen: ​http://www.stucox.com/blog/you-cant-detect-a-touchscreen/

[stephdau]

After all the headaches, 29906.2.diff​ is the most elegant way I could find to achieve the ticket's goal: will let touch-events users close the menu when opened by clicking on header genericon. That's the solution I'd advise to solve this.

[stephdau]

... but it also needs to deal with the focus background and color of the genericon, as they remain highlighted upon closing,likely because it's getting focused (but blur didn't have an effect in my tests), unlike when one clicks outside the menu.

[stephdau]

Better targeting.

[stephdau]

29906.4.diff​ has better targeting and uses .not(.mobile) (CSS3) to make sure the icon's highlight background and color are cleared when clicking to close on mobile. This is because the element still gets :focus() and :hover() on click.

What remains is to clear the blue color on the icon still applied through :before after the rest is handled. This is actually a long standing bug, as the "hamburger menu" icon suffers from the same issue regardless of this work.

To fix this, we'll have to deal with the same (:focus and :hover) in this monster in admin-bar.css, likely through a combination of ignoring/including .mobile vs .hover: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/css/admin-bar.css#L271

#wpadminbar .quicklinks .menupop ul li a:hover,
#wpadminbar .quicklinks .menupop ul li a:focus,
#wpadminbar .quicklinks .menupop ul li a:hover strong,
#wpadminbar .quicklinks .menupop ul li a:focus strong,
#wpadminbar .quicklinks .menupop.hover ul li a:hover,
#wpadminbar .quicklinks .menupop.hover ul li a:focus,
#wpadminbar.nojs .quicklinks .menupop:hover ul li a:hover,
#wpadminbar.nojs .quicklinks .menupop:hover ul li a:focus,
#wpadminbar li:hover .ab-icon:before,
#wpadminbar li:hover .ab-item:before,
#wpadminbar li a:focus .ab-icon:before,
#wpadminbar li .ab-item:focus:before,
#wpadminbar li.hover .ab-icon:before,
#wpadminbar li.hover .ab-item:before,
#wpadminbar li:hover #adminbarsearch:before,
#wpadminbar li #adminbarsearch.adminbar-focused:before {
	color: #00b9eb;
}

@helen: I could definitely use some help with that last part.:)

[stephdau]

Keep in mind patches are iterations on finding a solution, not yet a recommendation to commit. Hoping for the latter soon, based on feedback.

[designsimply]

I tested 29906.4.diff on WordPress 4.3-alpha-32841 using Safari iOS 8.3 on iPhone 6 and didn't find any blockers.

Screencast: ​3m42s
^{(sorry about the weird mic noise, this is the first time testing a new tool)}

My testing checklist:

1. Tap each top level menu item to open it and tap it again to close
2. Tap the empty black bar and then tap any icon (takes to taps to open it at that point)
3. Tap outside of an open menu (it should close)
4. Try to get two toolbar menus open at once (that shouldn't happen)
5. Test on desktop to see if anything seems off there

Nitpicks

• If you tap in the empty part of the menu and then tap an icon, you have to tap twice to open a menu after that.
• If you tap the triple bar icon (hamburger), you can open other menus on top of it.
• Icon remains blue after you open and close one of the menus.

Say the word and I'll open separate tickets for the nitpicks.

I don't think any of the nitpicks are blockers and would love to see this ship because it's a really nice improvement!!

[stephdau]

Replying to designsimply:

Thanks for the detailed testing, Sheri!

• If you tap in the empty part of the menu and then tap an icon, you have to tap twice to open a menu after that.

Annoying. :) That must be iOS-specific, I'll see what I can do Monday, when I have a device.

• If you tap the triple bar icon (hamburger), you can open other menus on top of it.

This can be addressed. It currently behaves as it would on desktop, if you were to collapse/expand the sidebar. Closing it while opening the others seems to make sense when dealing with touch events. Can do.

• Icon remains blue after you open and close one of the menus.

Yeah, that's what I asked help with in 51. At least we know where the issue is in the code. :)

Say the word and I'll open separate tickets for the nitpicks.

Let me spend some time on the 2 latter ones today, and we'll see if they deserve their own tickets. I believe Helen is still at a conference today,so I don't think they'd get committed today anyway.

[seanchayes]

I looked at the original reported issue using 4.3-alpha-32280.

The issue is slight more prevalent on landscape compared to portrait on either Android or iOS due to the amount of screen real estate occupied by the submenu.

The screenshot originally provided shows the issue best and the ticket mentions there's no way to dismiss the site-name submenu without ending up at an admin screen. The site-name parent navigation item is a link so tapping it again and having the link followed I would think is expected behavior. Tapping somewhere else that is not on the site-name navigation menu should dismiss the submenu.

In landscape, based on the screenshot and some tested I've been able to perform on an iPhone and and Android truly there was no where to tap that would just remove the submenu or dismiss it. In portrait there is often enough screen real estate to allow for a tap/click

With that in mind I applied 29906.4 and oriented my devices in landscape and concur that there are double taps in some circumstances.

1. Tapping to make the site-name submenu appear works and immediately tapping the site-name logo removes the dropdown and does not follow its related link. Meets the needs of the ticket description.
2. Tapping to make the site-name submenu appear works and tapping any area on on the admin bar prior to tapping the site-name to remove it the requires a second tap on the site-name logo before the submenu is removed. Once the submenu is removed, any further taps on the admin bar prior to tapping the site-name logo still require an additional tap on the site-name logo before the submenu is displayed. On iOS a flicker is present on the admin bar when tapping a blank space. Side effect requiring additional taps to progress.
3. After tapping to make the site-name submenu appear you can tap, hold and drag the submenu to reveal the body of the page and if you then tap in the body of the page the submenu is removed. Mimicking current behavior.
4. On both iOS and Android tapping a blank area on the admin bar doesn't dismiss the submenu. Mimicking current behavior.

(1 & 2 the focus color is retained on the site-name logo until you tap elsewhere within the display. For 3. the focus color is removed)
A side effect is that tapping a top level menu item like the site-name logo, the profile/avatar top level menu or the "plus" doesn't follow the link present in those items. So you can't get to those links.

I reviewed my patch 29906-2 and applied that repeating the tests above.

1. Tap, submenu appears, tap again and the link is followed mimicking current behavior.
2. Tap, submenu appears, tap elsewhere on the admin bar the submenu is dismissed - new behavior and meeting the needs of the ticket
3. Same behavior. Mimicking current behavior.
4. Active submenu is dismissed. Meets the needs of the ticket description

I didn't encounter any double taps although previously "eaten taps" were reported when 29906-2 was applied - I didn't encounter "eaten taps".

We've previously discussed / commented on the results of the tapping action on the top level menu item - should it just dismiss the menu or follow the link built into it. I think that's an important distinction / decision and could be addressed perhaps by reviewing the mobile navigation as a whole perhaps but not addressed or changed as a result of code in this ticket. We did cover that in the -comment15 portion of this ticket with some sample code.

Aside from that the submenu can be more easily dismissed by allowing the user to tap elsewhere on the admin bar to dismiss a submenu compared to actions required prior to the patch

However, I think with minor caveats each, between 29906-2 and 29906.4 we should have an acceptable solution for this ticket this time around.

[stephdau]

29906-2.diff​ + 29906.4.diff​ + tabs->spaces for 29906-2.diff, see 61

[stephdau]

Replying to seanchayes:

However, I think with minor caveats each, between 29906-2 and 29906.4 we should have an acceptable solution for this ticket this time around.

I agree. See 29906.5.diff​ (which description should have read spaces->tabs, not tabs->spaces, for the record).
Thanks for the very thorough feedback. :)

[stephdau]

Before commit: from a CSS perspective, should we use #wpadminbar:not(.mobile) as I did (modifying existing rules), or should we leave the existing ones as is, and add new ones for #wpadminbar:not(.mobile) right after, re-setting the bg and text color to the original colors?

I'm on the fence on this one. Current modification means older, non-CSS3 desktop browsers will no longer get the current assignment, which they would if we left the CSS as is. Adding new rules has the usual set of implementation unknowns.

[obenland]

I'd be fine with not catering to those older browsers.

[seanchayes]

Here's what I'm seeing with 29906.5.diff on iOS

When I tap the sitename nav element the submenu appears. I tap the admin bar and the submenu is dismissed and the focus coloring is remove from the sitename nav element. If I tap the sitename nav element immediately after then the tap is "eaten", the focus coloring still exists. If I then immediately tap the admin bar the focus coloring is removed. If I then tap again on the sitename nav element the experience repeats although this time I'll tap again on sitename nav element and the submenu appears. At this time I can tap the sitename nav element and the submenu is dismissed.

The by-product of this patch is that the original links for sitename, "+" and avatar nav elements are not available on mobile devices. Those links, however, can be accessed elsewhere in the available nav elements. And the focus coloring remains on the touched nav element - current behavior would take you to that link.

On Android there is no "eaten" tap and all other behavior is the same.

So, I have prepared 29906.6.diff that turns off the blue highlight color after you open and close a menu (see comment:51). However, the "eaten" tap on iOS is still apparent :-(

[seanchayes]

And I removed 2 lines of code that are unreachable due to inclusion of changes from 29906-2.diff

[designsimply]

Replying to seanchayes:

Here's what I'm seeing with 29906.5.diff on iOS
When I tap the sitename nav element the submenu appears. I tap the admin bar and the submenu is dismissed and the focus coloring is remove from the sitename nav element. If I tap the sitename nav element immediately after then the tap is "eaten", the focus coloring still exists. If I then immediately tap the admin bar the focus coloring is removed. If I then tap again on the sitename nav element the experience repeats although this time I'll tap again on sitename nav element and the submenu appears. At this time I can tap the sitename nav element and the submenu is dismissed.

I would summarize this as:

• Tap the empty black bar and then tap any icon (takes to taps to open it at that point)

I think this issue is an edge case bug and not a blocker for 4.3, and I think we should open a separate ticket for it if it's still a problem after #29906 is closed.

The by-product of this patch is that the original links for sitename, "+" and avatar nav elements are not available on mobile devices. Those links, however, can be accessed elsewhere in the available nav elements. And the focus coloring remains on the touched nav element - current behavior would take you to that link.

This sounds good. From what I understand, this is the expected behavior.

So, I have prepared 29906.6.diff that turns off the blue highlight color after you open and close a menu (see comment:51). However, the "eaten" tap on iOS is still apparent :-(

I tested 29906.6.diff and the blue icon color is turned off after closing the dashboard, home, and new post icons but the hamburger icon never turns blue at all like it used to. I tested with Safari iOS 8.3 on iPhone 6 with WP 4.3-alpha-32943 + 29906.6.diff. @seanchayes, does the hamburger icon still turn blue when you test 29906.6.diff on iOS? Is that something you can fix?

[helen]

In 33056:

Toolbar: Allow submenus to be closed with a second tap on touch devices.

Also closes submenus when the admin menu is toggled open, as it opens below and thus can be obscured.

props stephdau, seanchayes.
see #29906.

[helen]

I left this open because there's still the issue of taps getting "eaten" when another item has been tapped first. This is generally a problem on iOS Safari related to things like hover states, but we should take at least a few minutes to see if we can figure it out quickly, otherwise open a separate ticket for more generalized investigation.

[obenland]

Replying to helen:

otherwise open a separate ticket for more generalized investigation.

Looks like this is where we're headed with this?

[helen]

Replying to obenland:

Replying to helen:

otherwise open a separate ticket for more generalized investigation.

Looks like this is where we're headed with this?

Probably - I'll see if somebody is up for looking at this again, but based on the headache I have after looking at this fruitlessly for a few hours and what I had to do in 4.2, we'll probably have to survive with the double taps for release and just keep working on this separately. My first step is to strip this down to a minimal test case.

[seanchayes]

FWIW this weekend I looked to see if I could identify the 'eaten' taps and if there was any scope to remove/address/fix them but came up blank.

As you've said before helen, in comment:41 perhaps that would be the best approach.

And with ryan's comments on comment:15 perhaps there's some scope in the future to address markup improvements that would help alleviate some portion of this issue.

[helen]

In 33284:

Toolbar: Ensure icons are colored correctly in alternate schemes.

props ryelle.
fixes #32931. see #29906.

[Ipstenu]

Changeset #33056 has broken #18758 (Click empty admin bar to scroll to top).

Looks like it's _just_ the second part of https://core.trac.wordpress.org/changeset/33056#file3

If I remove this it works:

			} else {
				adminbar.find( 'li.menupop.hover' ).removeClass( 'hover' );
				e.stopPropagation();
				return;

I'm not JS savvy enough to go past that.

[obenland]

In 33334:

Toolbar: Restore scroll-to-top functionality.

H/t Ipstenu.
Introduced in [33056].

See #29906.

[helen]

Spun off double tap issues into #33087.

[33334] seems a little off to me, as it scrolls you back to top upon closing a submenu by tapping on the toolbar. At 600px wide it's at the top of the page anyway, so that's not a big deal, but from 601px to 782px it's still fixed, so it's a little weird. I think it's okay to stay that way, given that it would currently scroll up anyway and you can now tap the link again to close the submenu, but wanted to note it.

[helen]

#31589 was marked as a duplicate.