Add aspect-ratio to post featured image block #47854
Merged
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
added 6 commits
February 6, 2023 10:49
ajlende
added
[Type] Feature
ajlende
requested review from
draganescu,
scruffian,
getdave,
richtabor and
MaggieCabrera
|
Size Change: +12 kB (+1%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
added 2 commits
February 7, 2023 18:34
ajlende
requested review from
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Flaky tests detected in b797e6b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4140079765
|
This comment was marked as resolved.
This comment was marked as resolved.
scruffian
reviewed
Feb 8, 2023
| @@ -54,6 +54,7 @@ export default function AspectRatioDropdown( { toggleProps } ) { | |||
| } } | |||
| value={ aspect } | |||
| aspectRatios={ [ | |||
| // All ratios should be mirrored in PostFeaturedImage in @wordpress/block-library | |||
There was a problem hiding this comment.
How can we share this in a single file? (can be a followup).
There was a problem hiding this comment.
The best way is probably moving the width/height/aspect ratio controls to components. They're also different values which makes it a little more tricky: the image block one needs numbers and the post featured image block needs a CSS string. Agree it's worth a follow-up.
|
When the width and height are set, the aspect ratio is (rightly) ignored. It would be good to have way to show that this is what's going on (should the control become disabled?). This can be a followup too.
|
scruffian
approved these changes
Feb 8, 2023
MaggieCabrera
approved these changes
Feb 8, 2023
My thought was that when an aspect ratio is selected, the width and height become linked—changing one changes the other. But disabling it might be another good approach. It's also probably worthwile to not output the aspect-ratio CSS when both width and height are set. |
getdave
reviewed
Feb 9, 2023
| label={ __( 'Aspect ratio' ) } | ||
| value={ aspectRatio } | ||
| options={ [ | ||
| // These should use the same values as AspectRatioDropdown in @wordpress/block-editor |
There was a problem hiding this comment.
Maybe in a follow up this become a editor setting? Then you can share them.
There was a problem hiding this comment.
I think @getdave has a good point here - the default aspect rations do sound like an editor setting, one that should be open to be overriden by whomever instances the editor?
There was a problem hiding this comment.
In #47271 it's customizable in theme.json as the main source. We're holding off on that for 6.2, though.
|
I think we should merge this and create followups for these items:
|
draganescu
reviewed
Feb 9, 2023
| $wrapper_attributes = get_block_wrapper_attributes(); | ||
| } else { | ||
| $wrapper_attributes = get_block_wrapper_attributes( array( 'style' => $width . $height ) ); | ||
| $wrapper_attributes = get_block_wrapper_attributes( array( 'style' => $aspect_ratio . $width . $height ) ); | ||
| } | ||
| return "<figure {$wrapper_attributes}>{$featured_image}</figure>"; |
There was a problem hiding this comment.
Is this a good place and time to use the new html parser in this block?
I'm thinking that if an aspect ratio is assigned, then the height/width should adhere to that ratio (regardless of which is changed). For example, if you set the aspect ratio to 16:10, then set a width of 1600px, the height should be auto-set to 1000px. And if you change the height to 800px, then the width would auto-set to 1280 (maintaining the 16:10 aspect ratio). We could also leverage the existing link/unlink control to show that the values are linked together (and provide a way to unlink the height/width). If unlinked, then the aspect ratio value should no longer represent the chosen ratio, but rather "Freeform" (or another term if we prefer). CleanShot.2023-02-09.at.16.49.36.mp4 |
Just a quick note that to support versions of WordPress prior to that core patch landing, we could add the CSS property to the allow list in |
scruffian
added
the
Backport to Gutenberg RC
This was referenced Feb 10, 2023
This was referenced Feb 13, 2023
|
Before: After
I will need to wait inntil Gutenberg plugin 15.2 has been released. |
|
I cannot reproduce the problem @paaljoachim on |
DaisyOlsen
removed
the
Backport to Gutenberg RC
|
Good to hear Andrei! |
This was referenced Apr 6, 2023
27 tasks
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.
This requires a change to kses to allow the aspect-ratio property. WordPress/wordpress-develop#4032.
What?
Part of #47274. Does not include the sectioned select dropdown UI which can come as a follow-up.
Adds CSS aspect-ratio controls to the Post Featured Image block.
Why?
A simpler starting place for aspect ratio is in the context of the Post Featured Image block since it doesn't already have cropping options and it is displaying dynamic content.
It gives more options for controlling the size of the image without needing to explicitly set width or height parameters.
How?
Adds controls for the aspect ratio above width and height that apply the aspect-ratio CSS property to the container of the block.
Testing Instructions
This requires a change to kses to allow the aspect-ratio property. WordPress/wordpress-develop#4032.
Block examples html
Testing Instructions for Keyboard
Screenshots or screencast
UI
Output
I tested several combinations which can be copied under "Testing Instructions". The featured image was 512px square. I tested with a width/height smaller than that (400px) and a width/height larger than that (600px). The max-width for the page was 650px.
Pay special attention to "Horizontal large height". The aspect ratio of the image does not match the settings because the height and max-width properties take precedence over aspect-ratio.