Site Editor: Add Comment Avatar Core Block #35396
Conversation
github-actions
bot
added
the
First-time Contributor
|
👋 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. |
getdave
added
[Package] Block library
cbravobernal
changed the title
There was a problem hiding this comment.
Nice work. I left some minor comments related to the code.
You will have to add test fixtures for the new block as noted in the failing unit test:
You can find more details at https://github.com/WordPress/gutenberg/tree/trunk/test/integration/fixtures/blocks#readme.
cbravobernal
force-pushed
the
add/comment-avatar
branch
from
53efec3 to
a8d299d
Compare
cbravobernal
force-pushed
the
add/comment-avatar
branch
from
f202621 to
4097c19
Compare
cbravobernal
requested review from
ajitbohra,
chrisvanpatten,
spacedmonkey and
TimothyBJacobs
as code owners
cbravobernal
changed the title
cbravobernal
force-pushed
the
add/comment-avatar
branch
from
4097c19 to
be0da2d
Compare
There was a problem hiding this comment.
It looks like the only remaining decision is to settle on how we handle avatar sizes.
The other concern is how to deal with backward compatibility challenges related to the fact that get_avatar function uses hooks internally. The HTML code in the editor might differ with what PHP outputs.
|
Looking great. A couple of thoughts on this one: Since we have padding and radius, should we also include a background color option, and border controls? These seem to go hand in hand and will give themers the flexibility to create really interesting styles. The name feels a little awkward, should it be "Comment Author Avatar" or "Commenter Avatar"? 🤔 |
cbravobernal
force-pushed
the
add/comment-avatar
branch
from
8341388 to
c16b851
Compare
| commentId | ||
| ); | ||
| const containerRef = useRef(); | ||
| const clientWidth = useClientWidth( containerRef ); |
There was a problem hiding this comment.
Does this value depend on the size of the screen? Won't it change when you resize the size of the browser?
There was a problem hiding this comment.
Yes, it changes while I resize the browser:
There was a problem hiding this comment.
It's taking the max width of the container. Should we put instead a fixed value?
There was a problem hiding this comment.
I don't see my comment. I don't know what would be the best here, but one thing is clear. The max value shouldn't depend on the editor's size max width available.
There was a problem hiding this comment.
I will put as maximum the same we have for images, maximum avatar size * 2.5
| alt={ `${ authorName } ${ __( 'Avatar' ) }` } | ||
| /> | ||
| </ResizableBox> | ||
| ) : null } |
There was a problem hiding this comment.
Should it have a fallback when there is no avatar? Some sort of general placeholder?
There was a problem hiding this comment.
I think WordPress by default is putting always an Avatar. In this case the placeholder would appear while fetching the comment data.
There was a problem hiding this comment.
Right, but when there are no avatars after fetching is complete, we should also have this fallback included.
There was a problem hiding this comment.
What should we put as fallback? A text?
I will take a look if we can put the default image in an easy way.
There was a problem hiding this comment.
The image. I even wonder whether it should be resizable.
That should be fixed. It was due to the lack of a div before the resize component. Could you please check it again? I could not reproduce it completely. |
I had code from 1 hour, so it might be that. I will retest tomorrow 👍🏻 |
cbravobernal
force-pushed
the
add/comment-avatar
branch
from
710147a to
5729707
Compare
There was a problem hiding this comment.
We still need to improve how the block can be selected when it occupies the full row. In practice, it might be a less popular option. I suspect that many designs will prefer to use a group block with a flex layout to have a few similar blocks in one line.
Anyway. I think that this PR is good enough to start testing it with other blocks necessary for the Comments Loop block.
This is the follow-up of a previous PR that was not following the workflow standards.
Replaces #35180.
Closes #30575.
More info on issue #30575.
Description
We are creating a new Comment Avatar Block. The user will be able to adjust the size, border-radius and padding of the Avatar component of all comments on its site by using the FSE template editor.
This component will be loaded by the parent component Query Loop Block, as part of the Comment Template. We have also more information on this tracking issue.
Loom of the last iteration:
https://www.loom.com/share/60f7c06c6acc4a5995f07beab75b359d
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: