WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 5 years ago

Last modified 5 years ago

#13524 closed task (blessed) (fixed)

Open closed widget areas when dragging onto them

Reported by: mitchoyoshitaka Owned by: obenland
Milestone: 4.3 Priority: normal
Severity: normal Version: 3.0
Component: Widgets Keywords: has-patch
Focuses: administration Cc:

Description

Right now if you start dragging a widget in the widget admin and drag it over a closed widget area, you have to let go once, open up the widget area, then drag again. Make it so closed widget areas open up when you drag things onto them, perhaps after a slight (500ms?) delay.

Attachments (12)

widgets.dev.js-17732.diff (2.3 KB) - added by edward mindreantre 9 years ago.
13524.patch (513 bytes) - added by polevaultweb 5 years ago.
Added patch to open a closed widget on hover
13524.2.patch (730 bytes) - added by chriscct7 5 years ago.
Adds hover delay
13524.3.patch (1000 bytes) - added by polevaultweb 5 years ago.
Makes the hover delay related to the sortable events, not just mousehover
13524.4.patch (2.2 KB) - added by polevaultweb 5 years ago.
Clean path
13524.5.patch (1.2 KB) - added by polevaultweb 5 years ago.
Close previously closed sidebar when widget dragged out of sidebar
13524.6.patch (1.6 KB) - added by polevaultweb 5 years ago.
Ensure a previously closed sidebar stays open after a dropped widget is subsequently removed
13524.7.patch (1.5 KB) - added by polevaultweb 5 years ago.
Use droppable tolerance of intersect to fix blank sidebar jumpiness
13524.diff (2.3 KB) - added by obenland 5 years ago.
13524.2.diff (2.5 KB) - added by polevaultweb 5 years ago.
Code comment tweaks
13524.3.diff (2.5 KB) - added by obenland 5 years ago.
Changed tolerance to pointer to fix placeholder flickering
13524.4.diff (869 bytes) - added by polevaultweb 5 years ago.

Download all attachments as: .zip

Change History (58)

#1 @nacin
9 years ago

  • Keywords needs-patch added; widget-admin js removed
  • Milestone changed from Awaiting Review to Future Release

#2 @edward mindreantre
9 years ago

I've had a look at the code and sorta got it working. There are two problems:

  1. When dragging the closed widget holders don't open reliably. You have to drag the draggable item over the widget window title a whole bunch of times before it actually opens.
  1. When hover-opened the widget holder won't accept draggables. I'm guessing that the closed widget holder isn't available when the sorting process begins and therefore isn't included in the list of places where stuff can be dropped.

If those two problems can be solved then this patch should work. Haven't built in any delay, though.

#3 @scribu
9 years ago

  • Keywords has-patch added; needs-patch removed

#4 @jane
8 years ago

  • Component changed from Administration to Widgets

#5 @chriscct7
5 years ago

  • Focuses administration added
  • Keywords needs-patch added; has-patch removed

#6 @helen
5 years ago

  • Keywords good-first-bug added

@polevaultweb
5 years ago

Added patch to open a closed widget on hover

#7 @polevaultweb
5 years ago

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

#8 @DrewAPicture
5 years ago

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

#9 @polevaultweb
5 years ago

This issue is still present and the patch still works. Could I get someones eyes on this for a test/feedback please?

Last edited 5 years ago by polevaultweb (previous) (diff)

#10 @chriscct7
5 years ago

  • Keywords good-first-bug removed
  • Owner changed from polevaultweb to chriscct7
  • Priority changed from low to normal
  • Severity changed from minor to normal
  • Status changed from assigned to reviewing

Sure, give me a few minutes :)

Last edited 5 years ago by chriscct7 (previous) (diff)

#11 @chriscct7
5 years ago

  • Keywords commit added; needs-testing removed
  • Milestone changed from Future Release to 4.3
  • Status changed from reviewing to accepted

Works like a charm. Nice work @polevaultweb

#12 @johnbillion
5 years ago

  • Keywords needs-patch added; has-patch commit removed

I think this needs a short timeout. If you've got a bunch of closed widgets areas, they all open as you drag a widget over them.

#13 @chriscct7
5 years ago

What we need to do here is use on hover with the widget for at least 100 milliseconds open, and then if they hover over and then leave the area of the now open widget holder then close it again.

@chriscct7
5 years ago

Adds hover delay

#14 @chriscct7
5 years ago

  • Keywords commit added

Added delay. 100 ms worked the best in terms of experience.

@polevaultweb
5 years ago

Makes the hover delay related to the sortable events, not just mousehover

#15 @polevaultweb
5 years ago

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

@chriscct7 your previous patch meant that if you just hovered the mouse over a closed widget it would open. This patch now makes a closed widget area open if you drag a widget on to it after a 300ms delay (bit nicer at that length), the timeout gets cleared on the sortable.out event, so the area will not be opened if the widget is dragged away from the area within the timeout period.

Thanks for the feedback @johnbillion and help @chriscct7.

#16 @chriscct7
5 years ago

Ah indeed that is nicer

#17 follow-up: @polevaultweb
5 years ago

@johnbillion I know you are super busy, but if you have chance to check out the latest patch for this with the timeout that would be great!

#18 in reply to: ↑ 17 @chriscct7
5 years ago

Replying to polevaultweb:

@johnbillion I know you are super busy, but if you have chance to check out the latest patch for this with the timeout that would be great!

I'll go see if I can find a committer so we can get this in. Its a really nice enhancement.

#19 @chriscct7
5 years ago

  • Keywords needs-testing removed

#20 @obenland
5 years ago

  • Keywords needs-refresh added; commit removed

Patch doesn't apply cleanly anymore.

In testing I noticed that the sidebars that are being hovered over don't close themselves when I leave them (without dropping the widget), but rather stay open. Is that a Sortable thing or is that something we could change?

@polevaultweb
5 years ago

Clean path

#21 follow-up: @polevaultweb
5 years ago

  • Keywords needs-refresh removed

@obenland Thanks for testing. I have just applied a clean patch.

The functionality you describe is a little tricky and currently WIP on an improved patch.

#22 in reply to: ↑ 21 @obenland
5 years ago

  • Milestone changed from 4.3 to Future Release

Replying to polevaultweb:

The functionality you describe is a little tricky and currently WIP on an improved patch.

Let's improve that then. It is currently possible to drag widgets onto closed sidebars, so I'd rather do it right than swap one mediocre ux for another.

#23 @chriscct7
5 years ago

@polevaultweb any update on that patch?

@polevaultweb
5 years ago

Close previously closed sidebar when widget dragged out of sidebar

#24 @polevaultweb
5 years ago

  • Keywords needs-testing added

@obenland new patch submitted, with the UX you requested

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


5 years ago

#26 @obenland
5 years ago

Nice work @polevaultweb!

Can we add something that keeps the sidebar open when a widget gets removed again, since a sidebar has to be open to begin with for that. Having it close is somewhat jarring.

The patch could probably benefit from a bit more testing too.

@polevaultweb
5 years ago

Ensure a previously closed sidebar stays open after a dropped widget is subsequently removed

#27 @polevaultweb
5 years ago

@obenland thanks for the push here - patch added

#28 @chriscct7
5 years ago

  • Milestone changed from Future Release to 4.3

@polevaultweb
5 years ago

Use droppable tolerance of intersect to fix blank sidebar jumpiness

@obenland
5 years ago

@polevaultweb
5 years ago

Code comment tweaks

#29 @ocean90
5 years ago

I made too screencast, one without and one with the patch applied: https://cloudup.com/c2hTTi2vig3
There is some odd flickering when the widget is dragging for the first time over the opened sidebar.

@obenland
5 years ago

Changed tolerance to pointer to fix placeholder flickering

#30 @obenland
5 years ago

@ocean90: I updated the patch to fix the flicker, good eye!
How does the code look to you?

#31 follow-up: @ocean90
5 years ago

13524.3.diff works much smoother.
Sometimes the min-height doesn't get removed, but it's not quite easy to trigger this. It happens mostly when all sidebars are collapsed and you drag a widget multiple times over the sidebars. Otherwise it looks good.

#32 @obenland
5 years ago

  • Owner changed from chriscct7 to obenland

#33 in reply to: ↑ 31 @obenland
5 years ago

  • Keywords needs-testing removed

Replying to ocean90:

Sometimes the min-height doesn't get removed, but it's not quite easy to trigger this. It happens mostly when all sidebars are collapsed and you drag a widget multiple times over the sidebars.

This could be intentional to avoid jumping when dragging a widget from an open sidebar to a closed sidebar below.

#34 @obenland
5 years ago

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

In 33014:

Open closed widget areas when dragging onto them.

Allows dropping Widgets into the correct spot of a given sidebar, no matter
whether that sidebar is open or closed. Removes a step from the setup process
for closed sidebars.

Props polevaultweb.
Fixes #13524.

#35 @azaozz
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This fails for themes with several sidebars when you try to drag a widget from one (open) sidebar or from "Inactive widgets" to a closed sidebar.

Also, if I remember right, couple of years ago we specifically avoided adding "spring-jump" opening of the sidebars while the user is dragging. The argument against was that it makes dragging harder.

I'm 50/50 on whether it is a good thing or not, but if we want it, it will have to be fixed to work when dragging between sidebars.

#36 follow-up: @obenland
5 years ago

Can you be more specific on what fails and what is not working?

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


5 years ago

#38 @ocean90
5 years ago

#20988 was marked as a duplicate.

#39 @obenland
5 years ago

  • Keywords has-patch removed
  • Status changed from reopened to accepted
  • Type changed from enhancement to task (blessed)

Let's explore if hoverintent can improve the experience further.
@polevaultweb, would you be interested at taking a stab at this?

#40 in reply to: ↑ 36 @azaozz
5 years ago

Replying to obenland:

Can you be more specific on what fails and what is not working?

It doesn't keep the target sidebar open when dragging a widget from another sidebar or from Inactive Widgets. Steps to reproduce:

  • Activate Twenty Fourteen.
  • Go to the Widgets screen (it loads with the first sidebar open, the other two -- closed).
  • Drag a widget from the first to the second sidebar. The second sidebar opens, but when you attempt to position the widget, it closes again, so it's not possible to place it in there.

@polevaultweb
5 years ago

#41 @polevaultweb
5 years ago

  • Keywords has-patch needs-testing added

Ok so a new patch added -

The change of the tolerance to 'pointer' here - https://core.trac.wordpress.org/attachment/ticket/13524/13524.3.diff meant that the close of the sidebar after a widget was dragged out didn't happen. The tolerance has been reverted, and a true fix for the placeholder flicker reported here https://core.trac.wordpress.org/ticket/13524#comment:29

The patch also fixes the issue raised here https://core.trac.wordpress.org/ticket/13524#comment:36

#42 @polevaultweb
5 years ago

@ocean90 does the latest patch fix the flicker for you?

@azaozz does the latest patch fix your scenario?

Last edited 5 years ago by polevaultweb (previous) (diff)

#43 @obenland
5 years ago

  • Keywords needs-testing removed

Patch fixes the scenario azaozz reported. I could also not reproduce the flicker, although I'm not sure how to make it happen in the first place.

#44 @azaozz
5 years ago

Yep, 13524.4.diff fixes dragging from open to closed sidebar. The only small change there is to do sidebars.sortable( 'refresh' ); instead of $( this ).sortable( 'refresh' ); so when dragging over a closed sidebar to another closed sidebar, the second one is always refreshed too.

#45 @azaozz
5 years ago

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

In 33088:

Fix opening closed widget areas when dragging a widget over them.
Props polevaultweb. Fixes #13524.

#46 @ocean90
5 years ago

Follow-up: #33015

Note: See TracTickets for help on using tickets.