Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#32821 closed defect (bug) (fixed)

Customizer Menus: Difficulties when reordering menu items by Dragging and Dropping

Reported by: ahortin Owned by: adamsilverstein
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-patch dev-feedback
Focuses: ui, accessibility Cc:

Description (last modified by netweb)

Changing a menu item's depth in the new menu customizer is difficult. While moving horizontally, if you also happen to move the item up one or more pixels, the menu item snaps back to its original depth when dropped.

The drop zone when changing depth needs to be increased, as per the existing Appearance > Menus interface. Here's an animated gif showing what happens.

https://cldup.com/soDXFdjN8F.gif

With such a precise dropzone, this makes dragging menu items around in the menu customizer a slow process as you have to be more precise about where the item is dropped.

It will affect people who aren't that steady with their hands (eg. elderly, people with disabilities). It will also affect people who's eyesight isn't 100% as they may not notice if they've moved the menu item vertically whilst dragging.

As a comparison, here's a gif showing the existing Appearance > Menus interface. You can see the dropzone is much more forgiving.

https://cldup.com/5hAXy5gMAQ.gif

On a side note, if you happen to drag the menu item down whilst dragging, there seems to be a greater range of movement allowed.

Tested on 4.3-alpha-32991

Attachments (2)

32821.diff (698 bytes) - added by adamsilverstein 7 years ago.
Add some margins to sortable menu items
32821.2.diff (1.5 KB) - added by adamsilverstein 7 years ago.
add a margin of error when dragging

Download all attachments as: .zip

Change History (15)

#1 @netweb
7 years ago

  • Description modified (diff)
  • Keywords needs-patch added

#2 @celloexpressions
7 years ago

  • Milestone changed from Awaiting Review to 4.3
  • Type changed from enhancement to defect (bug)

Likely a bug in the admin's menus dragging JS that was masked by the wide margins between items. @adamsilverstein do you think you can track this down?

#3 @adamsilverstein
7 years ago

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

I will look into it!

#4 follow-up: @adamsilverstein
7 years ago

@netweb - thanks for the bug report!

This is almost certainly caused by the lack of margins between the sortable menu items.

As you start to drag up jQuery UI sortable interprets this as hovering over the previous item rather than the opened slot. I tried adjust the 'tolerance' value to 'pointer', but this made little or no difference - the previous item is just to close.

What does make a difference is adding margins similar to what we have on the existing Appearance->menu page. Patch incoming...

@adamsilverstein
7 years ago

Add some margins to sortable menu items

#5 @adamsilverstein
7 years ago

  • Keywords has-patch added; needs-patch removed

@netweb can you please test with 32821.diff - I think it fixes the issue raised here although I admit I liked the no-margin view better. I'm going to keep playing with this to see if there an underlying issue that can be addressed in JS instead of CSS.

#6 in reply to: ↑ 4 @netweb
7 years ago

Replying to adamsilverstein:

@netweb - thanks for the bug report!

Not mine, try @ahortin ;)

#7 @ahortin
7 years ago

@adamsilverstein That's working much better. Thanks.

I agree the no-margin view does look better, especially given the restrictive space of the Customizer, but this is considerably more usable, which is more important.

https://cldup.com/39m6TkUhmu.gif

Last edited 7 years ago by netweb (previous) (diff)

#8 follow-up: @celloexpressions
7 years ago

Unfortunately adding margins back is a no-go for a few reasons.

Most relevantly, the margins were identified as confusing the visual representation of hierarchy(which was previously addressed by adding the "sub item" text to all child menu items). Simply put, with margins, it's much more difficult to understand and even see where submenus are. Then there are also the more cosmetic issues of it looking better visually and allowing many more items to be visible on the screen at a time. The separation between the boxes doesn't serve any useful purpose here except potentially for this bug. We've designed the whole interface and done all of our recent testing with the no-margins view; reverting that would be a significant design change that is very likely to open a can of worms and leave us in a worse place overall.

So instead, we need to track down this bug. I haven't dug into this code, but it seems like it should definitely be fixable. Currently based on the gif, it looks like vertical and horizontal positioning are not being properly distinguished when determining the drop position. I have a hunch that the issue may be related to the drop position being based on the top of the menu item handle rather than basing it on the center of that element. @adamsilverstein can you keep digging into this?

If possible horizontal dragging within a vertical tolerance of +/- 50% of the menu item handle height should trigger submenu behaviors and vertical dragging with the vertical center of one menu item going higher/lower than the center of the adjacent item should trigger vertical dropping, with the two working together at the same time.

#9 in reply to: ↑ 8 @adamsilverstein
7 years ago

I will keep digging.

Replying to celloexpressions:

Unfortunately adding margins back is a no-go for a few reasons.

Most relevantly, the margins were identified as confusing the visual representation of hierarchy(which was previously addressed by adding the "sub item" text to all child menu items). Simply put, with margins, it's much more difficult to understand and even see where submenus are. Then there are also the more cosmetic issues of it looking better visually and allowing many more items to be visible on the screen at a time. The separation between the boxes doesn't serve any useful purpose here except potentially for this bug. We've designed the whole interface and done all of our recent testing with the no-margins view; reverting that would be a significant design change that is very likely to open a can of worms and leave us in a worse place overall.

So instead, we need to track down this bug. I haven't dug into this code, but it seems like it should definitely be fixable. Currently based on the gif, it looks like vertical and horizontal positioning are not being properly distinguished when determining the drop position. I have a hunch that the issue may be related to the drop position being based on the top of the menu item handle rather than basing it on the center of that element. @adamsilverstein can you keep digging into this?

If possible horizontal dragging within a vertical tolerance of +/- 50% of the menu item handle height should trigger submenu behaviors and vertical dragging with the vertical center of one menu item going higher/lower than the center of the adjacent item should trigger vertical dropping, with the two working together at the same time.

@adamsilverstein
7 years ago

add a margin of error when dragging

#10 follow-up: @adamsilverstein
7 years ago

  • Focuses accessibility added
  • Keywords dev-feedback added

Found a better solution for this in 32821.2.diff:

  • Added api.targetTolerance so we can add some margin of error when dragging items in the customizer where we have no margins between the sortable menu items
  • adjusts the calculation of the previous item bottom by 10 pixels, making dragging much easier (previously getting off by 1 px. would pop the menu back to the current position).

Before: http://cl.ly/0s163F073t0F

After: http://cl.ly/0r3w421O2Q0w

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


7 years ago

#12 @ocean90
7 years ago

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

In 33030:

Customizer: Make reordering menu items via drag and drop easier.

Introduce wpNavMenu.options.targetTolerance to define a tolerance when dragging items where no margins between the sortable items exists.

props adamsilverstein.
fixes #32821.

#13 in reply to: ↑ 10 @ahortin
7 years ago

Replying to adamsilverstein:

Found a better solution for this in 32821.2.diff:

Your new solution works well. Thanks.

Note: See TracTickets for help on using tickets.