-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Backport aspect ratio block support #5963
Backport aspect ratio block support #5963
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
c7addf5
to
2b6495e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Couple of minor questions below but overall LGTM!
'value' => 'unset', | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth adding an @since
for this change in compute_style_properties
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! Added in: 03d2d01
public function data_dimensions_block_support() { | ||
return array( | ||
'aspect ratio style is applied, with min-height unset' => array( | ||
'theme_name' => 'block-theme-child-with-fluid-typography', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason for using this particular theme as opposed to, say, block-theme-child
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, block-theme-child-with-fluid-typography
has appearanceTools
set to true
so that the block support will be in use. I think we're doing the same with the background image support, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see! Might be worth consolidating some of those themes at some point but this is fine for now.
03d2d01
to
4dc1c27
Compare
Committed in r57491. |
Wonderful, thank you @tellthemachines! 🙇 |
Backport the aspect ratio block support feature from the Gutenberg plugin, that originally landed in: WordPress/gutenberg#56897 and WordPress/gutenberg#58414
Note this PR backports the PHP parts of the block support. This can be safely done prior to the JS packages landing for core, as the UI for the feature will not be exposed until the JS packages are updated.
min-height:unset
rule:Trac ticket: https://core.trac.wordpress.org/ticket/60365
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.