Add aspect ratio presets support via theme.json #47271
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
jffng
requested review from
spacedmonkey,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Could we just go with |
|
Flaky tests detected in 18c2045. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8923035693
|
|
@mtias I believe We can drop |
Mamaduka
added
[Type] Enhancement
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ajlende
force-pushed
the
add/aspect-ratio-theme-json-spec
branch
from
6938ca4
to
c1f53b4
Compare
ajlende
reviewed
Feb 1, 2023
Comment on lines
+121
to
+138
| array( | ||
| 'path' => array( 'dimensions', 'aspectRatios' ), | ||
| 'prevent_override' => array( 'dimensions', 'defaultAspectRatios' ), | ||
| 'use_default_names' => false, | ||
| 'value_key' => 'ratio', | ||
| 'css_vars' => '--wp--preset--aspect-ratio--$slug', | ||
| 'classes' => array(), | ||
| 'properties' => array( 'aspect-ratio' ), | ||
| ), |
There was a problem hiding this comment.
I wanted to add just the settings for the image block aspect ratio tool in this PR, but it seems that the UI is coupled to the preset generation. So, even though this generated CSS isn't used, it had to be added to get the UI to work.
I don't think it's too big of a deal since we would add those presets for the aspect-ratio property anyway for the featured image in a follow-up, but it certainly surprised me that the code was coupled this way.
|
Size Change: -61 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This was referenced Feb 10, 2023
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
I would love to see this in an upcoming release. Unless I'm missing it, we cannot add a custom aspect ratio via theme.json. |
Mamaduka
reviewed
May 2, 2024
packages/block-editor/src/components/dimensions-tool/aspect-ratio-tool.js
Outdated
Show resolved
Hide resolved
Mamaduka
reviewed
May 2, 2024
packages/block-editor/src/components/dimensions-tool/aspect-ratio-tool.js
Outdated
Show resolved
Hide resolved
Mamaduka
reviewed
May 2, 2024
| @@ -397,8 +407,10 @@ class WP_Theme_JSON_Gutenberg { | |||
| ), | |||
| 'custom' => null, | |||
| 'dimensions' => array( | |||
| 'aspectRatio' => null, | |||
| 'minHeight' => null, | |||
| 'aspectRatio' => null, | |||
There was a problem hiding this comment.
Unrelated: I noticed that you can't disable aspect ration for Image block using this setting. cc @draganescu
There was a problem hiding this comment.
Was it supposed to be 😄 What is the expected behaviour here:
- does setting dimensions ≻ aspect ratio ≻ null disable the aspect ratio tools somewhere else?
- should it disappear from the UI of the image block? It does not appear in styles, but in settings, whereas for the cover block the UI appears in styles not in settings. What's preffered?
- should it be null or false? When I set theme.json ≻ settings ≻ dimensions ≻ aspectRatio the editor says it should be bool
There was a problem hiding this comment.
The initial null value is how theme json defines flag settings, and it's not important at the moment.
Setting it to false disables the aspect ratio UI for blocks. You can test it with the Cover block. I think behavior should be consistent across the blocks that use aspect ratio control.
There was a problem hiding this comment.
Yes, indeed setting to false hides this dimension tool from the cover block but it does not hide it from the image block. The reason is that the cover block uses the supports while the image block plops the control in the inspector.
There was a problem hiding this comment.
Yes. I'm aware of that. Still, the behavior should be consistent.
@fabiankaegy, do you mind creating an issue for the Image and Featured Image block follow-ups we discussed in Slack?
There was a problem hiding this comment.
Yes. I'm aware of that.
I knew that, you told me where to look! 😂 I mentioned here for archaeology reasons.
There was a problem hiding this comment.
Created a follow up issue for the aspect ratio block support cleanup: #61432
fabiankaegy
added
Needs Dev Note
|
@ajlende wanted to tag you specifically here for feedback since @oandregal mentioned you recently worked on this area and also you were one of the earlier contributors to this PR before I took it over :) |
|
Did some user testing on this branch and was able to get through all of the instructions testing custom & default aspect ratios across multiple block themes. |
|
One note about the testing instructions. Should it list the specific blocks that use aspect ratios? Here's what I tested but I may have missed some:
|
@bacoords I'll update the testing instructions to be a little more concise. But you are correct those are all the instances. The one thing to note it that for the image block it shows in two places. The sidebar setting for the aspect ratio and then also in the croping tools in the sidebar |
Mamaduka
approved these changes
May 7, 2024
|
I just created a sync PR on WordPress Develop for the PHP updates to the theme.json class https://core.trac.wordpress.org/ticket/61137 |
|
Internationalization needs to be fixed before this lands in core. See #61767 |
|
@swissspidy thanks for calling this out. I have created #61774 to fix this here in Gutenberg and also updated my PR in core to include the fix. Question for you. These strings were previously already present in core. Just hardcoded in JS and piped through |
|
They will be marked as fuzzy because now they have proper context (_x vs __) |
49 tasks
Dev Note DraftAdd custom Aspect Ratio presets through
|
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?
This PR adds support for aspect ratio presets via theme.json. It adds by default the same presets that are available for the crop utilities in the image block.
Why?
Part of #38990
How?
Bootstraps the presets via the theme.json class.
Testing Instructions
settings.dimensions.aspectRatios) and verify they appearsettings.dimensions.defaultAspectRatios->falseand verify they do not appearSample `theme.json`
{ "version": 3, "settings": { "dimensions": { "defaultAspectRatios": true, "aspectRatios": [ { "name": "Extra Wide - 2:1", "ratio": "2/1", "slug": "2-1" } ] } } }Testing Instructions for Keyboard
N/A, backend only change.
Screenshots or screencast
CleanShot.2024-05-02.at.11.35.18.mp4