WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 17 months ago

#20614 closed enhancement (fixed)

Making the menus work in Windows Phone 7, and work better in Android / iOS

Reported by: georgestephanis Owned by: azaozz
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4
Component: Toolbar Keywords: has-patch mobile needs-testing
Focuses: Cc:

Description (last modified by helenyhou)

Currently, on Windows Phone 7/7.5, the Toolbar Dropdown menus are non-functional (as there is no ontouchstart functionality in the browser) and the admin flyout menus are also entirely non-functional. This translates roughly to ... there is no way to actually log out of WordPress! (as logout is in the user dropdown menu)

iOS has some built in :hover faking code, but we should also make the admin menu flyouts more accessible and usable on Android -- where you have to touch and hold to activate the flyout/dropdown, and then slide your finger off to not trigger the actual link.

Attachments (12)

20614.diff (3.4 KB) - added by georgestephanis 2 years ago.
20614.1.diff (3.3 KB) - added by georgestephanis 2 years ago.
Whoops! I forgot to use wp_is_mobile() at one place.
20614-3.patch (6.9 KB) - added by azaozz 18 months ago.
20614-4.patch (7.9 KB) - added by azaozz 18 months ago.
Touchstart works better than click on touch screen devices
20614-alternate-3.patch (985 bytes) - added by lessbloat 18 months ago.
20614-5.patch (8.0 KB) - added by azaozz 18 months ago.
Having stopPropagation() when "touching" menu parents works well but is not ideal in case another script uses touch events in the menu or toolbar
20614-6.patch (8.1 KB) - added by azaozz 18 months ago.
20614.7.diff (4.4 KB) - added by georgestephanis 18 months ago.
20614.7-tabs.diff (3.6 KB) - added by DrewAPicture 18 months ago.
spaces to tabs
20614.7-tabs.2.diff (4.0 KB) - added by SergeyBiryukov 18 months ago.
Additional tab fixes
20614-8.patch (7.1 KB) - added by azaozz 18 months ago.
20614-9.patch (8.9 KB) - added by azaozz 17 months ago.

Download all attachments as: .zip

Change History (57)

georgestephanis2 years ago

comment:1 georgestephanis2 years ago

  • Keywords tableteers mobile added

georgestephanis2 years ago

Whoops! I forgot to use wp_is_mobile() at one place.

comment:2 georgestephanis2 years ago

  • Keywords changed from has-patch tableteers, mobile to has-patch tableteers mobile

The issue that this solves is that currently, on Windows Phone 7/7.5, the Admin Bar Dropdown menus are non-functional (as there is no ontouchstart functionality in the browser) and the admin flyout menus are also entirely non-functional. This translates roughly to ... there is no way to actually log out of WordPress! (as logout is in the user dropdown menu)

iOS has some built in :hover faking code, but this steps it up a bit, and also makes the flyouts more accessible and usable on Android -- where previously you had to touch and hold to activate the flyout/dropdown, and then slide your finger off to not trigger the actual link.

This way, if it's hidden, the first click will show it, and prevent the link from going anywhere. Huzzah!

Version 0, edited 2 years ago by georgestephanis (next)

comment:3 helenyhou2 years ago

  • Component changed from UI to Toolbar
  • Description modified (diff)

comment:4 georgestephanis2 years ago

It's not just the toolbar, btw -- it's the admin menu as well.

comment:5 georgestephanis2 years ago

this is something of a continuance from #20013

comment:6 maxcutler2 years ago

I have tested this on WP7 again and confirmed that it works as advertised.

This issue has been around since at least 3.3 and prevents users from logging out since "Log out" is hidden in a flyout menu (when touching "Howdy, <x>", it executes a page navigation before the flyout can appear).

comment:7 maxcutler2 years ago

  • Cc maxcutler added

comment:8 azaozz2 years ago

Imho the best way to do this is to use "states" on the links when on a mobile device. The JS would be similar to the patch above and use an additional class on the link to keep its "state". The first click (or touch, or tap) would open the submenu if any and the second will follow the link:

( if is_mobile )

$('#wpcontent').bind('click.adminbar-mobile', function(e){
  var target = $( e.target );

  if ( target.parent().parent().hasClass('ab-top-menu') ) {
    // target is top level toolbar menu item
    if ( target.hasClass('touched') ) {
      return true; // follow the link
    } else {
      target.addClass('touched');
      target.parent().addClass('hover');
      e.preventDefault();
    }
  } else if ( target.hasClass('wp-menu-image') || target.hasClass('menu-top') ) {
    // target is top level admin menu item
    ...
  }
});

Alternatively we can disable the links for top level menu items completely and make them work only as toggles for the submenus. This may be preferable on mobile devices.

Will make a full patch probably tomorrow.

Last edited 2 years ago by azaozz (previous) (diff)

comment:9 georgestephanis2 years ago

Azaozz -- any word on this? Bit of a time crunch here, as I understand it, if it's to get in.

comment:10 nacin2 years ago

  • Keywords 3.5-early added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

Will make a full patch probably tomorrow.

comment:11 azaozz21 months ago

The problem here is that iOS and (afaik) the newest Android browsers work really well without any JS. In order for us to support older/different mobile browsers we have to disable the default behavior in the browsers that work which is probably not the best thing for them, and replace it with JS.

The alternative is to try to detect which mobile browsers work or don't work but that's quite hard as there isn't a direct test for this functionality.

comment:12 azaozz21 months ago

  • Milestone changed from Future Release to 3.5

comment:13 azaozz21 months ago

  • Keywords 3.5-early removed

comment:14 lessbloat20 months ago

Discussed in IRC today (https://irclogs.wordpress.org/chanlog.php?channel=wordpress-ui&day=2012-08-29&sort=asc#m54995). Azaozz said, "that needs refresh after the menu html changed".

Last edited 20 months ago by lessbloat (previous) (diff)

comment:15 nacin20 months ago

  • Keywords needs-refresh added; tableteers removed

comment:16 nacin19 months ago

  • Keywords punt added

This sounds like a punt at this point, with no activity for some time. But, if it is revived during the 3.5 beta period, it can be re-considered.

comment:17 azaozz18 months ago

Seems iOS6 changed something making this not-so-great there too. Should probably be considered for 3.5.

azaozz18 months ago

azaozz18 months ago

Touchstart works better than click on touch screen devices

comment:18 lessbloat18 months ago

Was working on this this afternoon, then went away for an hour before finishing, and when I returned, azaozz had beat me to it ;-). Figured I'd upload mine anyways (in 20614-alternate-3.patch), since I was almost done.

Disclaimers: All mine does is prevent the click on the main menu link, once the sub menu has expanded (and I only tested it on my iPad).

Last edited 18 months ago by lessbloat (previous) (diff)

comment:19 follow-up: georgestephanis18 months ago

Windows Phone doesn't have ontouchstart events. It needs click.

azaozz18 months ago

Having stopPropagation() when "touching" menu parents works well but is not ideal in case another script uses touch events in the menu or toolbar

comment:20 in reply to: ↑ 19 azaozz18 months ago

Replying to lessbloat:

Was working on this this afternoon, then went away for an hour before finishing, and when I returned, azaozz had beat me to it ;-).

Ugh, sry about that. Should have coordinated in #wordpress-dev.

Replying to georgestephanis:

Windows Phone doesn't have ontouchstart events...

Don't have one to test with. Click works so-so in iOS and Android but touchstart is a lot better. Perhaps we could detect Win phone and swap the events to "click"?

On the other hand, not sure how well we support Win phone at all. Can the admin be used from there?

comment:21 follow-up: georgestephanis18 months ago

It can mostly, but you can't log out, as the logout link is hidden in a dropdown menu.

I'd worked with Max Cutler on the initial version of this patch and he confirmed it worked perfectly for him under WP7.

comment:22 azaozz18 months ago

In 20614-5.patch:

  • Disable hoverIntent on touch screen devices (it doesn't work well there).
  • Handle opening of submenus on the first touch and following the link on the second touch.
  • Close any open submenus on touch anywhere else on the screen.

This is implemented separately for the admin menu and the toolbar. There is quite a bit of code duplication as we need the toolbar functionality on the front-end too. A compromise would be to handle both the admin menu and the toolbar from admin-bar.js. Could possibly separate this in another JS file, would be a bit unclear as we need to disable hoverIntent in the "standard" JS.

Tested on iPad/iOS6 and Kindle Fire. Need testing on more mobile devices incl. newer Android.

comment:23 in reply to: ↑ 21 azaozz18 months ago

Replying to georgestephanis:

Ok, will try to redo it with "click" to accommodate Win phone or detect it and swap the events for it.

azaozz18 months ago

comment:24 follow-up: azaozz18 months ago

In 20614-6.patch:

  • Changed the JS events to 'click'. Some (newer) versions of Android work only with click too.
  • Added simple detection for Win phone 7-7.5 (IEMobile 7, 8, 9). Win phone 8/IEMobile10 that comes out in a few days supports the touch API.

comment:25 azaozz18 months ago

In [22262]:

Make the admin menu and toolbar work well on mobile devices, props georgestephanis, see #20614

comment:26 in reply to: ↑ 24 azaozz18 months ago

Replying to azaozz:

Chatting with @georgestephanis in #wordpress-dev, IEMobile 10 has a touch API but it's different, so extended the simple detection to catch it too.

comment:27 DrewAPicture18 months ago

  • Cc xoodrew@… added
  • Keywords needs-refresh punt removed

Confirmed working on Android (phone + touchpad), iPad 2 (IOS6). Closed #22065 as fixed.

comment:28 DrewAPicture18 months ago

Related-ish: #22064 - Unable to Read Help in Editor on iPad

EDIT: OK, related only in that it's a mobile device and can't do something :)

Last edited 18 months ago by DrewAPicture (previous) (diff)

comment:29 nacin18 months ago

  • Keywords close added

Anything else left here?

comment:30 nacin18 months ago

  • Resolution set to fixed
  • Status changed from new to closed

Good to go. azaozz may open a new ticket for lingering iOS 6 issues.

comment:31 Otto4218 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[22262] breaks the menu flyouts on my laptop.

This is technically a touchscreen laptop, though I leave the touchscreen off most of the time.

Well, in Chrome, it's apparently smart enough to detect this, because
alert('ontouchstart' in window);
returns true.

Thus, line 198 in common.js is turning on the touch screen support incorrectly in my case.

Only happens in Chrome. Firefox isn't that bright.

comment:32 georgestephanis18 months ago

Ideal resolution, imho, would be for it to not swap out this fix instead of the hover support -- but do an enhancement in addition to it.

comment:33 Otto4218 months ago

For reference, this is what the modernizr tells me with regard to touch support:

https://dl.dropbox.com/u/140824/touchsupport.png

http://modernizr.github.com/Modernizr/touch.html

comment:34 Otto4218 months ago

Argh. For reference, chrome is returning true for me for both of these as well:

alert(window.matchMedia("(pointer:coarse)").matches );
alert(window.matchMedia("(hover:0)").matches );

Dangit, chrome.

comment:35 nacin18 months ago

  • Keywords close removed

This resulted in #22382 as well. Seems to me like we are approaching revert status on [22262].

comment:36 georgestephanis18 months ago

Give me a chance to go all 'Fix-It Felix' this evening. If not, you can 'Wreck-It Ralph' tomorrow morning.

georgestephanis18 months ago

comment:37 georgestephanis18 months ago

  • Keywords needs-testing added

Patch attached. Instead of the initial "This Or That" statement, the normal HoverIntent is always run, and it's just a conditional for the touch enhancements.

comment:38 follow-up: georgestephanis18 months ago

Gah! Curse this new text editor I've been testing out. It swapped all the tabs for spaces in the patch!

/me commits ritual seppuku.

DrewAPicture18 months ago

spaces to tabs

comment:39 in reply to: ↑ 38 DrewAPicture18 months ago

Replying to georgestephanis:

Gah! Curse this new text editor I've been testing out. It swapped all the tabs for spaces in the patch!

Spaces to Tabs bundle in TextMate is a beautiful thing.

20614.7-tabs.diff converts your patch to tabs.

SergeyBiryukov18 months ago

Additional tab fixes

comment:40 nacin18 months ago

  • Owner set to azaozz
  • Status changed from reopened to assigned

azaozz18 months ago

comment:41 azaozz18 months ago

20614-8.patch attaches temporarily the 'click' event on touch devices then the toolbar is "touched" and detaches it after the "click" disabling hoverIntent for this click. That way if the device has mouse and the click was with the mouse, that won't be used and instead hoverIntent will handle it as usual.

comment:42 follow-up: Otto4217 months ago

Patch 8 fixes the left hand side menu hovering, but the expanded menus don't work properly.

When hovering over an item in the expanded menu (for example, when on the wp-admin/users.php page, the All Users/Add New/Your Profile links below the Users menu item), a single click does not work to take you to Your Profile, for example.

A second click works, but not if you unhover the link and then come back to it. So if you click, move the mouse, move the mouse back, and click again, then nothing happens. To access any menu item in that expanded menu, you have to click twice without leaving the hovered state on the item.

comment:43 in reply to: ↑ 42 azaozz17 months ago

Replying to Otto42:

Should be fixed in 20614-9.patch, could you re-test it. Also fixed some inconsistencies when the admin menu is folded.

azaozz17 months ago

comment:44 Otto4217 months ago

Ok, that one seems to work. Still need to test admin bar for multisite, but everything else looks okay.

comment:45 azaozz17 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 22636:

Make the admin menu and toolbar work well on mobile devices (take 2), props georgestephanis, fixes #20614, fixes #22382

Note: See TracTickets for help on using tickets.