FSE: Add a site title block. #33761

jeraldjuice · 2019-06-07T19:17:15Z

Changes proposed in this Pull Request

A basic site title block for use with Full Site Editing

Testing instructions

Open up your FSE environment.
Add a Site Title block in a FSE template.
Notice that the Site Title block reflects the site's title.
Update the Site Title and 'force dirty' the state by editing something else in the post contents.
Click "Publish..."
Refresh the page and notice that the site title has been updated.

Fixes #33106

matticbot · 2019-06-07T19:17:16Z

Test live: https://calypso.live/?branch=add/fse-site-title-block

matticbot · 2019-06-07T19:26:36Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

jeraldjuice · 2019-06-07T19:27:04Z

Outstanding thoughts, tasks, feelings:

❓Is hooking into the core editor state the best way of performing the site title update on publish? Does it make sense to use a different method? Is the existing save function in the registerBlockType (site-title/index.js) a good hook instead?
❓ Will we need to handle non-publish save states in the future for the site title?

⬜ Styling needs to be re-worked to ensure it is consistent and properly applied.
⬜ Some refactoring needed.
⬜ More exploratory testing also needed.
⬜ Needs more sanitizing/validation for API calls.

💡Way down the line, we can genericize and HOC this functionality, if we go with a method similar to this.

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js

kwight · 2019-06-07T19:28:35Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js

+	</svg>
+);
+
+if ( 'wp_template' === fullSiteEditing.editorPostType ) {


We can leave this check out for now for easier testing, while we sort out template plumbing.

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.php

kwight · 2019-06-07T19:51:58Z

Sweet! I can load the saved title, replace the title, then update the page, and have the new title display on the front-end 🎉

A few observations:

It tripped me up that I couldn't directly edit the title text; I didn't realize the text was a placeholder. It might make more sense to load the actual text into the element.
Site titles should only be a single string of text; maybe an input tag instead of a textarea tag would make more sense? (This would align with what we use in the Customizer when editing the title.)
Since this is a visual preview, it can use some styling to fully match the front-end output (font size, color, etc – there also seems to be a blue border on focus, rather than the grey border other blocks seem to have).
Since the block has no attributes, any changes aren't persisted on drafts or autosaves – I think this is fine though (I don't expect users to be disappointed should the situation arise).

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

gwwar · 2019-06-07T20:10:28Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+
+		apiFetch( { path: '/wp/v2/settings' } ).then( ( { title } ) => {
+			this.setState( { initialTitle: title } );
+		} );


Hmm, I do wonder how often we should query. This is probably okay for now, but we might want to follow up on this.

Data here may get stale in a long editing session. We previously had problems with state, in Gutenlypso when switching sites, but it isn't a problem in this implementation.

For context in Calypso, we normally use a data pattern of a query component. example: https://github.com/Automattic/wp-calypso/blob/master/client/components/data/query-comment/index.jsx

The idea is make a request on mount, and any time related props change like site id.

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

gwwar · 2019-06-07T20:18:48Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+}
+
+export default withSelect( select => {
+	const { isSavingPost, isPublishingPost, isAutosavingPost, isCurrentPostPublished } = select(


I might poke around in core/editor state to see if there's an action we can listen to for the primary button click.

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

mmtr · 2019-06-13T05:53:15Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+					<PlainText
+						className={ className }
+						value={ title }
+						onChange={ this.setSiteTitle }


We need to set a block attribute here with setAttribute in order to let the editor know that the content has changed. Otherwise the "Update" button will remain disabled until we change the content of other blocks:

I've got this working with attributes instead of local state for the value tracking. I admit I'm weirded out by the code view though; I really want whatever I see in the code view to coincide with what would get saved. If we never save attributes, should a user ever see them in the code view?

Actually, the code view is showing the content that is saved: nothing 🙂(save: () => null).

Remember that this is a dynamic block, so it is rendered by the server on the published view. Think of them like shortcodes. Wouldn't a shortcode allowing to override the site description look similar to that block syntax?

I admit the attribute is not strictly required when the post is saved (the back-end rendering will ignore the attribute and will retrieve always the description from the site data), but I don't know other way of letting the editor know there are changes that need to be saved.

Let's tackle this next sprint in #34004

mmtr

Awesome! This is working great!

Left a few minor suggestions to fix some linter issues and another one regarding the custom classes support.

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.php

mmtr

LGTM!

Noted we're importing stuff from undeclared dependencies, so feel free to land this after adding them to the full-site-editing's package.json file.

mmtr · 2019-06-18T01:49:24Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+ */
+import { __ } from '@wordpress/i18n';
+import { withNotices } from '@wordpress/components';
+import { PlainText } from '@wordpress/block-editor';


@wordpress/block-editor should be declared as dependency of the full-site-editing app.

Let's stick with editor for better support until it's actually removed (which already has its dependency declared): #33720 (comment)

Suggested change

import { PlainText } from '@wordpress/block-editor';

import { PlainText } from '@wordpress/editor';

Let's use the editor package until support is dropped, block-editor was only added recently.

mmtr · 2019-06-18T01:58:51Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+/**
+ * External dependencies
+ */
+import React, { Component } from 'react';


react should be declared as dependency of the full-site-editing app.

Actually, seems that import React is not really needed. And we can import Component from @wordpress/element, making the react dependency unnecessary.

Highly advised to use the @wordpress/element abstraction as it gives us better ways to handle compatibility cases (including mobile native) and avoids leaking too many assumptions.

mmtr · 2019-06-18T02:02:06Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

@@ -0,0 +1,113 @@
+/* eslint-disable wpcalypso/jsx-classname-namespace */


Do we really need to suppress this rule? I'm not getting any linter error if I remove this line.

mmtr · 2019-06-18T02:02:38Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+import { withSelect } from '@wordpress/data';
+import { compose } from '@wordpress/compose';
+
+/**


We can remove this comment since we're not importing any internal dep :)

mmtr · 2019-06-18T02:06:13Z

This also needs a rebase before merging.

mmtr · 2019-06-18T06:32:04Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

+		if ( userInitiatedPublish ) {
+			apiFetch( { path: '/wp/v2/settings', method: 'POST', data: { title } } )
+				.then( () => this.resetTitle() )
+				.catch( ( { message } ) => noticeOperations.createErrorNotice( message ) );


Note that we need to render noticesUI in the component in order to make this notice visible. See https://github.com/Automattic/wp-calypso/pull/33720/files#diff-ba2bdc4ff67c75de1773a2ef5e83c5eeR67

Ahhh thanks for pointing that out!

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js

Nineteen styling.

jeraldjuice added [Feature] Post/Page Editor The editor for editing posts and pages. [Goal] Full Site Editing labels Jun 7, 2019

jeraldjuice requested a review from a team June 7, 2019 19:17

jeraldjuice self-assigned this Jun 7, 2019

kwight reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js Outdated Show resolved Hide resolved

kwight reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js Outdated Show resolved Hide resolved

kwight reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.php Outdated Show resolved Hide resolved

gwwar added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] In Progress labels Jun 7, 2019

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 7, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

gwwar reviewed Jun 10, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

kwight reviewed Jun 11, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

jeraldjuice requested a review from a team as a code owner June 12, 2019 17:20

jeraldjuice removed the [Status] In Progress label Jun 12, 2019

mmtr reviewed Jun 13, 2019

View reviewed changes

mmtr mentioned this pull request Jun 14, 2019

FSE: Add a site description block. #33720

Merged

mmtr reviewed Jun 17, 2019

View reviewed changes

jeraldjuice requested a review from a team as a code owner June 17, 2019 16:40

mmtr approved these changes Jun 18, 2019

View reviewed changes

mmtr reviewed Jun 18, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/edit.js Outdated Show resolved Hide resolved

jeraldjuice and others added 5 commits June 18, 2019 17:49

FSE: Add a site title block.

dfa596b

Oops, fix icon.

f80d174

Updates in response to feedback

1345991

Use user-def className + code style tweaks

d7bb239

Updates

022317c

jeraldjuice force-pushed the add/fse-site-title-block branch from 9300fbe to 022317c Compare June 18, 2019 21:50

kwight reviewed Jun 19, 2019

View reviewed changes

apps/full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-title/index.js Show resolved Hide resolved

Align operations with the site description block. Add basic Twenty

80d8953

Nineteen styling.

kwight changed the title ~~[WIP] FSE: Add a site title block.~~ FSE: Add a site title block. Jun 19, 2019

kwight merged commit e4136ad into master Jun 19, 2019

kwight deleted the add/fse-site-title-block branch June 19, 2019 22:10

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 19, 2019

paaljoachim mentioned this pull request Jun 22, 2019

Add Block: Title WordPress/gutenberg#11553

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FSE: Add a site title block. #33761

FSE: Add a site title block. #33761

jeraldjuice commented Jun 7, 2019

matticbot commented Jun 7, 2019

matticbot commented Jun 7, 2019

jeraldjuice commented Jun 7, 2019 •

edited

Loading

kwight Jun 7, 2019

kwight commented Jun 7, 2019

gwwar Jun 7, 2019

gwwar Jun 7, 2019

mmtr Jun 13, 2019

kwight Jun 13, 2019 •

edited

Loading

mmtr Jun 14, 2019

gwwar Jun 14, 2019

mmtr left a comment

mmtr left a comment

mmtr Jun 18, 2019

kwight Jun 18, 2019 •

edited

Loading

gwwar Jun 18, 2019

mmtr Jun 18, 2019

mmtr Jun 18, 2019

mtias Jun 18, 2019 •

edited

Loading

mmtr Jun 18, 2019

mmtr Jun 18, 2019

mmtr commented Jun 18, 2019

mmtr Jun 18, 2019

jeraldjuice Jun 18, 2019

	import { PlainText } from '@wordpress/block-editor';
	import { PlainText } from '@wordpress/editor';

		@@ -0,0 +1,113 @@
		/* eslint-disable wpcalypso/jsx-classname-namespace */

FSE: Add a site title block. #33761

FSE: Add a site title block. #33761

Conversation

jeraldjuice commented Jun 7, 2019

Changes proposed in this Pull Request

Testing instructions

matticbot commented Jun 7, 2019

matticbot commented Jun 7, 2019

jeraldjuice commented Jun 7, 2019 • edited Loading

Choose a reason for hiding this comment

kwight commented Jun 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwight Jun 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmtr left a comment

Choose a reason for hiding this comment

mmtr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwight Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtias Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmtr commented Jun 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeraldjuice commented Jun 7, 2019 •

edited

Loading

kwight Jun 13, 2019 •

edited

Loading

kwight Jun 18, 2019 •

edited

Loading

mtias Jun 18, 2019 •

edited

Loading