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

Block Library: Add a Post Author block. #19576

Merged
merged 6 commits into from Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/blocks.php
Expand Up @@ -56,6 +56,7 @@ function gutenberg_reregister_core_block_types() {
'template-part.php' => 'core/template-part',
'post-title.php' => 'core/post-title',
'post-content.php' => 'core/post-content',
'post-author.php' => 'core/post-author',
'post-excerpt.php' => 'core/post-excerpt',
);

Expand Down
10 changes: 9 additions & 1 deletion packages/block-library/src/index.js
Expand Up @@ -68,6 +68,7 @@ import * as siteTitle from './site-title';
import * as templatePart from './template-part';
import * as postTitle from './post-title';
import * as postContent from './post-content';
import * as postAuthor from './post-author';
import * as postExcerpt from './post-excerpt';

/**
Expand Down Expand Up @@ -188,7 +189,14 @@ export const __experimentalRegisterExperimentalCoreBlocks =

// Register Full Site Editing Blocks.
...( __experimentalEnableFullSiteEditing ?
[ siteTitle, templatePart, postTitle, postContent, postExcerpt ] :
[
siteTitle,
templatePart,
postTitle,
postContent,
postAuthor,
postExcerpt,
] :
[] ),
].forEach( registerBlock );
} :
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/post-author/block.json
@@ -0,0 +1,4 @@
{
"name": "core/post-author",
"category": "layout"
}
22 changes: 22 additions & 0 deletions packages/block-library/src/post-author/edit.js
@@ -0,0 +1,22 @@
/**
* WordPress dependencies
*/
import { useEntityProp, useEntityId } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { sprintf, __ } from '@wordpress/i18n';

function PostAuthorDisplay() {
const [ authorId ] = useEntityProp( 'postType', 'post', 'author' );
const author = useSelect(
( select ) => select( 'core' ).getEntityRecord( 'root', 'user', authorId ),
[ authorId ]
);
return author ? <address>{ sprintf( __( 'By %s' ), author.name ) }</address> : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i18n?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
return <PostAuthorDisplay />;
}
18 changes: 18 additions & 0 deletions packages/block-library/src/post-author/index.js
@@ -0,0 +1,18 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import metadata from './block.json';
import edit from './edit';

const { name } = metadata;
export { metadata, name };

export const settings = {
title: __( 'Post Author' ),
edit,
};
33 changes: 33 additions & 0 deletions packages/block-library/src/post-author/index.php
@@ -0,0 +1,33 @@
<?php
/**
* Server-side rendering of the `core/post-author` block.
*
* @package WordPress
*/

/**
* Renders the `core/post-author` block on the server.
*
* @return string Returns the filtered post author for the current post wrapped inside "h6" tags.
*/
function render_block_core_post_author() {
$post = gutenberg_get_post_from_context();
if ( ! $post ) {
return '';
}
// translators: %s: The author.
return sprintf( __( '<address>By %s</address>' ), get_the_author() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

cc @aduth @youknowriad

Copy link
Contributor

@akirk akirk Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#19675

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

/**
* Registers the `core/post-author` block on the server.
*/
function register_block_core_post_author() {
register_block_type(
'core/post-author',
array(
'render_callback' => 'render_block_core_post_author',
)
);
}
add_action( 'init', 'register_block_core_post_author' );
1 change: 1 addition & 0 deletions packages/core-data/src/entities.js
Expand Up @@ -17,6 +17,7 @@ export const defaultEntities = [
{ name: 'media', kind: 'root', baseURL: '/wp/v2/media', plural: 'mediaItems' },
{ name: 'taxonomy', kind: 'root', key: 'slug', baseURL: '/wp/v2/taxonomies', plural: 'taxonomies' },
{ name: 'widgetArea', kind: 'root', baseURL: '/__experimental/widget-areas', plural: 'widgetAreas', transientEdits: { blocks: true } },
{ name: 'user', kind: 'root', baseURL: '/wp/v2/users', plural: 'users' },
];

export const kinds = [
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/fixtures/blocks/core__post-author.html
@@ -0,0 +1 @@
<!-- wp:post-author /-->
10 changes: 10 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__post-author.json
@@ -0,0 +1,10 @@
[
{
"clientId": "_clientId_0",
"name": "core/post-author",
"isValid": true,
"attributes": {},
"innerBlocks": [],
"originalContent": ""
}
]
18 changes: 18 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__post-author.parsed.json
@@ -0,0 +1,18 @@
[
{
"blockName": "core/post-author",
"attrs": {},
"innerBlocks": [],
"innerHTML": "",
"innerContent": []
},
{
"blockName": null,
"attrs": {},
"innerBlocks": [],
"innerHTML": "\n",
"innerContent": [
"\n"
]
}
]
@@ -0,0 +1 @@
<!-- wp:post-author /-->