Block Library: Add a Post Author block. by epiqueras · Pull Request #19576 · WordPress/gutenberg

epiqueras · 2020-01-11T01:27:21Z

Description

This PR adds a new Post Author block akin to the Post Title and Post Content blocks.

How has this been tested?

Inserted Post Author block in a post.
Confirmed post author rendered in the editor and front end.
Inserted Post Author block in a template.
Confirmed post author placeholder rendered in the editor and the relevant post author rendered in the front end.

Types of Changes

New Feature: There is a new Post Author block for template building.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

youknowriad · 2020-01-13T10:17:44Z

+
+function PostAuthorDisplay() {
+	const [ author ] = useEntityProp( 'postType', 'post', 'author' );
+	return <address>{ author }</address>;


Potentially we could show a Post Author Select if the block is selected. We could start with a readonly block as well.

We still don't have an async Combobox component for this and I don't want that to block this structural PR.

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.

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.

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

Oops, the post object references author IDs. I've updated the code to fetch the actual author object and use its name instead.

youknowriad · 2020-01-13T10:18:03Z

+
+export default function PostAuthorEdit() {
+	if ( ! useEntityId( 'postType', 'post' ) ) {
+		return 'Post Author Placeholder';


#19579 (comment)

youknowriad · 2020-01-13T10:18:28Z

+	if ( ! $post ) {
+		return '';
+	}
+	return '<address>' . __( 'By ' ) . get_the_author() . '</address>';


Should this use sprintf?

What would be the benefit?

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.

Oh, right. I totally forgot about the translation here.

Thanks for catching it.

Fixed!

youknowriad · 2020-01-13T10:20:14Z

Added "design feedback" tag for designers to start thinking about the potential of these blocks (style variations, styling...)

… of their ID.

youknowriad · 2020-01-14T16:40:13Z

+		( select ) => select( 'core' ).getEntityRecord( 'root', 'user', authorId ),
+		[ authorId ]
+	);
+	return author ? <address>{ sprintf( __( 'By %s' ), author.name ) }</address> : null;


In slow connection, there's a jumpiness happening when loading the author. This should be solved with design somehow.

We need loading block placeholders for these "async blocks."

ockham · 2020-01-15T14:15:26Z

+		return '';
+	}
+	// translators: %s: The author.
+	return sprintf( __( '<address>By %s</address>' ), get_the_author() );


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

Why is this an issue? It would just make the code harder to read with the concatenations to what end?

cc @aduth @youknowriad

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.

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.

That makes sense.

Should I open a PR or are you already on it?

I have a fix, but putting together the PR properly, testing, etc. will take a while (even for something simple as this). :D

#19675

will test tomorrow, but feel free to review pls :-)

For posterity's sake, there is a follow-up task related to this conversation at: #19911

epiqueras added the [Feature] Full Site Editing label Jan 11, 2020

epiqueras added this to the Future milestone Jan 11, 2020

epiqueras requested review from Soean, TimothyBJacobs, ajitbohra, jorgefilipecosta and talldan as code owners January 11, 2020 01:27

epiqueras self-assigned this Jan 11, 2020

epiqueras requested a review from youknowriad January 11, 2020 01:27

MarcoZehe suggested changes Jan 11, 2020

View reviewed changes

Comment thread packages/block-library/src/post-author/edit.js Outdated

Soean reviewed Jan 11, 2020

View reviewed changes

Comment thread packages/block-library/src/post-author/index.php Outdated

youknowriad reviewed Jan 13, 2020

View reviewed changes

youknowriad added the Needs Design Feedback Needs general design feedback. label Jan 13, 2020

epiqueras added 5 commits January 13, 2020 11:35

Block Library: Add a Post Author block.

16ac572

Block Library: Localize Post Author block "By " text fragment.

49eab59

Block Library: Use address tag in Post Author block.

225b79d

Block Library: Use sprintf in Post Author block.

ae1861d

E2E Tests: Generate Post Author block fixtures.

5ec16de

epiqueras force-pushed the add/post-author-block branch from fc9a078 to 5ec16de Compare January 13, 2020 16:42

epiqueras requested review from nerrad and ntwb as code owners January 13, 2020 16:42

epiqueras mentioned this pull request Jan 13, 2020

Query Block Automattic/newspack-blocks#176

Closed

9 tasks

Block Library: Render actual author name in Post Author block instead…

4fb15c7

… of their ID.

youknowriad reviewed Jan 14, 2020

View reviewed changes

youknowriad approved these changes Jan 14, 2020

View reviewed changes

epiqueras merged commit 6118c92 into master Jan 14, 2020

epiqueras deleted the add/post-author-block branch January 14, 2020 17:12

ockham reviewed Jan 15, 2020

View reviewed changes

ellatrix modified the milestones: Future, Gutenberg 7.3 Jan 20, 2020

kjellr added the New Block Suggestion for a new block label Feb 4, 2020

naiba4 mentioned this pull request Jan 4, 2024

[Snyk] Security upgrade appium from 1.22.3 to 2.4.0 naiba4/gutenberg#36

Open

MaxMood96 mentioned this pull request Jan 4, 2024

[Snyk] Security upgrade appium from 1.22.3 to 2.4.0 MaxMood96/gutenberg#130

Open

Conversation

epiqueras commented Jan 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Types of Changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad Jan 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad Jan 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youknowriad commented Jan 13, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akirk Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

epiqueras commented Jan 11, 2020 •

edited

Loading

youknowriad Jan 14, 2020 •

edited

Loading

youknowriad Jan 13, 2020 •

edited

Loading

akirk Jan 15, 2020 •

edited

Loading