Gallery block: ensure image attributes copy correctly between transforms #42796
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
glendaviesnz
added
[Status] In Progress
|
Size Change: -21.8 kB (-2%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
|
I have made adjustments to ensure that all attributes are correctly carried over in the transformation between the image and the gallery block. |
glendaviesnz
changed the title
glendaviesnz
added
[Type] Bug
|
Your changes tested well for me @t-hamano |
|
I've run through the test steps a few times, and here's what I'm seeing: Insert image<!-- wp:image {"id":42,"width":591,"height":788,"sizeSlug":"large","linkDestination":"media","className":"bridge-kaputt"} -->
<figure class="wp-block-image size-large is-resized bridge-kaputt" id="bridge-kaputt"><a href="http://localhost:8888/wp-content/uploads/2022/07/bridge-kaputt-scaled.jpg" rel="test"><img src="http://localhost:8888/wp-content/uploads/2022/07/bridge-kaputt-768x1024.jpg" alt="bridge-kaputt" class="wp-image-42" width="591" height="788" title="bridge-kaputt"/></a><figcaption class="wp-element-caption">bridge-kaputt</figcaption></figure>
<!-- /wp:image -->Covert image to gallery<!-- wp:gallery {"linkTo":"none"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"><!-- wp:image {"id":42,"sizeSlug":"large","linkDestination":"media","className":"bridge-kaputt"} -->
<figure class="wp-block-image size-large bridge-kaputt" id="bridge-kaputt"><a href="http://localhost:8888/wp-content/uploads/2022/07/bridge-kaputt-scaled.jpg" rel="test"><img src="http://localhost:8888/wp-content/uploads/2022/07/bridge-kaputt-768x1024.jpg" alt="bridge-kaputt" class="wp-image-42" title="bridge-kaputt"/></a><figcaption class="wp-element-caption">bridge-kaputt</figcaption></figure>
<!-- /wp:image --></figure>
<!-- /wp:gallery -->Covert gallery to image<!-- wp:image {"id":42,"sizeSlug":"large","linkDestination":"none","className":"bridge-kaputt"} -->
<figure class="wp-block-image size-large bridge-kaputt" id="bridge-kaputt"><img src="http://localhost:8888/wp-content/uploads/2022/07/bridge-kaputt-768x1024.jpg" alt="bridge-kaputt" class="wp-image-42" title="bridge-kaputt"/><figcaption class="wp-element-caption">bridge-kaputt</figcaption></figure>
<!-- /wp:image -->
|
hmm, weird, I thought the link was kept full circle when I tested, but I get the same result as you now - will take look. |
|
@ramonjd this is a strange one, repeated your issue the first couple of times, and now I can't repeat it to try and debug - will take another look on Monday |
No worries! I think it might have worked for me once. I might have hit I'll take another look next week too! |
|
@ramonjd this is weird - only seems to happen the first time you do a full circle transformation on a freshly added image, eg.
If at step 3 above you add the link to media back again and do the full circle transform again the link stays - in both cases the block markup at the point of going from Gallery back to Image is identical 🤔 |
Following the instructions you gave above, the link tag is also stripped for me at step 2 unless I save the post after step 1. 😕
One difference I did notice, and one I can't explain, is that the
But after linking again it's there:
|
Ah, I may have omitted the fact that I was adding a save - have added to the instructions
Yeh, that is what I am seeing as well - very strange as the block markup and attributes all appear to be identical each time apart from the missing href 🤔 I wonder if it is an edge case with pursuing at this point or if we merge this fix which is an improvement on the current state as it is? |
I'd say it's okay to merge as is, but would definitely consider a follow up. What if there were a gallery, each of whose images had a custom link? Would a transform back to image wipe them? |
Good question, will do some more testing |
|
Gave this a test, and I'm also seeing the link stripped when transforming from image to gallery. It was an external link, and I'd saved after step one. Possibly the issue is |
Thanks @talldan, that did seem to be the issue - have updated this method to fall back to the Image block destination if the gallery block destination is undefined, and wasn't able to replicate the issue after doing that. |
Is it just me? The link is still stripped when converting from Gallery to Image. Saving the post makes a difference, as the <!-- wp:image {"id":77,"width":300,"height":300,"sizeSlug":"large","linkDestination":"media","style":{"border":{"width":"26px"}},"className":"Flowers"} -->
<figure class="wp-block-image size-large is-resized has-custom-border Flowers" id="Flowers"><a href="http://localhost:8888/wp-content/uploads/2022/08/poppy.jpeg"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-684x1024.jpeg" alt="Flowers" class="wp-image-77" style="border-width:26px" width="300" height="300" title="Flowers"/></a></figure>
<!-- /wp:image -->
<!-- Saved post, then coverted to gallery... -->
<!-- wp:gallery {"linkTo":"none"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"><!-- wp:image {"id":77,"sizeSlug":"large","linkDestination":"media","style":{"border":{"width":"26px"}},"className":"Flowers"} -->
<figure class="wp-block-image size-large has-custom-border Flowers" id="Flowers"><a href="http://localhost:8888/wp-content/uploads/2022/08/poppy.jpeg"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-684x1024.jpeg" alt="Flowers" class="wp-image-77" style="border-width:26px" title="Flowers"/></a></figure>
<!-- /wp:image --></figure>
<!-- /wp:gallery -->
<!-- Didn't save post, then coverted to image... -->
<!-- wp:image {"id":77,"sizeSlug":"large","linkDestination":"none","className":"Flowers"} -->
<figure class="wp-block-image size-large Flowers" id="Flowers"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-684x1024.jpeg" alt="Flowers" class="wp-image-77" title="Flowers"/></figure>
<!-- /wp:image -->No saving: <!-- wp:image {"id":79,"sizeSlug":"full","linkDestination":"attachment"} -->
<figure class="wp-block-image size-full"><a href="http://localhost:8888/?attachment_id=79"><img src="http://localhost:8888/wp-content/uploads/2022/08/black-neutral-ancient-greek-columns-wallpaper-mural-Plain-820x532-copy.jpg" alt="" class="wp-image-79"/></a></figure>
<!-- /wp:image -->
<!-- Didn't save post, then coverted to gallery... -->
<!-- wp:gallery {"linkTo":"none","sizeSlug":"full"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"><!-- wp:image {"id":79,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image size-full"><img src="http://localhost:8888/wp-content/uploads/2022/08/black-neutral-ancient-greek-columns-wallpaper-mural-Plain-820x532-copy.jpg" alt="" class="wp-image-79"/></figure>
<!-- /wp:image --></figure>
<!-- /wp:gallery -->
<!-- Didn't save post, then coverted to image... -->
<!-- wp:image {"id":79,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image size-full"><img src="http://localhost:8888/wp-content/uploads/2022/08/black-neutral-ancient-greek-columns-wallpaper-mural-Plain-820x532-copy.jpg" alt="" class="wp-image-79"/></figure>
<!-- /wp:image --> |
|
When I convert back to an Image from a Gallery, |
Weird, I just tried with the markup you proved and the link survived the round trip both saving and not saving. @talldan when you get a minute are you able to give it a try please?
That is correct, this method is only used by the gallery to set the correct href and destination when an image is added to the gallery, not when it is transformed back to an image |
|
I tried again, did a hard reset using the latest commit from this branch, cleaned out cache, deleted npm modules... It's still happening for me. I think I'm special 😄 Let's see if others have better luck. Sorry! |
|
Looks like the link only gets stripped if you toggle between code and editor view to see of the link is present - checking the link via the block toolbar doesn't cause it to be stripped |
…n to prevent previously set image links being wiped when switching between code and editor view
ramonjd
approved these changes
Aug 22, 2022
There was a problem hiding this comment.
Retested this after the latest change. Thanks for tracking this down @glendaviesnz
Now the link and attributes survive transformation when:
- toggling between code view and visual editor
- saving or not saving
Image:
<!-- wp:image {"id":81,"width":303,"height":202,"sizeSlug":"medium","linkDestination":"media","className":"Poppy"} -->
<figure class="wp-block-image size-medium is-resized Poppy" id="Poppy"><a href="http://localhost:8888/wp-content/uploads/2022/08/poppy.jpg"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-300x200.jpg" alt="Poppy" class="wp-image-81" width="303" height="202" title="Poppy"/></a><figcaption class="wp-element-caption">Several red poppy heads against a blue sky. Copy space for text.</figcaption></figure>
<!-- /wp:image -->
Image > Gallery
<!-- wp:gallery {"linkTo":"none","sizeSlug":"medium"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"><!-- wp:image {"id":81,"sizeSlug":"medium","linkDestination":"media","className":"Poppy"} -->
<figure class="wp-block-image size-medium Poppy" id="Poppy"><a href="http://localhost:8888/wp-content/uploads/2022/08/poppy.jpg"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-300x200.jpg" alt="Poppy" class="wp-image-81" title="Poppy"/></a><figcaption class="wp-element-caption">Several red poppy heads against a blue sky. Copy space for text.</figcaption></figure>
<!-- /wp:image --></figure>
<!-- /wp:gallery -->
Gallery > image
<!-- wp:image {"id":81,"sizeSlug":"medium","linkDestination":"media","className":"Poppy"} -->
<figure class="wp-block-image size-medium Poppy" id="Poppy"><a href="http://localhost:8888/wp-content/uploads/2022/08/poppy.jpg"><img src="http://localhost:8888/wp-content/uploads/2022/08/poppy-300x200.jpg" alt="Poppy" class="wp-image-81" title="Poppy"/></a><figcaption class="wp-element-caption">Several red poppy heads against a blue sky. Copy space for text.</figcaption></figure>
<!-- /wp:image -->
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.
What?
Modifies which Image block attributes are copied when transforming between Image blocks and Gallery block
Why?
Currently the size attribute is copied to Gallery images which caused layout issues which can't be fixed as size can't be altered currently on Gallery block images. Also some other attributes like rel and additional classnames are not copied between transforms
Fixes: #42755
How?
Updates which attribs are copied between transforms
Testing Instructions
Screenshots
Before:
ia-before.mp4
After:
ia-after.mp4