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

[17.5] Extensibility bug in the Image block due to interactivity changes #57966

Closed
ndiego opened this issue Jan 18, 2024 · 5 comments
Closed
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended

Comments

@ndiego
Copy link
Member

ndiego commented Jan 18, 2024

Description

A user reported that 17.5 breaks the Screen Size control in Block Visibility, so porting this over here.

The Screen Size control adds CSS classes to the block, allowing the user to show or hide the block on the front end. This implementation is a bit unique because classes are not added to the "Additional CSS" field on the block. Instead, these classes are appended on the front end using render_block. You can see the code here.

I am not sure how many other plugins are using a similar method, but it has worked for the last couple of years without issue.

It's likely that the code in Block Visibility needs a rewrite (it was written before the HTML tag processor was available), but guidance on this would be helpful. I would guess that there may be others who are modifying the Image block with render_block, which could be impacted as well.

Step-by-step reproduction instructions

Steps to reproduce

  • Create a new page.
  • Add a Group block.
  • Add an Image block inside the Goup and leave its visibility unchanged.
  • Add an Image block inside the Group and change its visibility to "Hide on desktop" and "Hide on tablet".
image

Expected Results

  • The second image should show on mobile.

Actual Result

  • The second image never shows up.

The markup for the second image on the front end looks like the following. You can see how the added class breaks the wrapper markup.

<!-- wp:interactivity-wrapper -- class="block-visibility-hide-large-screen block-visibility-hide-medium-screen">
<figure class="wp-block-image size-full"><img decoding="async" width="801" height="801" src="http://block-visibility.local/wp-content/uploads/2023/07/beanie-2.jpg" alt="" class="wp-image-134" srcset="http://block-visibility.local/wp-content/uploads/2023/07/beanie-2.jpg 801w, http://block-visibility.local/wp-content/uploads/2023/07/beanie-2-300x300.jpg 300w, http://block-visibility.local/wp-content/uploads/2023/07/beanie-2-100x100.jpg 100w, http://block-visibility.local/wp-content/uploads/2023/07/beanie-2-600x600.jpg 600w, http://block-visibility.local/wp-content/uploads/2023/07/beanie-2-150x150.jpg 150w, http://block-visibility.local/wp-content/uploads/2023/07/beanie-2-768x768.jpg 768w" sizes="(max-width: 801px) 100vw, 801px" /></figure>
<!-- /wp:interactivity-wrapper -->

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 6.4
  • Gutenberg 17.5

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

@ndiego ndiego added [Type] Bug An existing feature does not function as intended Backwards Compatibility Issues or PRs that impact backwards compatability [Block] Image Affects the Image Block labels Jan 18, 2024
@ndiego
Copy link
Member Author

ndiego commented Jan 18, 2024

Perhaps this might be due to #57556, or #56302? cc @DAreRodz @luisherranz

@ndiego
Copy link
Member Author

ndiego commented Jan 18, 2024

After doing some more testing, the issue appears to be here.

The filter has too low of a priority. If you bump to something like 100, the code in Block Visibility works again. This might be fixed in trunk because it looks like there was a recent rewrite of this file in #57729

@t-hamano
Copy link
Contributor

I think this issue is similar to #55407. If the current implementation is impacting a large number of developers, we may need to add this issue to the WP6.5 Editor Tasks board.

@luisherranz
Copy link
Member

We've already reverted the wp:interactivity-wrapper block delimiters of #56302 in #57729. Can you test with trunk to see if the problem is solved?

@ndiego
Copy link
Member Author

ndiego commented Jan 18, 2024

Can you test with trunk to see if the problem is solved?

Yup, I just completed my testing on trunk, and the issue appears resolved. Since #57729 will be included in 17.6, I am going to close this issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants