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

Drag and drop performance regression #42363

Closed
msurdi opened this issue Jul 12, 2022 · 6 comments · Fixed by #42806
Closed

Drag and drop performance regression #42363

msurdi opened this issue Jul 12, 2022 · 6 comments · Fixed by #42806
Assignees
Labels
[Feature] Drag and Drop Drag and drop functionality when working with blocks [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@msurdi
Copy link
Contributor

msurdi commented Jul 12, 2022

Description

When dropping an image on the editor, there are certain position intervals where the blue drop position indicator is added and removed from the DOM in an infinite loop. See the attached video.

Step-by-step reproduction instructions

  1. Write three paragraphs
  2. Drag any image and, without actually dropping it, try to position it between two paragraphs as shown in the video
  3. Try moving slowly up or down the drop position until the blinking starts to happen.

Screenshots, screen recording, code snippet

blinking.drop.mp4

Environment info

Gutenberg: v13.6.0
Theme: Twenty Twenty Two
WordPress version: 6.0-RC4-53438

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@kathrynwp kathrynwp added [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block labels Jul 12, 2022
@simison simison added the [Feature] Drag and Drop Drag and drop functionality when working with blocks label Jul 13, 2022
@simison
Copy link
Member

simison commented Jul 13, 2022

@mcsf @ellatrix could this and another similar issue in #42365 popped up with changes in Rich text editing / text selections with more recent Gutenberg releases?

@mcsf
Copy link
Contributor

mcsf commented Jul 14, 2022

Intuition tells me it isn't RichText (but Ella will know better), but Popover would be more likely. Could you regression-test against #40740?

@simison
Copy link
Member

simison commented Jul 26, 2022

@mcsf the bug (both this issue and #42365) happen even with Gutenberg v13.2.0 so the regression must be coming from earlier changes than from #40740 which went to v13.3.0.

@talldan
Copy link
Contributor

talldan commented Jul 29, 2022

I just started reporting what I think is the same bug in a separate issue, then found this. I did some investigation.

In trying to diagnose this, I can spot two bugs. The first is that the dragged block is initially 'greyed out' when starting a drag, but when you drag past its position it suddenly reappears even though the user hasn't stopped dragging. The BlockDraggable is unmounted and stopDraggingBlocks is called. When bisecting I discovered this bug was introduced in #38892.

Kapture.2022-07-29.at.13.23.21.mp4

The second is the big performance regression where, in particular, dragging near the greyed out area of the dragged block causes flickering and lots of lag. This does also happen sometimes when dragging between blocks like the video in the description. On profiling I can see that onDragLeave and onDragOver in useBlockDropZone are being called repeatedly. It looks like this was introduced by #40441.

Kapture.2022-07-29.at.13.40.32.mp4

@talldan talldan changed the title Image drop position marker added and removed in an infinite loop Drag and drop performance regression Jul 29, 2022
@talldan talldan added [Priority] High Used to indicate top priority items that need quick attention and removed [Block] Image Affects the Image Block labels Jul 29, 2022
@talldan
Copy link
Contributor

talldan commented Jul 29, 2022

It looks like the main performance issue is happening because of the removal of pointer-events: none in a couple of places in #40441.

When dragging over the insertion point indicator, the block drop zone is no longer dragged over, so it triggers the onDragLeave event, which hides the insertion point. Once the insertion point is hidden, the drop zone is being dragged over again, so it shows the insertion point. And that repeats ad infinitum.

I should be able to put together a relatively straightforward fix.

edit - fix in #42806.

@talldan
Copy link
Contributor

talldan commented Aug 1, 2022

This should be fixed as of #42806. I reported the other issue in #42840.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 17, 2023
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 [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants