Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zoom out: drag and drop zones behave strangely (disappear, bounce, etc) #65927

Closed
annezazu opened this issue Oct 7, 2024 · 15 comments · Fixed by #66399
Closed

Zoom out: drag and drop zones behave strangely (disappear, bounce, etc) #65927

annezazu opened this issue Oct 7, 2024 · 15 comments · Fixed by #66399
Assignees
Labels
[Feature] Drag and Drop Drag and drop functionality when working with blocks [Feature] Zoom Out [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@annezazu
Copy link
Contributor

annezazu commented Oct 7, 2024

When playing around with drag and drop, I noticed that the zones behave a bit strangely:

  • Appearing then disappearing.
  • "Bouncing" where the drop zone quickly moves up and down trying to expand.
  • Patterns with grid block causes a large overlay to appear with grid lines
drag.and.drop.inserter.zoom.out.mov

This was found using WordPress 6.7 nightly.

@annezazu annezazu added [Feature] Drag and Drop Drag and drop functionality when working with blocks [Feature] Zoom Out [Type] Bug An existing feature does not function as intended labels Oct 7, 2024
@ndiego ndiego moved this to 📥 Todo in WordPress 6.7 Editor Tasks Oct 8, 2024
@richtabor
Copy link
Member

Patterns with grid block causes a large overlay to appear with grid lines

#65974

@richtabor
Copy link
Member

What behavior were you expecting?

@ndiego
Copy link
Member

ndiego commented Oct 14, 2024

Since RC1 is occurring later today and there is no active PR, I think we should punt this to 6.8. Thoughts @colorful-tones @getdave @kevin940726?

@colorful-tones
Copy link
Member

Since RC1 is occurring later today and there is no active PR, I think we should punt this to 6.8.

Agree 👍

@getdave
Copy link
Contributor

getdave commented Oct 14, 2024

This bug was introduced during this cycle. Therefore I see no reason to punt it at this stage. Can we reassess as we move through RC?

@colorful-tones
Copy link
Member

This bug was introduced during this cycle

@getdave Wouldn't this make it a regression if it was introduced into the WordPress 6.7 release cycle and it was not in the WordPress 6.6 release? 🤔

@getdave
Copy link
Contributor

getdave commented Oct 14, 2024

The Zoom Out feature was introduced during this 6.7 cycle. At some point the experience referenced in this Issue changed and provided a suboptimal experience. Happy to label it however you'd prefer, but ultimately it would be better to fix it 👍

@ndiego
Copy link
Member

ndiego commented Oct 14, 2024

The Zoom Out feature was introduced during this 6.7 cycle. At some point the experience referenced in this Issue changed and provided a suboptimal experience. Happy to label it however you'd prefer, but ultimately it would be better to fix it 👍

Sounds good, let's reassess as we move further through the release cycle.

@stokesman
Copy link
Contributor

stokesman commented Oct 15, 2024

I expect #66110 may have slightly alleviated some of this because it makes for less displacement. Still, I’d suggest that the displacement while dragging is maybe more harmful than helpful for two reasons:

  • It lends to the false impression that the drop target is only there, when actually if the separator/zone is visible you’re already over the drop target. This is exacerbated by the prompt text and background color animation when dragged over.
  • Shifting the layout in response to the pointer is what seems to make "bouncing" possible and in the case of the first or last insertion points makes it easier to overshoot the zone you’re trying to drag to.

My feeling is that a drop indicator without displacement while dragging would be better. This version with displacement could be saved for when an inserter button was clicked to basically serve as a help message.

@getdave
Copy link
Contributor

getdave commented Oct 21, 2024

I gave this another test and some issues are still present but they are less pronounced.

The most obvious issue is when dragging near the edge of a drop zone.

Screen.Capture.on.2024-10-21.at.13-44-53.mp4

I will try and see if it's possible to narrow down the causes for this. That said if anyone else would like to jump in I'd be very grateful.

@getdave getdave added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Oct 21, 2024
@talldan
Copy link
Contributor

talldan commented Oct 21, 2024

From debugging (adding logpoints) I could see hideInsertionPoint is being called a lot when that bouncing effect happens:

onDragLeave( event ) {
const { ownerDocument } = event.currentTarget;
// If the drag event is leaving the drop zone and entering an insertion point,
// do not hide the insertion point as it is conceptually within the dropzone.
if (
isInsertionPoint( event.relatedTarget, ownerDocument ) ||
isInsertionPoint( event.target, ownerDocument )
) {
return;
}
throttled.cancel();
hideInsertionPoint();
},

When I logged the elements that were triggering onDragLeave, it was often the <main> element that's actually behind the block and insertion point I'm dragging between. There shouldn't be anyway for it to receive mouse events, as the other elements are on top of it.

My theory is that the animation of the insertion point causes momentary gaps between the insertion point and neighbouring block, and sometimes the mouse is inadvertently over those gaps causing onDragLeave to be triggered.

There might be a few different ways to fix. I have one or two ideas in the works, but don't let that prevent anyone else putting a fix together.

I think there's also a secondary issue. Sometimes when you move you mouse over the insertion point it hides and reappears (only once, not repeatedly). I think the same happens for the regular drop zones, and that's more of a longstanding issue. I seem to recall trying to fix it once, will see if I can dig anything up.

@talldan
Copy link
Contributor

talldan commented Oct 21, 2024

My theory is that the animation of the insertion point causes momentary gaps between the insertion point and neighbouring block, and sometimes the mouse is inadvertently over those gaps causing onDragLeave to be triggered.

I'm revising this. I think it's more related to layout gaps between sections. For example if you add a huge amount of margin above a section, enter zoom out, and then try dragging above that block, whenever the mouse crosses from the block itself into the margin and back again, the drop zone is retriggered (I think because onDragLeave is being triggered calling hideInsertionPoint):

Kapture.2024-10-21.at.18.54.34.mp4

I think the animation can cause this to go into a bit of a loop at times, causing the onDragLeave triggering repeatedly, which also triggers the animation on and off.

@getdave
Copy link
Contributor

getdave commented Oct 22, 2024

That will be it. I wonder how we resolve that? Maybe ignore events that move from a block into the main (underlying element) and back again?

@talldan
Copy link
Contributor

talldan commented Oct 24, 2024

Maybe ignore events that move from a block into the main (underlying element) and back again?

Possibly something like that. I tried one approach, but I think it resulted in some other issues with the insertion points not hiding when they should. I'll continue exploring options.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 24, 2024
@talldan
Copy link
Contributor

talldan commented Oct 24, 2024

Put together a fix here - #66399

It turns out that during Zoom Out, there can be too many drop zones active, and that's what's causing the extra onDragLeave events, so it was a fairly simple fix in the end.

@ndiego ndiego moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.7 Editor Tasks Oct 24, 2024
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.7 Editor Tasks Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Drag and Drop Drag and drop functionality when working with blocks [Feature] Zoom Out [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants