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

Add Site Title block and required functionality. #17207

Merged
merged 9 commits into from
Oct 23, 2019
1 change: 1 addition & 0 deletions lib/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function gutenberg_reregister_core_block_types() {
'search.php' => 'core/search',
'social-link.php' => gutenberg_get_registered_social_link_blocks(),
'tag-cloud.php' => 'core/tag-cloud',
'site-title.php' => 'core/site-title',
);

$registry = WP_Block_Type_Registry::get_instance();
Expand Down
1 change: 1 addition & 0 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ $z-layers: (
".wp-block-cover__inner-container": 1, // InnerBlocks area inside cover image block
".wp-block-cover.has-background-dim::before": 1, // Overlay area inside block cover need to be higher than the video background.
".wp-block-cover__video-background": 0, // Video background inside cover block.
".wp-block-site-title__save-button": 1,

// Active pill button
".components-button.is-button {:focus or .is-primary}": 1,
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ The default editor settings
**experimentalEnableLegacyWidgetBlock boolean Whether the user has enabled the Legacy Widget Block
**experimentalEnableMenuBlock boolean Whether the user has enabled the Menu Block
**experimentalBlockDirectory boolean Whether the user has enabled the Block Directory
\_\_experimentalEnableFullSiteEditing boolean Whether the user has enabled Full Site Editing

<a name="SkipToSelectedBlock" href="#SkipToSelectedBlock">#</a> **SkipToSelectedBlock**

Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/src/store/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const PREFERENCES_DEFAULTS = {
* __experimentalEnableLegacyWidgetBlock boolean Whether the user has enabled the Legacy Widget Block
* __experimentalEnableMenuBlock boolean Whether the user has enabled the Menu Block
* __experimentalBlockDirectory boolean Whether the user has enabled the Block Directory
* __experimentalEnableFullSiteEditing boolean Whether the user has enabled Full Site Editing
*/
export const SETTINGS_DEFAULTS = {
alignWide: false,
Expand Down Expand Up @@ -152,6 +153,7 @@ export const SETTINGS_DEFAULTS = {
__experimentalEnableLegacyWidgetBlock: false,
__experimentalEnableMenuBlock: false,
__experimentalBlockDirectory: false,
__experimentalEnableFullSiteEditing: false,
Copy link
Contributor

@Copons Copons Oct 22, 2019

Choose a reason for hiding this comment

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

Since I've started to see this popping up in other PRs, what do you think if we add the Full Site Editing experiment plumbing in its own PR to make it easier to contribute towards this feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already been merged. This is just adding missing default values and JSDoc comments.

gradients: [
{
name: __( 'Vivid cyan blue to vivid purple' ),
Expand Down Expand Up @@ -228,4 +230,3 @@ export const SETTINGS_DEFAULTS = {
},
],
};

1 change: 1 addition & 0 deletions packages/block-library/src/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
@import "./text-columns/editor.scss";
@import "./verse/editor.scss";
@import "./video/editor.scss";
@import "./site-title/editor.scss";

/**
* Import styles from internal editor components used by the blocks.
Expand Down
33 changes: 23 additions & 10 deletions packages/block-library/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ import * as classic from './classic';
import * as socialLinks from './social-links';
import * as socialLink from './social-link';

// Full Site Editing Blocks
import * as siteTitle from './site-title';

/**
* Function to register an individual block.
*
Expand Down Expand Up @@ -162,14 +165,24 @@ export const registerCoreBlocks = () => {
* __experimentalRegisterExperimentalCoreBlocks( settings );
* ```
*/
export const __experimentalRegisterExperimentalCoreBlocks = process.env.GUTENBERG_PHASE === 2 ? ( settings ) => {
const { __experimentalEnableLegacyWidgetBlock, __experimentalEnableMenuBlock } = settings;
export const __experimentalRegisterExperimentalCoreBlocks =
process.env.GUTENBERG_PHASE === 2 ?
( settings ) => {
const {
__experimentalEnableLegacyWidgetBlock,
__experimentalEnableMenuBlock,
__experimentalEnableFullSiteEditing,
} = settings

[
__experimentalEnableLegacyWidgetBlock ? legacyWidget : null,
__experimentalEnableMenuBlock ? navigationMenu : null,
__experimentalEnableMenuBlock ? navigationMenuItem : null,
socialLinks,
...socialLink.sites,
].forEach( registerBlock );
} : undefined;
;[
__experimentalEnableLegacyWidgetBlock ? legacyWidget : null,
__experimentalEnableMenuBlock ? navigationMenu : null,
__experimentalEnableMenuBlock ? navigationMenuItem : null,
socialLinks,
...socialLink.sites,

// Register Full Site Editing Blocks.
...( __experimentalEnableFullSiteEditing ? [ siteTitle ] : [] ),
].forEach( registerBlock );
} :
undefined;
4 changes: 4 additions & 0 deletions packages/block-library/src/site-title/block.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "core/site-title",
"category": "layout"
}
39 changes: 39 additions & 0 deletions packages/block-library/src/site-title/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* WordPress dependencies
*/
import {
useEntityProp,
__experimentalUseEntitySaving,
} from '@wordpress/core-data';
import { Button } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { RichText } from '@wordpress/block-editor';

export default function SiteTitleEdit() {
const [ title, setTitle ] = useEntityProp( 'root', 'site', 'title' );
const [ isDirty, isSaving, save ] = __experimentalUseEntitySaving(
'root',
'site',
'title'
);
return (
<>
<Button
isPrimary
className="wp-block-site-title__save-button"
disabled={ ! isDirty || ! title }
isBusy={ isSaving }
onClick={ save }
>
{ __( 'Update' ) }
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably remove that button once we land the other PR (multi-entity saving)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep!

<RichText
Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding allowedFormats={ [] }?
Site options typically don't support HTML, and in fact if you try, for example, to make the title bold, it would be eventually output as <strong>Site Title</strong>.

Copy link
Member

Choose a reason for hiding this comment

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

There's also <PlainText>.

Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIK <PlainText> renders as a textarea in the editor which, among other quirks, makes it very annoying to keep it styled consistently with the front end.
On WP.com we used to use it for the Site Title and Site Description blocks, but eventually I replaced them both with a <RichText>, making it very much easier to handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

allowedFormats={ [] } makes sense here. This was just opened when we still had the malfunctioning formattingControls.

I'm adding it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like if it doesn't support HTML at all, PlainText is the way to go. Even if we disallow formats from a RichText, it's still a RichText and someone can tweak the HTML and pass it and it will render.

We should seek to fix PlainText issues instead IMO. How can we make it easier to style...

Copy link
Contributor

Choose a reason for hiding this comment

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

While I can see the reasons against using RichText, I also disagree with using PlainText.
PlainText is pretty much a simple textarea.
This causes the following problems, among others:

  • Site Title and Description are single line (input fields in wp-admin), while textarea is natively multiline.
    This means that we'll need to add a JS hack on enter keydown (or on paste multiline content, etc.).
    RichText already has a built-in onSplit/onReplace logic that takes care of this.

  • PlainText uses TextareaAutosize to autoresize the textarea height.
    But it doesn't automatically resize if late-injected CSS (such as theme editor styles) changes the font size.
    RichText simply adds contenteditable to any HTML element we want (e.g. h1 for the Site Title, p for the Description), delegating the resize to the element itself.

  • textarea (and other input fields) doesn't inherit the CSS from outside like any other elements.
    We would end up having to style the Site Title in the front end and in the editor in two different ways (for example, we might need to explicitly providing the font family or color to the editor's Site Title, while they would be inherited from the body on the front end).
    RichText would use the same element and CSS class as the front end, making the style consistency efforts much easier for the themes.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are all good reasons to improve PlainText and not use RichText to me.

1- If we one a single line text, we can add a prop for that and use an input.
2- This sounds like a bug that should be fixed
3- That's harder, and potentially we could solve it by adding font-size: inherit and things like that to PlainText.

Worst-case scenario we could refactor PlainText to use a contenteditable but we shouldn't be using RichText as it's mean for RichText (formatted html) and we shouldn't try to hack its purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are all good reasons to improve PlainText and not use RichText to me.

I can give it a try, if y'all don't mind.
If we plan to move SiteTitle to PlainText, I'll also need to keep the WPCOM version consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do, that's great 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I gave this a try and have some doubts.

1- If we one a single line text, we can add a prop for that and use an input.

Though, input can't wrap text: if the Title is longer than the input width, it would end up half hidden, instead of wrapping in a new line.

As of now, I've got some good enough ways to prevent line breaks (maybe replacing them with an insertDefaultBlock, for example), but it still feels very fragile.

2- This sounds like a bug that should be fixed

This is where I'm stuck.
We don't have a proper way of listening to font size changes, and TextareaAutosize doesn't expose any API to force a resize anyway.

3- That's harder, and potentially we could solve it by adding font-size: inherit and things like that to PlainText.

This was actually easier than expected: the PlainText's CSS already have everything set to inherit 😄


Worst-case scenario we could refactor PlainText to use a contenteditable but we shouldn't be using RichText as it's mean for RichText (formatted html) and we shouldn't try to hack its purpose.

In the end, I think we are back to this.
But the fact is that RichText already contains all the needed logic here, it's tried and tested, and works as expected.
Should I duplicate that much code and create a new ContentEditableText component, in order to remove formatted HTML support and cater to a handful of new blocks?

I'm not being dismissive, but I'm unsure how to proceed.
For example, I guess I could extract the underlying RichText logic into a new basic component, that's more appropriate for "plainer" texts, and then import and extend it for RichText.

tagName="h1"
placeholder={ __( 'Site Title' ) }
value={ title }
onChange={ setTitle }
allowedFormats={ [] }
/>
</>
);
}
6 changes: 6 additions & 0 deletions packages/block-library/src/site-title/editor.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.wp-block-site-title__save-button {
position: absolute;
right: 0;
top: 0;
z-index: z-index(".wp-block-site-title__save-button");
}
12 changes: 12 additions & 0 deletions packages/block-library/src/site-title/icon.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* WordPress dependencies
*/
import { SVG, Path, Circle } from '@wordpress/components';

export default (
<SVG xmlns="https://www.w3.org/2000/svg" viewBox="0 0 24 24">
<Path fill="none" d="M0 0h24v24H0V0z" />
<Path d="M12 2C8.13 2 5 5.13 5 9c0 5.25 7 13 7 13s7-7.75 7-13c0-3.87-3.13-7-7-7zM7 9c0-2.76 2.24-5 5-5s5 2.24 5 5c0 2.88-2.88 7.19-5 9.88C9.92 16.21 7 11.85 7 9z" />
<Circle cx="12" cy="9" r="2.5" />
</SVG>
);
20 changes: 20 additions & 0 deletions packages/block-library/src/site-title/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

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

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

export const settings = {
title: __( 'Site Title' ),
icon,
edit,
};
28 changes: 28 additions & 0 deletions packages/block-library/src/site-title/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* Server-side rendering of the `core/site-title` block.
*
* @package WordPress
*/

/**
* Renders the `core/site-title` block on the server.
*
* @return string The render.
*/
function render_block_core_site_title() {
return sprintf( '<h1>%s</h1>', get_bloginfo( 'name' ) );
Copy link
Contributor

@Copons Copons Oct 10, 2019

Choose a reason for hiding this comment

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

What about linking this to the home?

E.g.

<h1>
  <a href="<?php echo esc_url( home_url( '/' ) ); ?>">
    <?php bloginfo( 'name' ); ?>
  </a>
</h1>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe that should be an option/attribute?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a very cool idea!
And maybe the home_url could be the default.

Copy link

Choose a reason for hiding this comment

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

Hi,

What happened with the site title linking to the home page? I can be done like this:

function gutenberg_render_block_core_site_title() { return sprintf( '<h1 class="site-title"><a href="%s">%s</a></h1>', home_url( '/' ), get_bloginfo( 'name' ) ); }

Cheers!

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 could be a nice toggleable feature. Feel free to open an issue or PR for it 😄

}

/**
* Registers the `core/site-title` block on the server.
*/
function register_block_core_site_title() {
register_block_type(
'core/site-title',
array(
'render_callback' => 'render_block_core_site_title',
)
);
}
add_action( 'init', 'register_block_core_site_title' );
1 change: 1 addition & 0 deletions packages/core-data/src/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { apiFetch, select } from './controls';
export const DEFAULT_ENTITY_KEY = 'id';

export const defaultEntities = [
{ name: 'site', kind: 'root', baseURL: '/wp/v2/settings' },
{ name: 'postType', kind: 'root', key: 'slug', baseURL: '/wp/v2/types' },
{ name: 'media', kind: 'root', baseURL: '/wp/v2/media', plural: 'mediaItems' },
{ name: 'taxonomy', kind: 'root', key: 'slug', baseURL: '/wp/v2/taxonomies', plural: 'taxonomies' },
Expand Down
76 changes: 74 additions & 2 deletions packages/core-data/src/entity-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ export default function EntityProvider( { kind, type, id, children } ) {
return <Provider value={ id }>{ children }</Provider>;
}

/**
* Hook that returns the ID for the nearest
* provided entity of the specified type.
*
* @param {string} kind The entity kind.
* @param {string} type The entity type.
*/
export function useEntityId( kind, type ) {
return useContext( getEntity( kind, type ).context );
}

/**
* Hook that returns the value and a setter for the
* specified property of the nearest provided
Expand All @@ -66,11 +77,13 @@ export default function EntityProvider( { kind, type, id, children } ) {
* setter.
*/
export function useEntityProp( kind, type, prop ) {
const id = useContext( getEntity( kind, type ).context );
const id = useEntityId( kind, type );

const value = useSelect(
( select ) => {
const entity = select( 'core' ).getEditedEntityRecord( kind, type, id );
const { getEntityRecord, getEditedEntityRecord } = select( 'core' );
getEntityRecord( kind, type, id ); // Trigger resolver.
const entity = getEditedEntityRecord( kind, type, id );
return entity && entity[ prop ];
},
[ kind, type, id, prop ]
Expand All @@ -88,3 +101,62 @@ export function useEntityProp( kind, type, prop ) {

return [ value, setValue ];
}

/**
* Hook that returns whether the nearest provided
* entity of the specified type is dirty, saving,
* and a function to save it.
*
* The last, optional parameter is for scoping the
* selection to a single property or a list properties.
*
* By default, dirtyness detection and saving considers
* and handles all properties of an entity, but this
* last parameter lets you scope it to a single property
* or a list of properties for each instance of this hook.
*
* @param {string} kind The entity kind.
* @param {string} type The entity type.
* @param {string|[string]} [props] The property name or list of property names.
*/
export function __experimentalUseEntitySaving( kind, type, props ) {
const id = useEntityId( kind, type );

const [ isDirty, isSaving, edits ] = useSelect(
( select ) => {
const { getEntityRecordNonTransientEdits, isSavingEntityRecord } = select(
'core'
);
const _edits = getEntityRecordNonTransientEdits( kind, type, id );
const editKeys = Object.keys( _edits );
return [
props ?
editKeys.some( ( key ) =>
typeof props === 'string' ? key === props : props.includes( key )
) :
editKeys.length > 0,
isSavingEntityRecord( kind, type, id ),
_edits,
];
},
[ kind, type, id, props ]
);

const { saveEntityRecord } = useDispatch( 'core' );
const save = useCallback( () => {
let filteredEdits = edits;
if ( typeof props === 'string' ) {
filteredEdits = { [ props ]: filteredEdits[ props ] };
} else if ( props ) {
filteredEdits = filteredEdits.reduce( ( acc, key ) => {
if ( props.includes( key ) ) {
acc[ key ] = filteredEdits[ key ];
}
return acc;
}, {} );
}
saveEntityRecord( kind, type, { id, ...filteredEdits } );
}, [ kind, type, id, props, edits ] );

return [ isDirty, isSaving, save ];
}
7 changes: 6 additions & 1 deletion packages/core-data/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,9 @@ registerStore( REDUCER_KEY, {
resolvers: { ...resolvers, ...entityResolvers },
} );

export { default as EntityProvider, useEntityProp } from './entity-provider';
export {
default as EntityProvider,
useEntityId,
useEntityProp,
__experimentalUseEntitySaving,
} from './entity-provider';
2 changes: 1 addition & 1 deletion packages/core-data/src/resolvers.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function* getCurrentUser() {
* @param {string} name Entity name.
* @param {number} key Record's key
*/
export function* getEntityRecord( kind, name, key ) {
export function* getEntityRecord( kind, name, key = '' ) {
const entities = yield getKindEntities( kind );
const entity = find( entities, { kind, name } );
if ( ! entity ) {
Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-tests/fixtures/block-transforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ export const EXPECTED_TRANSFORMS = {
'Group',
],
},
'core__site-title': {
availableTransforms: [
'Group',
],
originalBlock: 'Site Title',
},
'core__social-link-amazon': {
availableTransforms: [
'Group',
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/fixtures/blocks/core__site-title.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- wp:site-title /-->
10 changes: 10 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__site-title.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[
{
"clientId": "_clientId_0",
"name": "core/site-title",
"isValid": true,
"attributes": {},
"innerBlocks": [],
"originalContent": ""
}
]
18 changes: 18 additions & 0 deletions packages/e2e-tests/fixtures/blocks/core__site-title.parsed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"blockName": "core/site-title",
"attrs": {},
"innerBlocks": [],
"innerHTML": "",
"innerContent": []
},
{
"blockName": null,
"attrs": {},
"innerBlocks": [],
"innerHTML": "\n",
"innerContent": [
"\n"
]
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!-- wp:site-title /-->
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ describe( 'Block transforms', () => {

const transformStructure = {};
beforeAll( async () => {
await enableExperimentalFeatures( [ '#gutenberg-widget-experiments', '#gutenberg-menu-block' ] );
await enableExperimentalFeatures( [
'#gutenberg-widget-experiments',
'#gutenberg-menu-block',
'#gutenberg-full-site-editing',
] );
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a disable equivalent to make sure we clean up after this test suite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

await createNewPost();

for ( const fileBase of fileBasenames ) {
Expand Down
Loading