Make WordPress Core

Opened 13 years ago

Last modified 3 years ago

#18556 accepted defect (bug)

Toolbar dropdowns when dragging items

Reported by: chexee's profile chexee Owned by: drecodeam's profile drecodeam
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2.1
Component: Toolbar Keywords: has-patch needs-refresh
Focuses: ui, javascript, css, administration Cc:

Description (last modified by nacin)

When dragging metaboxes, widgets, menu items, etc to the top of the window in order to scroll to drop the item on a portion of the UI that's above the viewing window, the toolbar interferes and produces dropdowns, etc.

If dragging an item, can we disable toolbar actions?

Attachments (8)

menu-adminbar-drop.png (101.6 KB) - added by chexee 13 years ago.
18556.diff (1.7 KB) - added by drecodeam 13 years ago.
resolved issue by binding empty functions on 'start' method of jquery sortable function and then binding original functions on 'stop' method.
18556.2.diff (2.8 KB) - added by jessepollak 11 years ago.
18556.3.diff (5.4 KB) - added by sujin2f 10 years ago.
18556.4.diff (3.4 KB) - added by sujin2f 10 years ago.
18556.5.diff (1.4 KB) - added by akibjorklund 8 years ago.
18556.6.diff (1.3 KB) - added by sabernhardt 3 years ago.
dropdown.png (78.7 KB) - added by Boniu91 3 years ago.

Download all attachments as: .zip

Change History (34)

#1 @ocean90
13 years ago

Screenshot?

#2 @chexee
13 years ago

Attached. Sorry, had to open grab and do a timed screenshot. Kept dropping the menu item when I used shortcuts.

#3 @DrewAPicture
13 years ago

  • Type changed from defect (bug) to enhancement
  • Version set to 3.2.1

#4 @nacin
13 years ago

  • Component changed from General to UI
  • Type changed from enhancement to defect (bug)

Sounds like a bug, but I can reproduce this in 3.2 as well.

I imagine this should be easy to do?

@drecodeam
13 years ago

resolved issue by binding empty functions on 'start' method of jquery sortable function and then binding original functions on 'stop' method.

#5 @drecodeam
13 years ago

  • Cc drecodeam added
  • Keywords has-patch reporter-feedback added

#6 @drecodeam
13 years ago

  • Owner set to drecodeam
  • Status changed from new to accepted

#7 @nacin
13 years ago

  • Description modified (diff)
  • Summary changed from Admin menu dropdowns when dragging items to Toolbar dropdowns when dragging items

Updating "admin menu" to "toolbar" since "admin menu" refers to the left menu.

#8 @DrewAPicture
12 years ago

  • Component changed from UI to Toolbar

#9 @helen
12 years ago

  • Keywords ui-focus added; reporter-feedback removed

#10 @enej
11 years ago

  • Keywords needs-patch added; has-patch removed

This tickets needs a new patch. The current patch only changes the post meta boxes, also the file against witch the patch was applied to doesn't exist any more.

The bug currently effects all the different places we have the drag and drop feature.
(menus, widgets and post-meta boxes but not galleries)

Since its a minor bug I would suggest fixing it once MP6 is released.

@jessepollak
11 years ago

#11 @jessepollak
11 years ago

Definitely not the most elegant solution, but I figured I would take a stab and then work from there with people's feedback. This fixes the problem for widgets, post-meta boxes and menus; I'm pretty sure those are the only places it was an issue.

In a more ideal patch, we'd only add the draggable.start and draggable.stop triggers once, rather than every time we make something draggable. The current solution would leave room to reintroduce this bug the next time we add a draggable element.

@sujin2f
10 years ago

#12 @sujin2f
10 years ago

Oh, I think I misunderstood this system. I did checkout from stable version. Please ignore it. I will patch it later.

@sujin2f
10 years ago

#13 @sujin2f
10 years ago

I changed it by giving the css class 'disable' to dropdown menu while drag&drop.

#14 @morganestes
9 years ago

  • Keywords needs-refresh added; needs-patch removed

@akibjorklund
8 years ago

#15 @akibjorklund
8 years ago

  • Keywords has-patch needs-testing added; needs-refresh removed

I've attached another attempt to fix this. With a single selector I'm targeting all current draggables and sortables, so there is no need to add code to all event handlers. The patch also disables popup menus on the admin menu in addition to the toolbar, which is less of an issue but still.

I'm using toggleClass on dragstart and dragstop instead of explicitly adding a class and removing it. This might cause issues with state on some browsers if the events don't always fire in a predictable order, but it did work flawlessly on the ones I tested it with.

Last edited 8 years ago by akibjorklund (previous) (diff)

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


4 years ago

#17 @sabernhardt
4 years ago

  • Focuses javascript css administration added
  • Keywords needs-refresh added; needs-testing removed
  • Milestone set to Future Release

I see this is still an issue, and the patch would need a refresh (editing the script within \src\js\_enqueues\admin\common.js, and the admin-menu stylesheet doesn't apply anymore).

@sabernhardt
3 years ago

#18 @sabernhardt
3 years ago

  • Keywords needs-testing added; needs-refresh removed
  • Milestone changed from Future Release to 5.9

18556.6.diff helps when dragging Menu items and metaboxes (in the block editor, Classic Editor, Classic Widgets and Dashboard). The block editor (posts or widgets) already prevents opening the menus when dragging blocks.

Changes:

  • The side admin selectors needed to be more specific to avoid hiding the "current" submenus (which show directly under their parent).
  • I used the existing hidden class, available in the admin, instead of adding a new class in the CSS.
  • The toolbar's About dropdown still needed a CSS adjustment to override display:block on the secondary list.

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


3 years ago

#21 @audrasjb
3 years ago

  • Keywords needs-testing removed

Just opened a PR to refresh the patch against trunk and so it could be tested against the test suite

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

@Boniu91
3 years ago

#25 @Boniu91
3 years ago

@audrasjb I'm not able to validate the patch, or I might be missing something. The dropdowns are still being opened, etc. Please find the Test Report below.

Test Report

Env

  • WordPress 5.9-alpha-51272-src
  • Chrome 95.0.4638.69
  • Windows 10
  • Theme: Twenty Twenty One

Steps to test

  1. Add a new menu
  2. Catch one of the items and navigate with it to the top menu
  3. See that dropdown opens
  4. Navigate with it to the sidebar menu and check if other menus open

Screencast:
https://www.loom.com/share/180f28d8ed224b10ae671b825653f8b3

Screenshot:

Last edited 3 years ago by Boniu91 (previous) (diff)

#26 @sabernhardt
3 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.9 to Future Release

@Boniu91 Thanks for testing!

We can revisit this for a future release, then.

Note: See TracTickets for help on using tickets.