Skip to content

Conversation

@mukeshpanchal27
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 commented Dec 16, 2024

Summary

This PR Checks the functionality of the image block in classic themes. After merging #1738, the sizes attribute for the classic theme has been producing incorrect values.

Issue Example:
When selecting the wide alignment for an image block, the sizes attribute is rendered as: sizes="(max-width: ) 100vw, ", This is incorrect. The expected output should be: sizes="(max-width: 1024px) 100vw, 1024px"

This PR aims to fix this issue and ensure the sizes attribute is generated correctly for classic themes.

Steps to reproduce the issue

  • Activate classic theme (TT1)
  • Add image block and select wide alignment
  • Check frontend.

@mukeshpanchal27 mukeshpanchal27 added [Type] Bug An existing feature is broken no milestone PRs that do not have a defined milestone for release [Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) labels Dec 16, 2024
@mukeshpanchal27 mukeshpanchal27 self-assigned this Dec 16, 2024
@mukeshpanchal27 mukeshpanchal27 removed the no milestone PRs that do not have a defined milestone for release label Dec 16, 2024
@mukeshpanchal27 mukeshpanchal27 added this to the auto-sizes 1.4.0 milestone Dec 16, 2024
@mukeshpanchal27 mukeshpanchal27 added no milestone PRs that do not have a defined milestone for release and removed no milestone PRs that do not have a defined milestone for release labels Dec 16, 2024
@mukeshpanchal27 mukeshpanchal27 marked this pull request as ready for review December 16, 2024 07:08
@github-actions
Copy link

github-actions bot commented Dec 16, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: joemcgill <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@joemcgill
Copy link
Member

That makes sense because classic themes won't have a layout setting for this value. We could consider making a default guess, but disabling for now is probably sensable.

@joemcgill joemcgill merged commit 4c97d52 into trunk Dec 16, 2024
14 checks passed
@joemcgill joemcgill deleted the fix/check-classic-theme branch December 16, 2024 14:02
@felixarntz
Copy link
Member

@mukeshpanchal27 @joemcgill This PR looks good to me, though a related question: Do we already have an issue to explore adding a way to define those widths for classic themes?

@joemcgill joemcgill changed the title Accurate sizes: Check sizes issue for classic theme Accurate sizes: Disable layout calculations for classic themes Dec 16, 2024
@joemcgill
Copy link
Member

@felixarntz creating an API for defining layout values in classic themes is listed in the todo list for #760, but we've not yet created an issue for this yet. I wanted us to explore the implementation in the block theme context first before starting to define what that API should look like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Enhanced Responsive Images Issues for the Enhanced Responsive Images plugin (formerly Auto Sizes) [Type] Bug An existing feature is broken

Projects

Status: Done 😃

Development

Successfully merging this pull request may close these issues.

4 participants