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

Add ability to use drag & drop to select and replace an image in the Image component #218

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented Apr 16, 2023

Description of the Change

Adds ability to drag and replace the image in the Image Component

Closes #165

Screencast

Screen.Recording.2023-05-29.at.1.24.42.PM.mov

How to test the Change

  1. Add an image using the Image Example block.
  2. Drag a new image over the block to replace it.

Changelog Entry

Added - Drag to replace image in the Image component.

Credits

Props @fabiankaegy @Sidsector9

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@Sidsector9 Sidsector9 self-assigned this Apr 16, 2023
@github-actions
Copy link

github-actions bot commented Apr 16, 2023

Size Change: +72 B (0%)

Total Size: 52.3 kB

Filename Size Change
dist/index.js 52.3 kB +72 B (0%)

compressed-size-action

@Sidsector9 Sidsector9 changed the title enhancement/165: allow to replace image using drag/drop [WIP] enhancement/165: allow to replace image using drag/drop Apr 16, 2023
@fabiankaegy fabiankaegy marked this pull request as draft April 16, 2023 20:58
@Sidsector9 Sidsector9 changed the title [WIP] enhancement/165: allow to replace image using drag/drop enhancement/165: allow to replace image using drag/drop May 29, 2023
@Sidsector9
Copy link
Member Author

@fabiankaegy the functionality works without error but fails within the E2E context. I've tried debugging and got this error:

Screenshot 2023-05-29 at 2 35 58 PM

Would you have any clue about this?

@fabiankaegy
Copy link
Member

@Sidsector9 Yeah, I've also seen that issue unrelated to this PR. But I've not yet been able to figure out what the cause for that actually is :(

@Sidsector9
Copy link
Member Author

:( I'll spend some time this week to investigate further. I'll open this PR for review.

@Sidsector9 Sidsector9 marked this pull request as ready for review May 30, 2023 06:05
@Sidsector9 Sidsector9 requested a review from fabiankaegy May 30, 2023 06:05
@fabiankaegy
Copy link
Member

@Sidsector9 Thank you for working on this :) I noticed one small regression:

The Focal point picker now renders when no image is present, which should not be the case.

CleanShot 2023-05-31 at 09 22 46@2x

I'm also still a little unsure how to best go about the image markup. It causes some issues that we have to introduce the additional DIV in the markup since many usages of this component rely on the image being a direct descendant of a figure element for example... We are running into the same issue here: #172

I'm not sure we have a better solution though. Just throwing it out there.

@Sidsector9
Copy link
Member Author

@fabiankaegy The console error originates from WP Core only when the Block Inserter (the one on the top left of the editor) is used. This happens as the patterns start to load. I did not dig into Core to investigate further, I instead used an alternative way to resolve it. The solution is related to 10up/cypress-wp-utils#96

See the video below:

Screen-2023-06-11-211409.mp4

--

As for the regression with the focal point picker, that is resolved.

I'm also still a little unsure how to best go about the image markup. It causes some issues that we have to introduce the additional DIV in the markup since many usages of this component rely on the image being a direct descendant of a figure element

That makes sense. On a high level #222 describes good solutions, I'll dig further and respond if I can think of any more solutions.

@fabiankaegy fabiankaegy changed the title enhancement/165: allow to replace image using drag/drop Add ability to use drag & drop to select and replace an image in the Image component Aug 23, 2023
@fabiankaegy fabiankaegy marked this pull request as draft January 29, 2024 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to drag an image onto the Image component to directly upload and replace the selected image
2 participants