Block Library: Add a Post Author block. #19576
Conversation
epiqueras
requested review from
ajitbohra,
jorgefilipecosta,
Soean,
talldan and
TimothyBJacobs
as code owners
|
|
||
| function PostAuthorDisplay() { | ||
| const [ author ] = useEntityProp( 'postType', 'post', 'author' ); | ||
| return <address>{ author }</address>; |
There was a problem hiding this comment.
Potentially we could show a Post Author Select if the block is selected. We could start with a readonly block as well.
There was a problem hiding this comment.
We still don't have an async Combobox component for this and I don't want that to block this structural PR.
There was a problem hiding this comment.
We should use the same thing we already have for the Post Author in the document sidebar (imperfect) but I don't see this as mandatory for that PR anyway.
There was a problem hiding this comment.
We should use the same thing we already have for the Post Author
Not on that though. Implementing that is so complex that it will be faster to implement the generic Combobox first and use that instead.
There was a problem hiding this comment.
In my testing, this shows the id of the author, I believe we should show the username at least and it should be shown exactly like the fontend By %s
There was a problem hiding this comment.
Oops, the post object references author IDs. I've updated the code to fetch the actual author object and use its name instead.
|
|
||
| export default function PostAuthorEdit() { | ||
| if ( ! useEntityId( 'postType', 'post' ) ) { | ||
| return 'Post Author Placeholder'; |
| if ( ! $post ) { | ||
| return ''; | ||
| } | ||
| return '<address>' . __( 'By ' ) . get_the_author() . '</address>'; |
There was a problem hiding this comment.
Should this use sprintf?
There was a problem hiding this comment.
What would be the benefit?
There was a problem hiding this comment.
In some languages, there might be no need for "by " or the author should come first. Ideally concatenation of translated strings and non translated ones should be avoided as much as possible because the sentence format changes from language to another.
There was a problem hiding this comment.
Oh, right. I totally forgot about the translation here.
Thanks for catching it.
Fixed!
|
Added "design feedback" tag for designers to start thinking about the potential of these blocks (style variations, styling...) |
epiqueras
force-pushed
the
add/post-author-block
branch
from
fc9a078
to
5ec16de
Compare
| ( select ) => select( 'core' ).getEntityRecord( 'root', 'user', authorId ), | ||
| [ authorId ] | ||
| ); | ||
| return author ? <address>{ sprintf( __( 'By %s' ), author.name ) }</address> : null; |
There was a problem hiding this comment.
In slow connection, there's a jumpiness happening when loading the author. This should be solved with design somehow.
There was a problem hiding this comment.
We need loading block placeholders for these "async blocks."
| return ''; | ||
| } | ||
| // translators: %s: The author. | ||
| return sprintf( __( '<address>By %s</address>' ), get_the_author() ); |
There was a problem hiding this comment.
This is causing a lint error on WP.com when attempting to update the plugin there:
!! Please put the opening and closing HTML tags outside of the translatable string:
wp-content/plugins/gutenberg-core/7.3.0-alpha/build/block-library/blocks/post-author.php:19 __( "<address>By %s</address>" )
which seems fair. @chriskmnds is working on a fix.
If core contributors feel that a lint check this makes sense, is there any chance we can add one to CI?
/cc @akirk
There was a problem hiding this comment.
Why is this an issue? It would just make the code harder to read with the concatenations to what end?
There was a problem hiding this comment.
The suggestion is to have translators only translate By %s. For one, it lets the translator focus on the text. Secondly, there is possible translation reuse because By %s has already been translated. Finally, if the HTML tags are added through code, it means you can change them later to add CSS classes, ids, whatnot, without having to change the translations.
There was a problem hiding this comment.
It makes easier for translators because the html tag in that case will just be repeated. It doesn't make sense to allow translators to change the HTML tag.
There was a problem hiding this comment.
That makes sense.
Should I open a PR or are you already on it?
There was a problem hiding this comment.
I have a fix, but putting together the PR properly, testing, etc. will take a while (even for something simple as this). :D
There was a problem hiding this comment.
will test tomorrow, but feel free to review pls :-)
There was a problem hiding this comment.
For posterity's sake, there is a follow-up task related to this conversation at: #19911
Description
This PR adds a new Post Author block akin to the Post Title and Post Content blocks.
How has this been tested?
Types of Changes
New Feature: There is a new Post Author block for template building.
Checklist: