-
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
Add spacing attributes to comment author avatar #36322
Add spacing attributes to comment author avatar #36322
Conversation
Size Change: +45 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
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.
It looks like this PR is on track. @oandregal, is there a better way for spacing handling in PHP? useSpacingProps
usage in the edit
function is very convenient, it would be super nice to have something similar in PHP.
81a4369
to
691c0a9
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.
I think I missed this PR 😅
@@ -45,7 +61,10 @@ function render_block_core_comment_author_avatar( $attributes, $content, $block | |||
'class' => $classes, | |||
) | |||
); | |||
return $avatar_string; | |||
if ( isset( $spacing_attributes ) ) { | |||
return sprintf( '<div style="%1s">%2s</div>', $spacing_string, $avatar_block ); |
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.
One thing that came up to my mind. Should we sanitize the computed string for the style?
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.
Yep, seems a good idea. Should I create a new PR, right?
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, that's the best way to move forward. It's also why a proper API would be better as we would have a central place to address this type of concers 😄
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.
Fixed in #36988, thank you!
Yeah, we should extract utility functions from the "apply" functions in block supports. Aaron had a PR that also needed it for typography #36104 |
Description
With this PR, we are adding spacing attributes (margin and padding) to the Comment Author Avatar.
We include specific PHP code in order to apply some styles to the wrapper and other styles to the image tag.
Please, feel free to propose a better solution if possible, as I could not find any core function that provides the attribute non serialized in a style HTML attribute string.
Maybe is a good idea to include a function to return scoped styles and classes with supports fields in block.json attributes. For example:
`get_scoped_style_and_classes(['spacing', 'border']);
I would be happy to work on that function if is a good approach.
How has this been tested?
Screenshots
Editor:
Frontend:
![frontend](https://user-images.githubusercontent.com/37012961/140803790-4ab66ade-3380-4ad5-a167-3e34bf6525ba.png
Types of changes
New small feature. Comment Author Avatar Update.
Checklist:
*.native.js
files for terms that need renaming or removal).