WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 7 months ago

#31652 closed defect (bug) (fixed)

Drag/Drop Ordering of Media Does Not Work in Chrome on touch enabled devices

Reported by: dan.rossiter Owned by: adamsilverstein
Milestone: 4.7 Priority: normal
Severity: critical Version: 4.1.1
Component: Media Keywords: has-patch needs-testing
Focuses: ui, javascript, administration Cc:

Description

This appears to be a regression of issue #22607. I have noticed it for quite some time (at least a few minor releases ago if I were to guess), but only now am getting around to reporting.

I've observed the correct behavior in IE, but in Chrome (most recent dev and mainstream versions), dragging does nothing. All I get is the blue highlighting around the items on click.

In case it's relevant, the OS I'm testing on is Windows 8.1.

Attachments (2)

31652.diff (1.4 KB) - added by adamsilverstein 7 months ago.
31652.2.diff (3.9 KB) - added by adamsilverstein 7 months ago.

Download all attachments as: .zip

Change History (26)

#1 @HristoK
2 years ago

Can you post steps to reproduce?

#2 follow-up: @dan.rossiter
2 years ago

The steps to reproduce are basically in the title, but sure.

  1. Open a post/page.
  2. Click "Add Media"
  3. Click "Create Gallery"
  4. Select multiple attachments and click "Create a new gallery"
  5. Note that drag-drop ordering of included attachments does not work

#3 @dan.rossiter
17 months ago

  • Severity changed from normal to major

Still completely broken. Now in Windows 10 and still current version of Chrome.

#4 in reply to: ↑ 2 ; follow-up: @SergeyBiryukov
17 months ago

Replying to dan.rossiter:

  1. Open a post/page.
  2. Click "Add Media"
  3. Click "Create Gallery"
  4. Select multiple attachments and click "Create a new gallery"
  5. Note that drag-drop ordering of included attachments does not work

Can't reproduce with WordPress 4.4 and the latest Chrome on Windows (47.0.2526.80).

Does it still happen with all plugins disabled and a default theme (Twenty Sixteen or Twenty Fifteen) activated?

#5 in reply to: ↑ 4 @dan.rossiter
17 months ago

Replying to SergeyBiryukov:

Replying to dan.rossiter:

  1. Open a post/page.
  2. Click "Add Media"
  3. Click "Create Gallery"
  4. Select multiple attachments and click "Create a new gallery"
  5. Note that drag-drop ordering of included attachments does not work

Can't reproduce with WordPress 4.4 and the latest Chrome on Windows (47.0.2526.80).

Does it still happen with all plugins disabled and a default theme (Twenty Sixteen or Twenty Fifteen) activated?

Yes.

#6 @webbistro
14 months ago

Hi all,

I have the same issue in my Chrome (various versions for about 2 years) on Windows 8.1 and could not find a solution and even understand why it happens. Today I passed by this thread https://github.com/dbushell/Nestable/issues/92 and realized that I have a laptop with a touch screen as well, and this is the thing that can make my case different from others who try to reproduce the issue. I set "Enable touch events" to "Disabled" as described and this alone fixed the issue for me! I am not sure if it's possible to fix it in more general manner with JS and without disabling touch events. But I hope it will help in further investigation.

#7 @celloexpressions
13 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

I have also been experiencing this issue for over a year, maybe two. Both on Windows 8.1 and Windows 10, and numerous versions of Chrome. I have a suspicion that it only occurs with devices that support touch. This is a very significant problem, and I always work around it by manually resorting the ids in the text editor, but the average user wouldn't be able to reorder images at all in this scenario.

I have no idea what would be required for core to fix this but would be happy to test any proposed patches. I would be surprised if there isn't something core can do to fix this since drag and drop works elsewhere (like menus).

#8 @cgrymala
11 months ago

I am currently using Chrome 50.0.2661.102 on Windows 10, on a touch-enabled device, and am experiencing the same issue.

I've tested in 4.5.2 on a production install, and I've tested on the build and src copies of WordPress in VVV (currently reporting as 4.6-alpha-20160517.195630 and 4.6-alpha-37438-src as of this morning).

I have tested with my mouse and I've tested with the touch-interface. It is not possible to drag the images, so they cannot be re-ordered in Chrome on my computer.

I also tested in Firefox 45.0.2 on the same computer, and it works just fine there, so it seems to be related specifically to Chrome.

#9 @celloexpressions
7 months ago

  • Severity changed from major to critical

Bumping severity (but not priority) again. This is really bad. The only way to reorder a gallery is by going in to the text editor and reordering the ids in the shortcode. Which is very difficult to do, and not something that most users will know how to do. This has also been broken for 2+ years, perhaps even as long as I've had a touch-enabled device now that I think about it. A majority of Windows devices now support touch.

Or, you can switch to a different browser whenever you need to post a gallery. Also obviously unrealistic, and by the time you get everything uploaded and are ready to adjust the order, you realize that it doesn't work and it's too late.

#10 @adamsilverstein
7 months ago

  • Owner set to adamsilverstein
  • Status changed from new to assigned

#11 @adamsilverstein
7 months ago

  • Summary changed from Drag/Drop Ordering of Media Does Not Work in Chrome to Drag/Drop Ordering of Media Does Not Work in Chrome on touch enabled devices

#12 @adamsilverstein
7 months ago

  • Keywords has-patch needs-testing fixed-major added; needs-patch removed
  • Milestone changed from Future Release to 4.7

31652.diff restores drag and drop functionality to the gallery editor for touch enabled devices (i've patched both built and source files to make testing/committing easier)

I was able to reproduce this bug locally using the chrome debugger device mode to emulate a touch device.

I tracked this bug back to this commit: https://core.trac.wordpress.org/changeset/29446 which was part of an extensive effort to make the media modal more usable on small screen devices in https://core.trac.wordpress.org/ticket/27423. In this commit an explicit check for wp.media.isTouchDevice was added that bypasses sortability for touch devices. I believe this was added because sortable interfered with selection which no longer seems to be the case in my testing.

@celloexpressions @cgrymala @webbistro @dan.rossiter can you please test my patch with your touch enabled devices to see if this resolves the issue you reported and also check to see if you notice any inadvertent side effects from the change.

Last edited 7 months ago by adamsilverstein (previous) (diff)

This ticket was mentioned in Slack in #core-images by adamsilverstein. View the logs.


7 months ago

#14 @ocean90
7 months ago

  • Keywords fixed-major removed

#15 follow-up: @celloexpressions
7 months ago

Thanks @adamsilverstein! The patch fixes it.

I'll note that reordering still doesn't work with touch, but removing the touch check doesn't impact the ability to select or scroll with touch.

#16 in reply to: ↑ 15 @adamsilverstein
7 months ago

Replying to celloexpressions:

I'll note that reordering still doesn't work with touch, but removing the touch check doesn't impact the ability to select or scroll with touch.

@celloexpressions what do you mean by "reordering still doesn't work with touch"? sortable is in place, but you can't reorder the items or it doesn't stick?

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 months ago

#18 @adamsilverstein
7 months ago

In 31652.2.diff

  • Always enqueue jquery-touch-punch in the admin - let its feature detection determine if it should load, not wp_is_mobile which poorly represents touch devices

#19 follow-up: @adamsilverstein
7 months ago

@celloexpressions I have a feeling we were failing to enqueue jquery-touch-punch on your device, we were using wp_is_mobile which relies on user agent sniffing and doesn't really tell us if a device is touch enabled. Touch punch uses feature detection anyway so we can safely enqueue it everywhere.

Can you please retest with the latest patch. Also, can you please test widget and menu drag and drop before/after the patch on your device? Curious if we fixed things there.

Thanks!

#20 in reply to: ↑ 19 ; follow-up: @azaozz
7 months ago

Replying to adamsilverstein:

Always enqueue jquery-touch-punch in the admin

I was hoping we can stop using jquery-touch-punch as it is a 5 years old quick hack for jQuery UI Mouse to somewhat make it work on touch devices. Re-testing it again and thinking I'd rather not have these thumbnails draggable/sorable at all on a phone. Makes "normal" scrolling much harder. A proper fix would probably be to show arrow buttons in the middle of the thumbnails.

Also suspect it is failing in some cases which is probably what @celloexpressions is seeing.

Last edited 7 months ago by azaozz (previous) (diff)

#21 in reply to: ↑ 20 @adamsilverstein
7 months ago

Replying to azaozz:

Re-testing it again and thinking I'd rather not have these thumbnails draggable/sortable at all on a phone.

Thanks for reviewing. Good point, for small screens drag and drop could make using the gallery harder, although its still probably useful on larger screen phones.

The real issue is with touch screen devices with much larger screens that take input by mouse or touch (or stylus), such as many recent windows laptops or the iPad pro (these are becoming more common apparently). Our current code does not enable sortable for gallery items for any touch enabled devices, making the gallery unsortable by mouse or touch on these devices. 31652.diff addresses this issue alone (can you please test that one as well?). Maybe we can also skip sortable here for small viewports using JS to detect the viewport size?

I was hoping we can stop using jquery-touch-punch as it is a 5 years old quick hack for jQuery UI Mouse to somewhat make it work on touch devices.

Isn't Touch Punch required to add touch event support for jQuery UI for now? If anything touch devices of all sizes are becoming way more common.

As for 31652.2.diff and the removal of the wp_is_mobile check: the problem with relying on wp_is_mobile here is that it is USER_AGENT specific and includes ipads and possibly other devices with large screens and alternate input methods where drag and drop does work well. Touch Punch already includes feature detection and will bail early on non-touch devices. This deserves some testing across a variety of devices... I have a hunch this will be helpful for users who currently cannot sort galleries on their devices.

A proper fix would probably be to show arrow buttons in the middle of the thumbnails.

This is a great idea and could work similar to the move links on menus, this would make it much easier to manage galleries for smaller screens.

Last edited 7 months ago by adamsilverstein (previous) (diff)

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-images by mike. View the logs.


7 months ago

#24 @azaozz
7 months ago

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

In 38793:

Media modal: make it possible to reorder images by dragging on devices with both touch screen and mouse support.

Props adamsilverstein.
Fixes #31652.

Note: See TracTickets for help on using tickets.