-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix: Refactor Featured Image Block to use aspectRatio block support #61485
base: trunk
Are you sure you want to change the base?
Conversation
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @eric-michel. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I've not properly tested this, but I think the Featured Image won't retain the aspect ratio after the update.
|
Hi @Mamaduka, I tried the above steps and yes, the aspectRatio which is set is no more used. We need to make sure it is backward compatible, if aspectRatio is set, it should respect that, and if changed then we can use new one. Let me know if you have some idea how can we proceed with that? I tried some findings but did not succeed. Maybe we should retain the attribute and check if that is set, use that, and once change it from block support make it undefined. I am not sure what could be the better approach. Thank You. |
@@ -80,6 +75,9 @@ const DimensionControls = ( { | |||
label: name, | |||
} ) ); | |||
|
|||
// Get the block Supports aspect ratio. | |||
const aspectRatio = attributes?.style?.dimensions?.aspectRatio; |
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.
const aspectRatio = attributes?.style?.dimensions?.aspectRatio;
Above line can be changed to something like,
const blockSupportAspectRatio = attributes?.style?.dimensions?.aspectRatio || aspectRatio;
Here we should not remove the attribute from the block itself, we just keep it to get the value which was stored previously?
But it would have issue like, blockSupport
aspectRatio dropdown would not show the selected aspectRatio from the atttribute.
@hbhalodia can you confirm if this implementation of |
What?
Resolves #61432
Why?
aspectRatio
from attributes and used theBlock Supports
for thecore/post-featured-image
, because the attributes were redundant.How?
aspectRatio
from the attributes.block supports
,dimensions[aspectRatio]
in block.json.Testing Instructions
core/post-featured-image
block.Testing Instructions for Keyboard
Screenshots or screencast