Image: Reflect media deletion in the editor #35973
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +165 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
ramonjd
commented
Oct 27, 2021
ramonjd
force-pushed
the
try/reset-image-block-after-attachment-deletion
branch
2 times, most recently
from
9151d39
to
884697f
Compare
ramonjd
force-pushed
the
try/reset-image-block-after-attachment-deletion
branch
from
8acf54e
to
bb3b296
Compare
7 tasks
ramonjd
added
[Block] Image
ramonjd
force-pushed
the
try/reset-image-block-after-attachment-deletion
branch
from
bb3b296
to
b41f862
Compare
ramonjd
changed the title
ramonjd
changed the title
…on and passing to onRemove prop.
Now relying on the `onClose` event of the media upload, and the media object to test if the media has been "destroyed."
This removes the `isSelected` check, but also removes a bunch of code as well. We could run the same check on `isSelected`... TBC
… added to the Image props
ramonjd
force-pushed
the
try/reset-image-block-after-attachment-deletion
branch
from
b41f862
to
5410b0b
Compare
Will do! It would be great to get it tested by someone who is not me too. 🍺 |
Thanks for the nudge on this one @glendaviesnz Seems actually to be a nice little upgrade Kapture.2021-12-14.at.13.39.44.mp4 |
|
This tests well for me - this seems like a pretty robust fix for the single image deletion, and a One last question at this point - should we be assuming that on deleting the image the user also expects the |
Good question. I didn't consider that angle 🤔, and assumed that metadata should also be deleted with the image. I was taking the lead from the way we already clear the attributes if we can't find the media properties. I can imagine that one might delete the image and select another one at the same time. Looking at it that way, it could be annoying to have the data wiped. What do you reckon? |
I imagine that 50% of users will want the data to stay and 50% for it to be cleared 😄 Maybe, given that it is easier to highlight and overwrite text you thought should have been wiped than to remember and rewrite the text you had hoped would stay, we start with keeping it? |
… That way the caption, title and alt values persist in case the user deletes, then replaces.
Ha! Too easy. I've also updated the test description. |
glendaviesnz
approved these changes
Dec 14, 2021
paaljoachim
added
the
Backport to WP 6.7 Beta/RC
|
I added the Backport label as I assume this will be backported to WP 5.9. |
|
@paaljoachim could you clarify why this needs to be backported to 5.9? At this stage in the release process, we should only backport regressions and fixes to the new features going into this release e.g. site editor, global styles and navigation block. |
paaljoachim
removed
the
Backport to WP 6.7 Beta/RC
|
Hi @tellthemachines I removed the Backport label. |
7 tasks
33 tasks
ramonjd
added a commit
that referenced
this pull request
May 23, 2022
…eleted from the attachments. If the image block was not replaced with an embed, and it's not an external image, and it's been deleted from the database, clear the attributes and trigger the placeholder.
This was referenced May 23, 2022
ramonjd
added a commit
that referenced
this pull request
Jun 3, 2022
…eleted from the attachments. If the image block was not replaced with an embed, and it's not an external image, and it's been deleted from the database, clear the attributes and trigger the placeholder.
ramonjd
added a commit
that referenced
this pull request
Jun 23, 2022
…eleted from the attachments. If the image block was not replaced with an embed, and it's not an external image, and it's been deleted from the database, clear the attributes and trigger the placeholder.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Looking at fixes for #23262
It should close #28914 as well.
Very rough start.
So far we're:
new Image()How to test
Repeat steps 1-7 but add multiple images, including the new Gallery block (with Image InnerBlocks), ensuring that the same image appears multiple times in your post.
To test that image embed replacement still works, insert a new Image Block and use an url from an image service, e.g., https://www.flickr.com/photos/normanwest4tography/51737773160/in/explore-2021-12-11/
See Embeds for a list of embeddable image sources.
Repeat steps 1-7, but at step 3, after you've deleted the image, select another image to replace the deleted one. If you've added a caption, the caption should persist.
Screenshots
Nov-16-2021.20-37-30.mp4
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).