Post Author Block: Show avatar placeholder and size control when no context #47211
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +67 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
t-hamano
changed the title
|
Flaky tests detected in ec96ff7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3937359789
|
t-hamano
commented
Jan 17, 2023
Comment on lines
+34
to
+37
| const DEFAULT_AVATAR_SIZES = [ 24, 48, 96 ].map( ( size ) => ( { | ||
| value: size, | ||
| label: `${ size } x ${ size }`, | ||
| } ) ); |
There was a problem hiding this comment.
The default value is hard-coded, but the avatar size retrieved by the REST API has a filter hook.
Therefore, if the consumer has changed the size variation on the hook, it may not match the default variation. However, there does not appear to be an endpoint to directly retrieve the result of the rest_get_avatar_sizes() function.
Perhaps we could solve this by making the size resizable, like a regular avatar block.
If you have a better way to do this, please let me know.
There was a problem hiding this comment.
I think that the more similar the blocks are, the easier it will be for the users.
t-hamano
added
[Type] Bug
|
🤔 I believe the intention was to switch to using the avatar block inside the post author block. |
t-hamano
changed the title
I found in #35596 that the Post Author block is scheduled to be split. In that case, would the current Post Author Block be deprecated? Either way, this block is also used in the Twenty Twenty Three theme, so I think a fix is in order. |
carolinan
reviewed
Jan 20, 2023
| } ) ); | ||
|
|
||
| const DEFAULT_AVATAR_URL = | ||
| 'https://www.gravatar.com/avatar/00000000000000000000000000000000?d=mp&f=y'; |
There was a problem hiding this comment.
Is this only used as a fallback?
There was a problem hiding this comment.
For example, suppose you inserted this block in Single template. On the front end, the author image would be displayed based on the actual articles. However, in Single template, the actual author image URL cannot be retrieved because the post context doesn't exist.
In such cases, avatars should be displayed as placeholders.
There was a problem hiding this comment.
But this remote url will not always be available, for example on closed networks.
I wonder if there were any prior discussions about it during development of the other avatar blocks.
There was a problem hiding this comment.
I examined the other blocks and found that the avatarUrl was being retrieved by __experimentalDiscussionSettings(), so I refactored it in 4b5988e. This URL will be http://1.gravatar.com/avatar/?s=96&d=mm&f=y&r=g, etc.
I have looked at #38072 and there does not appear to be any discussion on closed networks.
However, since the block pattern is also read from the remote network, that might not need to be taken into account.
There was a problem hiding this comment.
Thank you for tracking this down 👍
t-hamano
force-pushed
the
fix/post-author-avatar
branch
from
ec96ff7 to
4b5988e
Compare
|
The PR works well for me as is. |
|
Update: I've submitted #55352 to deprecate this block, so I'd like to close this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #35693
Fixes #47169
What?
This PR fixes the following two problems when there is no post context in the editor (i.e., when the author ID cannot be found):
Why?
In the current implementation, the avatar size variation relies on the existence of the author ID. The variations are generated from the
avatar_urlsof the information retrieved bygetUser().This implementation works as expected as long as it is within the post editor or query loop. However, if this block is placed outside the query context (e.g., in a single template), the avatar size variations would be an empty array and the avatar size dropdown would not render, since the author ID is not present.
As for avatar images, there is no fallback for the absence of the avatar URL, so even if "Show avatar" option is enabled, the avatar image will not be displayed.
I think even when there is no avatar information related with a post, the size should be able to changed and a placeholder image should be displayed.
How?
When no author information is available, the default size variation is used.
As for the avatar placeholder, the default mystery-person is displayed according to Gravatar's document. The request parameter that determines the size (
s=) includes the selected avatar size.Testing Instructions
In the following context, confirm that the author's avatar or placeholder image and the avatar size control are displayed.