We already implemented this for contextual links in #1971108: Convert contextual.js to use Backbone (and support dynamic contextual links). We've seen zero complaints about the implementation there so far.

We should also implement it for the Toolbar. It's a trivial patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Status: Active » Needs review
Issue tags: +Usability, +JavaScript, +mobile
FileSize
1.42 KB
Wim Leers’s picture

Now there's also an issue to backport this to the D7 backport of D8's Toolbar: #2175639: Touch support for the navbar: greatly improves the UX of Navbar on mobile devices.

Sam152’s picture

Status: Needs review » Needs work

I have spooled this patch up in Safari on an iPad 4 running iOS6. Works as advertised, however when I hold down the menu items the event is fired twice, opening and then closing the menu in quick succession. I would hazard a guess that the click and touchend events are both being fired.

Can anyone else confirm this?

Going to set this to needs work, however if this issue is not covered by the scope of this patch feel free to set it back to needs review.

Wim Leers’s picture

Status: Needs work » Needs review

#3: can you please check whether you can reproduce that problem with contextual links in general and the pencil icon in the toolbar in specific also? (Because if you experience this problem with the proposed patch, you should also be able to reproduce it there. If not, then something else is at play.)

Sam152’s picture

So I spooled the patch back up and tested with an iPad 4 running iOS 6. I also spooled up a copy of 8.x without the patch.

The problem existed and was replicable on the version with #1 applied. The pencil icon was also not clickable whatsoever. Neither of these issues seemed to happen on the plain 8.x version.

Here is a quick video I took testing the two patches, apologies for the bad quality: https://www.youtube.com/watch?v=GMKzykNzIDg

nod_’s picture

can we haz that for tabledrag as well? it's broken on touch-enabled laptop: #2177293: Drag and drop broken on touch enabled laptops

LewisNyman’s picture

Status: Needs review » Needs work

The last submitted patch, 1: toolbar_touch_support-2175637-1.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
1.48 KB

Straight reroll. Was broken by the massive JS reindentation patch.

hass’s picture

There is an indendation bug...

Wim Leers’s picture

FileSize
1.46 KB

HAH! Stupid! Thanks for catching that :) Pretty awful indentation bug even! This one should be good.

LewisNyman’s picture

Issue tags: +frontend
LewisNyman’s picture

Issue tags: -sprint
Wim Leers’s picture

Wow, blast from the past. What's holding this back?

LoMo’s picture

I've tested this on simplytest.me with Android and the Samsung Galaxy Note 2 (GT-N7105 running Android 4.3) and the toolbar seems pretty sweet. I don't notice any delay.

LoMo’s picture

Status: Needs review » Reviewed & tested by the community

Looking good, setting to RTBC.

LewisNyman’s picture

I just tested it on my iPhone and it's much better :) RTBC++

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c70170c and pushed to 8.0.x. Thanks!

  • alexpott committed c70170c on 8.0.x
    Issue #2175637 by Wim Leers: Touch support for the toolbar: greatly...

Status: Fixed » Closed (fixed)

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