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

Gallery randomization does not work when nested within another block #58404

Closed
richtabor opened this issue Jan 29, 2024 · 9 comments · Fixed by #58733
Closed

Gallery randomization does not work when nested within another block #58404

richtabor opened this issue Jan 29, 2024 · 9 comments · Fixed by #58733
Assignees
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@richtabor
Copy link
Member

After testing #57477, I realized that the gallery block's new randomization feature does not function when the gallery block is nested within another block.

Visuals

Here's a video I recorded with the gallery block within a group block (not randomizing):

random-images.mp4

Here's a video of the randomization working, when not nested:

random-topleve.mp4

cc @t-hamano

@richtabor richtabor added [Type] Bug An existing feature does not function as intended [Block] Gallery Affects the Gallery Block - used to display groups of images labels Jan 29, 2024
@t-hamano
Copy link
Contributor

Thanks for the report. I was able to reproduce it too. I'll try to find out the cause.

@t-hamano
Copy link
Contributor

t-hamano commented Feb 6, 2024

While researching the cause of this problem, I noticed something strange. Besides randomizing the images, the Gallery block has another render_block_data hook that adds a data-id to the image block for backwards compatibility.

function block_core_gallery_data_id_backcompatibility( $parsed_block ) {
if ( 'core/gallery' === $parsed_block['blockName'] ) {
foreach ( $parsed_block['innerBlocks'] as $key => $inner_block ) {
if ( 'core/image' === $inner_block['blockName'] ) {
if ( ! isset( $parsed_block['innerBlocks'][ $key ]['attrs']['data-id'] ) && isset( $inner_block['attrs']['id'] ) ) {
$parsed_block['innerBlocks'][ $key ]['attrs']['data-id'] = esc_attr( $inner_block['attrs']['id'] );
}
}
}
}
return $parsed_block;
}

This hook works when the Gallery block is at the root level, but doesn't seem to work when the block is nested.

HTML rendered when gallery block exists at root level:

<figure class="wp-block-gallery has-nested-images columns-3 is-cropped wp-block-gallery-1 is-layout-flex wp-block-gallery-is-layout-flex">
	<figure class="wp-block-image size-large">
		<img fetchpriority="high" decoding="async" width="300" height="300" data-id="18" src="http://localhost:8888/wp-content/uploads/2024/02/one.png" alt="" class="wp-image-18" srcset="http://localhost:8888/wp-content/uploads/2024/02/one.png 300w, http://localhost:8888/wp-content/uploads/2024/02/one-150x150.png 150w" sizes="(max-width: 300px) 100vw, 300px">
	</figure>
</figure>

HTML rendered when gallery block is inside a Group block:

<div class="wp-block-group has-global-padding is-layout-constrained wp-block-group-is-layout-constrained">
	<figure class="wp-block-gallery has-nested-images columns-3 is-cropped wp-block-gallery-1 is-layout-flex wp-block-gallery-is-layout-flex">
		<figure class="wp-block-image size-large">
			<img fetchpriority="high" decoding="async" width="300" height="300" src="http://localhost:8888/wp-content/uploads/2024/02/one.png" alt="" class="wp-image-18" srcset="http://localhost:8888/wp-content/uploads/2024/02/one.png 300w, http://localhost:8888/wp-content/uploads/2024/02/one-150x150.png 150w" sizes="(max-width: 300px) 100vw, 300px">
		</figure>
	</figure>
</div>

As you can see, when the Gallery block is inside the group block, the data-id attribute is not attached to the Image block.

I'm concerned that this hook may not be able to override $parsed_block when blocks are nested.

Similar issues have been reported with Trac: https://core.trac.wordpress.org/ticket/56417

@t-hamano
Copy link
Contributor

t-hamano commented Feb 6, 2024

I'm worried about whether it's okay to ship the gallery randomization feature to WP6.5, so I'm pinging @youknowriad and @getdave.

This feature reorders $parsed_block['innerBlocks'] via the render_block_data hook, but this hook seems to reset the $parsed_block override when blocks are nested.

This issue has also been reported in core: https://core.trac.wordpress.org/ticket/56417

If it makes sense not to ship this feature, I would like to submit a PR to bring this feature back.

@youknowriad
Copy link
Contributor

I think we should seek to fix the bug, if it's not possible on time, it's probably better to disable this feature for the moment.

@t-hamano
Copy link
Contributor

t-hamano commented Feb 6, 2024

I see, I'll try to find a solution by the end of this week, and if I can't find a solution, I'll revert.

@ramonjd
Copy link
Member

ramonjd commented Feb 6, 2024

My instinct is that https://core.trac.wordpress.org/ticket/56417 should be addressed first before adding a work around to the gallery randomizer. What do folks think?

@t-hamano
Copy link
Contributor

t-hamano commented Feb 7, 2024

My instinct is that https://core.trac.wordpress.org/ticket/56417 should be addressed first before adding a work around to the gallery randomizer. What do folks think?

Ideally, yes. However, I am not familiar with the mechanism of block rendering, so if anyone is familiar with this problem, I would appreciate your help 🙇

@t-hamano
Copy link
Contributor

t-hamano commented Feb 7, 2024

As a solution at the moment, I'm working on #58733. This approach also seems to work correctly, so I think we can ship this PR in WP6.5 and refactor this approach when render_block_data hooks are fixed in the future.

@youknowriad
Copy link
Contributor

Thanks for the fix @t-hamano

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants