Site Editor: Add new Post Comment Avatar block #35180

cbravobernal · 2021-09-28T10:05:23Z

Moved to a cleanest PR -> PR #35396

We added a new Post Comment Avatar block. Related to issue #30575.

How has this been tested?

WordPress Version: 5.8.1

Gutenberg Version: 11.6.0-rc.1

Database: MySQL 8.0.16

Web Server: nginx

PHP Version: 7.3.5

Screenshots

Types of changes

Type Experimental
New feature Site Editor
Needs Accessibility Feedback (Although I tried with both keyboard and voice over and seems to be fine)
Needs Testing

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.

github-actions · 2021-09-28T10:11:35Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @c4rl0sbr4v0! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

jameskoster · 2021-09-28T11:07:45Z

Is there any recommendation on this? Should we set a maximum? Sizes of more than 96px will get blurry.

The 96px is a default value that comes from WordPress core, but themes can override it through get_avatar(), correct? Since this block essentially replaces that function, shouldn't the size values in the block settings dictate how large the image is? IE it should be possible to set any value.

The percentage controls are perhaps a little confusing because one users gravatar source image might be a different size to anothers. Perhaps we don't need them?

Also, if you only edit the height, is not increasing the image, as default CSS is giving a height: auto that preservers aspect ratio. Is that OK also?

Preserving aspect ratio makes sense to me.

It might also be interesting to try making the Dimensions panel available here so that folks can adjust the padding value.

The one other thing missing is the icon, we should probably add that to the library. Here's the svg:

<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path fill-rule="evenodd" clip-rule="evenodd" d="M7.25 16.4371C6.16445 15.2755 5.5 13.7153 5.5 12C5.5 8.41015 8.41015 5.5 12 5.5C15.5899 5.5 18.5 8.41015 18.5 12C18.5 13.7153 17.8356 15.2755 16.75 16.4371V16C16.75 14.4812 15.5188 13.25 14 13.25L10 13.25C8.48122 13.25 7.25 14.4812 7.25 16V16.4371ZM8.75 17.6304C9.70606 18.1835 10.8161 18.5 12 18.5C13.1839 18.5 14.2939 18.1835 15.25 17.6304V16C15.25 15.3096 14.6904 14.75 14 14.75L10 14.75C9.30964 14.75 8.75 15.3096 8.75 16V17.6304ZM4 12C4 7.58172 7.58172 4 12 4C16.4183 4 20 7.58172 20 12C20 16.4183 16.4183 20 12 20C7.58172 20 4 16.4183 4 12ZM14 10C14 11.1046 13.1046 12 12 12C10.8954 12 10 11.1046 10 10C10 8.89543 10.8954 8 12 8C13.1046 8 14 8.89543 14 10Z" fill="black"/>
</svg>

mtias · 2021-09-28T13:29:58Z

packages/block-library/src/post-comment-avatar/editor.scss

@@ -0,0 +1,5 @@
+.wp-block-post-comment-avatar img {


It seems we should be able to avoid editor-only styles for this one — what is it accomplishing?

The ResizableBox creates a div, this one the one that is getting the width and height based on options selected. Then we save those variables as attributes for the frontend display.
In order to get the img inside the div to grow as the user changes those variables, we add a width and height inherit class.

We have something similar on image block: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/image/editor.scss#L31-L39

We can look for another approach if needed of course!

It's something we should consider form the perspective of the DX — adding a component like ResizableBox should not be forcing me to supply editor styles, the component itself should be handling that for me.

I added a class to the main ResizableBox component in order for the next img to inherit the width. Let me know if that is ok. As that component is in a lot of different components of the editor :-)

packages/block-library/src/post-comment-avatar/block.json

ntsekouras

👋 - thanks for working on this Carlos!!

When adding a new core block, we need to also register it server side here: https://github.com/WordPress/gutenberg/blob/trunk/lib/blocks.php#L73.

Also you need to add a fixture for the auto generated tests to resolve the error:

Expected a fixture file called 'core__post-comment-avatar.html' or 'core__post-comment-avatar__*.html'.

When you're ready with the PR, create a core__post-comment-avatar.html file here and then run npm run test-unit test/integration/full-content/full-content.test.js . This will generate the json and parsed versions. The new file should have the content(html) of the block when is inserted(use the code editor to see the output).

packages/block-library/src/post-comment-avatar/block.json

packages/block-library/src/post-comment-avatar/edit.js

ntsekouras · 2021-09-28T14:28:56Z

packages/block-library/src/post-comment-avatar/edit.js

+	const { commentId } = context;
+
+	// Maximum provided by the WordPress site.
+	const maxImageWidth = 96;


I haven't checked this, by you might want to make sure there is no php filter for this value. WP has tons of filters 😄

What is the API to run PHP filters from JS?

You have a get_avatar hook that sets the size: https://developer.wordpress.org/reference/hooks/get_avatar/

Is this affecting the edit.js code, right?

I removed this code, instead, now we use the default values of block.json file. I hope that will be a better approach!

cbravobernal · 2021-09-29T14:12:17Z

The percentage controls are perhaps a little confusing because one users gravatar source image might be a different size to anothers. Perhaps we don't need them?

In order to remove this. We have to add a new property on ImageSizeControl core block-editor library. Should we do this on this PR or better create a new issue and see what the community prefers?

jameskoster · 2021-09-29T15:04:37Z

Good question. Maybe it makes sense to do that separately and give it the necessary attention?

ntsekouras

Thanks for iterating on this @c4rl0sbr4v0 ! Avatar will have many nuances but we can see what can be done iteratively.

I guess this PR should take into account the imminent renaming of these blocks: #34994 (comment)

Also I would suggest not to bother with the fixtures until the block is ready 😄

ntsekouras · 2021-09-30T14:12:10Z

packages/block-library/src/post-comment-avatar/edit.js

+	}, [] );
+
+	useEffect( () => {
+		setAttributes( {


When we setAttributes we also add a step in history (undo, redo). Since avatarUrls is an array will always be a new reference, so it causes a history trap. You can check this by inserting the block and try to undo. If you needed only this avatarUrls[ avatarUrls.length - 1 ] which is a string, this should be the dependency.

ntsekouras · 2021-09-30T14:13:28Z

packages/block-library/src/post-comment-avatar/edit.js

+
+	// Set default widht, height and url on first render.
+	useEffect( () => {
+		setDefaultSize( { defaultWidth: width, defaultHeight: height } );


I haven't checked all the nuances with the avatars, but I think we should take into account the core handling. For example this filter: https://developer.wordpress.org/reference/functions/rest_get_avatar_sizes/, which is responsible for author_avatar_urls value.

Also if we go this direction with a default value it shouldn't be the current width as this can change from the control and would work as you expected on insertion only. If we end up not getting this dynamically, we should have a hardcoded value (if it makes sense).

ntsekouras · 2021-09-30T14:15:55Z

packages/block-library/src/post-comment-avatar/save.js

+	const { url, alt, width, height, id, title } = attributes;
+	return (
+		<img
+			{ ...useBlockProps.save() }


You can pass props in useBlockProps.save like className.

ntsekouras · 2021-09-30T14:17:25Z

packages/block-library/src/post-comment-avatar/edit.js

+	const { comment } = useSelect(
+		( select ) => {
+			const { getEntityRecord } = select( coreStore );
+			return {


Nit: you can return the single value directly (not an object).

gziolo · 2021-10-07T08:50:06Z

Work moved to #35396.

cbravobernal mentioned this pull request Sep 28, 2021

Comment Author Gravatar Block #30575

Closed

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 28, 2021

jameskoster linked an issue Sep 28, 2021 that may be closed by this pull request

Comment Author Gravatar Block #30575

Closed

mtias reviewed Sep 28, 2021

View reviewed changes

packages/block-library/src/post-comment-avatar/block.json Outdated Show resolved Hide resolved

mtias added the New Block Suggestion for a new block label Sep 28, 2021

ntsekouras reviewed Sep 28, 2021

View reviewed changes

ntsekouras reviewed Sep 30, 2021

View reviewed changes

cbravobernal closed this Oct 6, 2021

cbravobernal force-pushed the trunk branch from 3b17852 to 6a9d667 Compare October 6, 2021 10:40

cbravobernal mentioned this pull request Oct 6, 2021

Site Editor: Add Comment Avatar Core Block #35396

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Site Editor: Add new Post Comment Avatar block #35180

Site Editor: Add new Post Comment Avatar block #35180

cbravobernal commented Sep 28, 2021 •

edited

github-actions bot commented Sep 28, 2021

jameskoster commented Sep 28, 2021

mtias Sep 28, 2021

cbravobernal Sep 28, 2021

mtias Sep 28, 2021

cbravobernal Sep 29, 2021

ntsekouras left a comment

ntsekouras Sep 28, 2021

luisherranz Sep 28, 2021

cbravobernal Sep 28, 2021

cbravobernal Sep 29, 2021

cbravobernal commented Sep 29, 2021

jameskoster commented Sep 29, 2021

ntsekouras left a comment •

edited

ntsekouras Sep 30, 2021

ntsekouras Sep 30, 2021

ntsekouras Sep 30, 2021

ntsekouras Sep 30, 2021

gziolo commented Oct 7, 2021

Site Editor: Add new Post Comment Avatar block #35180

Site Editor: Add new Post Comment Avatar block #35180

Conversation

cbravobernal commented Sep 28, 2021 • edited

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Sep 28, 2021

jameskoster commented Sep 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntsekouras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbravobernal commented Sep 29, 2021

jameskoster commented Sep 29, 2021

ntsekouras left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Oct 7, 2021

cbravobernal commented Sep 28, 2021 •

edited

ntsekouras left a comment •

edited