[Post Featured Image]: Try fix max-width sizing issues #50370
Closed
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
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
Mamaduka,
jasmussen,
tellthemachines and
andrewserong
|
Thanks for the followup, seems like this just need a quick code check! |
|
Size Change: +79 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Mamaduka
reviewed
May 5, 2023
There was a problem hiding this comment.
Thanks for looking into this! As per my comments below, this approach has the limitation of not being aware of the parent layout, which should determine if max content width is applied or not. Unless the parent's layout type is "constrained", we shouldn't limit the content width at all.
The problem is that for the front end we don't have access to the parent layout settings.
I wonder if we couldn't instead use the same approach as in the Image block, where any fixed width value in the front end gets applied to the img element, so it can't override any max-width set by layout on the block outer wrapper.
| // Depending on the `align` value we need to set a different max-width | ||
| // to restrict properly the image size. | ||
| if ( ! align ) { | ||
| style.maxWidth = 'min(100%, var(--wp--style--global--content-size))'; |
There was a problem hiding this comment.
We should only output this style is there is a width set, otherwise if there's no align and no width the block works fine as it is.
The other thing we should be checking here is parent layout, because this rule only makes sense for a child of a constrained type layout. There's an __unstableParentLayout prop that gets passed into the block edit function that can help with that 🙂
| @@ -92,7 +92,13 @@ function render_block_core_post_featured_image( $attributes, $content, $block ) | |||
| if ( ! $height && ! $width && ! $aspect_ratio ) { | |||
| $wrapper_attributes = get_block_wrapper_attributes(); | |||
| } else { | |||
| $wrapper_attributes = get_block_wrapper_attributes( array( 'style' => $aspect_ratio . $width . $height . 'max-width:100%;' ) ); | |||
| $max_width = ''; | |||
| if ( empty( $attributes['align'] ) ) { | |||
There was a problem hiding this comment.
It would be good to check the parent layout before outputting these rules here too, but there's no way to access it currently from the block's render callback function. Maybe we need a slightly different approach here 🤔
|
Thank you for the feedback! I've opened a PR to revert this change and give us time to think this through better. |
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?
Follow up of: #49641 (comment)
The above PR tried to fix some sizing issues with Post Featured Image, but introduced a regression.
This PR tries to constraint the max-width based on the alignment, but I'm not sure of all the nuances here and we might end up reverting the original PR.
This PR while it seems to handle properly the
wide, nonealignments, doesn't prevent the overflowing of images when we have set afull alignand a bigwidthin the block settings. Noting though that this will still be the case by reverting the original PR. Any ideas or insights maybe from @tellthemachines would be much appreciated!Testing Instructions
fullalign.