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

Cover block MediaPlaceholder interferes with block inserter drag and drop #66422

Closed
3 of 6 tasks
talldan opened this issue Oct 24, 2024 · 8 comments · Fixed by #66986
Closed
3 of 6 tasks

Cover block MediaPlaceholder interferes with block inserter drag and drop #66422

talldan opened this issue Oct 24, 2024 · 8 comments · Fixed by #66986
Assignees
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Status] In Progress Tracking issues with work in progress

Comments

@talldan
Copy link
Contributor

talldan commented Oct 24, 2024

Description

The cover block's MediaPlaceholder seems to prevent dragging and dropping blocks or patterns from the block inserter.

Additionally, it seems to interpret these blocks/patterns as files and shows incorrect messaging in the UI.

This issue may also happen for other blocks that use MediaPlaceholder but it's hard to say as I haven't tested widely.

Step-by-step reproduction instructions

  1. Insert a cover block
  2. Open the inserter and try dragging a paragraph block over the placeholder
  3. Observe it shows a message 'Drop files to upload' even though files aren't being dragged
  4. Select a color from the placeholder
  5. Try dragging a block or pattern from the inserter into the cover block (which is now not showing the placeholder and should accept blocks)
  6. Observe the 'Drop files to upload' still shows and prevents inserting any blocks or patterns into the cover block

Screenshots, screen recording, code snippet

Kapture.2024-10-24.at.16.34.54.mp4

Environment info

No response

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

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@talldan talldan added [Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Bug An existing feature does not function as intended labels Oct 24, 2024
@getdave getdave moved this to 🗣️ In Discussion / Needs Decision in WordPress 6.7 Editor Tasks Oct 25, 2024
@getdave
Copy link
Contributor

getdave commented Oct 25, 2024

@ndiego @colorful-tones I believe this affects Zoom Out so we should gauge it's impact there and decide whether we need to try and resolve during this cycle.

@ndiego
Copy link
Member

ndiego commented Oct 25, 2024

If we do include it in 6.7, we should ideally get it into RC2 so there is a bit of time for folks to test the fix. Can a fix be implemented in time? If so, I'm all for it.

@getdave
Copy link
Contributor

getdave commented Oct 25, 2024

Looking into this and documenting findings on this comment as I go...

Suggestion

We could conditionalise the label here based on the type of data being dragged over the drop zone.

However it looks like browser deliberately disable the ability to access the data about the type of "thing" being dragged until it's actually dropped. This is a security feature so I don't think there's a way to tell what got dragged unless we move that to state and track manually 🤔

Moreover, as the Cover block doesn't accept blocks but only images, we should conditionalise the label to be relevant only to images.

However, it looks as though we'd have to do some work to distinguish blocks from just "any old HTML" if we wanted to show a better message (e.g. Drop blocks).

@getdave
Copy link
Contributor

getdave commented Nov 4, 2024

@kevin940726 @ndiego @colorful-tones I can't see this being resolved in time for RC3 tomorrow. I'm minded to punt to 6.8 cycle. We should fix but there are lots of elements to consider here which I wouldn't feel comfortable rushing into 6.7.

@ndiego
Copy link
Member

ndiego commented Nov 4, 2024

@kevin940726 @ndiego @colorful-tones I can't see this being resolved in time for RC3 tomorrow. I'm minded to punt to 6.8 cycle. We should fix but there are lots of elements to consider here which I wouldn't feel comfortable rushing into 6.7.

Agreed.

@ndiego ndiego moved this from 🗣️ In Discussion / Needs Decision to 🏈 Punted to 6.8 in WordPress 6.7 Editor Tasks Nov 4, 2024
@richtabor
Copy link
Member

I'd expect the following:

  • I should only ever see the DropZone when dragging media over a cover block, not any other blocks.
  • When not in placeholder state, I can place blocks precisely within the cover block. Again, the DropZone should not be rendered.

This below should not happen. Instead it should behave like dragging any other blocks into any other groups.

CleanShot.2024-11-05.at.11.44.40.mp4

@getdave
Copy link
Contributor

getdave commented Nov 6, 2024

Agreed. It's something to fix.

@richtabor richtabor changed the title Cover Block: Media Placeholder interferes with block inserter drag and drop Cover bock MediaPlaceholder interferes with block inserter drag and drop Nov 7, 2024
@richtabor richtabor changed the title Cover bock MediaPlaceholder interferes with block inserter drag and drop Cover block MediaPlaceholder interferes with block inserter drag and drop Nov 7, 2024
@richtabor richtabor removed the [Type] Bug An existing feature does not function as intended label Nov 7, 2024
@tellthemachines
Copy link
Contributor

Looks like this was caused by #49673 adding a non-optional onHTMLDrop inside the media placeholder. Because anything coming from the inserter has its type set to HTML, the placeholder always shows.

One way to fix this is by creating an additional dataTransfer type that is exclusive to media, and changing the media placeholder drop handler to handle that one type only. I'll have a play with this and see if it's viable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Status] In Progress Tracking issues with work in progress
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants