Skip to content
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

Comment Author Gravatar Block #30575

Closed
jameskoster opened this issue Apr 7, 2021 · 17 comments · Fixed by #35396
Closed

Comment Author Gravatar Block #30575

jameskoster opened this issue Apr 7, 2021 · 17 comments · Fixed by #35396
Assignees
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Feature] Blocks Overall functionality of blocks [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") New Block Suggestion for a new block [Status] In Progress Tracking issues with work in progress

Comments

@jameskoster
Copy link
Contributor

Here is an initial take on the design for the Comment Author Gravatar block. This block should only be available in the Site Editor, and potentially only when editing certain templates (Index, Single, Singular, Page).

comment-author-gravatar

Design feedback is politely requested for all aspects:

  • Icon
  • Placeholder when there is no context present
  • Toolbar actions
  • Inspector settings
  • Name
  • Description

Figma link here.

@jameskoster jameskoster added Needs Design Feedback Needs general design feedback. [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Apr 7, 2021
@aristath
Copy link
Member

aristath commented Apr 8, 2021

Could this be author-avatar and not author-gravatar? Most sites have some sort of avatar, but not everyone uses gravatar, and forcing a core block to be tied to an external service would be controversial to many.

@jameskoster
Copy link
Contributor Author

I don't disagree, but WordPress is already tied to Gravatar, so I don't know if this is the time or place for that discussion?

This block would basically be an equivalent of get_avatar().

@jameskoster
Copy link
Contributor Author

I do wonder now whether this block should be more generic so that it can be used in both comment and post loops. "Author Gravatar" or something.

@aristath
Copy link
Member

aristath commented Apr 8, 2021

This block would basically be an equivalent of get_avatar().

Exactly. get_avatar() gets not only the gravatar, but any avatar the site is using. Since we don't want to limit the block to just gravatars and instead we want it to use whatever the site-author has decided to use on their site, "gravatar" seems a bit too specific (and inaccurate) as the block-name.

@jameskoster
Copy link
Contributor Author

Ah yes, I'm with you now.

"Author Avatar" or even just "Avatar" could work.

@paaljoachim
Copy link
Contributor

It would be nice to also give the opportunity to upload another default avatar image.
I assume that styles is a round or square style. Instead of style one can use a Border Radius control. Similar to the Buttons block Border settings. To go from a square and then a roundish shape and all the way to a circle.

@jameskoster
Copy link
Contributor Author

It would be nice to also give the opportunity to upload another default avatar image.

That's an interesting idea. I suppose we could pull the options from the avatar settings:

Screenshot 2021-04-21 at 09 45 25

into the "Advanced" panel on the block?

I assume that styles is a round or square style.

Since the block renders an image, I assume it will inherit styles from the image block. That may not be the case though, and in general I agree that particular style would be better handled by the border radius control.

@paaljoachim
Copy link
Contributor

Well the avatar settings show really really old avatars (as it has not been updated in many years).
It would be better to have the opportunity to upload an avatar, as set it as a new default avatar for the site.

@jameskoster
Copy link
Contributor Author

That might be nice in the future, but for now it probably makes sense to work with what we have.

@SantosGuillamot
Copy link
Contributor

Hello! 👋 I would like to wrap up all the issues related to the Post Comments block to start working on a Comment Loop block as explained in this tracking issue. I'd like to make a quick summary to ensure we are aligned on this one:

Description

Adds the author avatar of the specific comment.

Potential Settings

  • Image Settings:
    • Image size: Select the avatar size as it's being done in the Post Author block when avatar is enabled.
    • Rounded: It would be nice to have the possibility to make the avatar rounded if needed.

Requirements

  • There is a Discussion Setting to select a default avatar for users without a custom avatar of their own. That should be respected.

References

Of course, any feedback is welcome 🙂

@SantosGuillamot SantosGuillamot added [Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Feature] Blocks Overall functionality of blocks [Feature] Full Site Editing New Block Suggestion for a new block and removed Needs Design Feedback Needs general design feedback. labels Sep 21, 2021
@cbravobernal
Copy link
Contributor

Hi there! May I take this one?

Regarding the rounded option. What would be better for this case for the options? A toggle option or a text input for the border radius?

@luisherranz
Copy link
Member

What would be better for this case for the options? A toggle option or a text input for the border radius?

Can't we reuse the control used in blocks like the Button?

Screenshot 2021-09-23 at 10 17 51

@SantosGuillamot SantosGuillamot added the [Status] In Progress Tracking issues with work in progress label Sep 23, 2021
@jameskoster
Copy link
Contributor Author

Yeah let's add the border panel :) The Dimensions panel is also worth considering.

@cbravobernal
Copy link
Contributor

cbravobernal commented Sep 23, 2021

Hi @jameskoster! The Dimensions panel, you mean for the Avatar size?

I've taken a look at the post author avatar block. Regarding the size, I have seen that we receive a variable with 3 different size URL values.

avatar_urls

The approach, in this case, has been creating a selector with the three options.
avatar_size_selector

If we want a more dynamic option, like the width-height options. We can think about different approaches, of course, feel free to add more ideas, these are the ones I can guess right now:

  • Set a maximum width-height of 96px for those options. Depending on the user selection, use each maximum width (96 automatically avoiding all time a "bigger resize".

  • Invest some time to investigate how we are providing those avatar URLs. Check which is the maximum size we can get, as those images are all provided by Gravatar, even the default ones.

Any recommendations about how to proceed? 🙂

@jameskoster
Copy link
Contributor Author

I was thinking more about the padding options that come with the dimensions panel, but this is a good question!

The design in the OP is based on the regular image block:

Screenshot 2021-09-23 at 12 46 59

These seem more flexible than the pre-determined sizes in the current block.

I didn't include the alt text option as this should be generated dynamically. The "Image size" option was also omitted as that relates specifically to items in the media library (which gravatars are not).

I suspect that the height/width settings will eventually move to the dimensions panel, but cannot say for sure. So in the mean time this approach makes the most sense to me. What do you think?

@cbravobernal
Copy link
Contributor

Hi there!

I've done some work on the component. If you prefer just to read, feel free to skip it and continue!
https://www.loom.com/share/54114bf6634e43ce9d365f15c1c5b4e7

All the code available is on this PR #35180

I've added both the border-radius option and the Image Dimensions field to the Comment Avatar. We have also the 25%, 50%, 75% and 100%. Right now, we are using the biggest size available on the WordPress site for Avatars, which is 96px, but you can increase to whatever size you want.

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

For the resize box, I've put what we have for images, one dot on the bottom and the other one on the right or on the left, depending on if the site is RTL or not. In images, it also depends on the image alignment, but I think we won't have that alignment option on the comment Avatar.

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?

I will now work on the frontend display. It will use 96px, 48px or 24px size depending on the minimum width set up on the editor. (24 if the width is set up in less than 25, 48 if is less than 49, and 96 for the rest).

@jameskoster jameskoster linked a pull request Sep 28, 2021 that will close this issue
6 tasks
@jameskoster
Copy link
Contributor Author

Nice! I'll leave feedback on the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Comments Affects the Comments Block - formerly known as Comments Query Loop [Feature] Blocks Overall functionality of blocks [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") New Block Suggestion for a new block [Status] In Progress Tracking issues with work in progress
Projects
None yet
6 participants