From d92a85de4e7d70959486efc46efd63170f5e96f0 Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Tue, 4 Sep 2018 14:25:53 +0100 Subject: [PATCH 1/5] feat: Focus title when empty Fix #3828. --- .../editor/src/components/post-title/index.js | 11 +++++ test/e2e/specs/new-post.test.js | 46 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 test/e2e/specs/new-post.test.js diff --git a/packages/editor/src/components/post-title/index.js b/packages/editor/src/components/post-title/index.js index 8393c2d4d67aee..afd740538749ee 100644 --- a/packages/editor/src/components/post-title/index.js +++ b/packages/editor/src/components/post-title/index.js @@ -1,6 +1,7 @@ /** * External dependencies */ +import React from 'react'; import Textarea from 'react-autosize-textarea'; import classnames from 'classnames'; import { get } from 'lodash'; @@ -36,12 +37,21 @@ class PostTitle extends Component { this.onUnselect = this.onUnselect.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); this.redirectHistory = this.redirectHistory.bind( this ); + this.textareaRef = React.createRef(); this.state = { isSelected: false, }; } + componentDidMount() { + // If there is a post title and it's empty, we should focus the title so + // the user can start typing the title right away. + if ( ! this.props.title || ! this.props.title.length ) { + this.textareaRef.current.textarea.focus(); + } + } + handleFocusOutside() { this.onUnselect(); } @@ -119,6 +129,7 @@ class PostTitle extends Component { onFocus={ this.onSelect } onKeyDown={ this.onKeyDown } onKeyPress={ this.onUnselect } + ref={ this.textareaRef } /> { isSelected && isPostTypeViewable && } diff --git a/test/e2e/specs/new-post.test.js b/test/e2e/specs/new-post.test.js new file mode 100644 index 00000000000000..e33303d50ab2c2 --- /dev/null +++ b/test/e2e/specs/new-post.test.js @@ -0,0 +1,46 @@ +/** + * Internal dependencies + */ +import { + newPost, +} from '../support/utils'; + +describe( 'new post', () => { + beforeEach( async () => { + await newPost(); + } ); + + it( 'should focus the title if the title is empty', async () => { + const activeElementClasses = await page.evaluate( () => { + return Object.values( document.activeElement.classList ); + } ); + const activeElementTagName = await page.evaluate( () => { + return document.activeElement.tagName.toLowerCase(); + } ); + + expect( activeElementClasses ).toContain( 'editor-post-title__input' ); + expect( activeElementTagName ).toEqual( 'textarea' ); + } ); + + it( 'should not focus the title if the title exists', async () => { + // Enter a title for this post. + await page.type( '.editor-post-title__input', 'Here is the title' ); + // Save the post as a draft. + await page.click( '.editor-post-save-draft' ); + await page.waitForSelector( '.editor-post-saved-state.is-saved' ); + // Reload the browser so a post is loaded with a title. + await page.reload(); + + const activeElementClasses = await page.evaluate( () => { + return Object.values( document.activeElement.classList ); + } ); + const activeElementTagName = await page.evaluate( () => { + return document.activeElement.tagName.toLowerCase(); + } ); + + expect( activeElementClasses ).not.toContain( 'editor-post-title__input' ); + // The document body should be the `activeElement`, because nothing is + // focused by default when a post already has a title. + expect( activeElementTagName ).toEqual( 'body' ); + } ); +} ); From e20aa131c2471176122c7e47d8a3cadf170c2c2a Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Tue, 4 Sep 2018 14:26:50 +0100 Subject: [PATCH 2/5] chore: Tweak comments/code clarity --- packages/block-library/src/list/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/list/index.js b/packages/block-library/src/list/index.js index b3880662721f89..503fcdd1f028c2 100644 --- a/packages/block-library/src/list/index.js +++ b/packages/block-library/src/list/index.js @@ -244,12 +244,12 @@ export const settings = { } ); } ); - // this checks for languages that do not typically have square brackets on their keyboards + // Check for languages that do not have square brackets on their keyboards. const lang = window.navigator.browserLanguage || window.navigator.language; - const keyboardHasSqBracket = ! /^(?:fr|nl|sv|ru|de|es|it)/.test( lang ); + const keyboardHasSquareBracket = ! /^(?:fr|nl|sv|ru|de|es|it)/.test( lang ); - if ( keyboardHasSqBracket ) { - // keycode 219 = '[' and keycode 221 = ']' + if ( keyboardHasSquareBracket ) { + // `[` is keycode 219; `]` is keycode 221. editor.shortcuts.add( 'meta+219', 'Decrease indent', 'Outdent' ); editor.shortcuts.add( 'meta+221', 'Increase indent', 'Indent' ); } else { @@ -265,7 +265,7 @@ export const settings = { const { setAttributes } = this.props; const { internalListType } = this.state; if ( internalListType ) { - // only change list types, don't toggle off internal lists + // Only change list types, don't toggle off internal lists. if ( internalListType !== type && this.editor ) { this.editor.execCommand( command ); } From 0f589776c2b8ee9f46051a3ade3a0008544c422b Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Tue, 4 Sep 2018 15:38:34 +0100 Subject: [PATCH 3/5] chore: PR Tweaks --- .../editor/src/components/post-title/index.js | 5 ++- test/e2e/specs/hello.test.js | 31 ------------------- test/e2e/specs/new-post.test.js | 29 ++++++++++++++--- 3 files changed, 26 insertions(+), 39 deletions(-) delete mode 100644 test/e2e/specs/hello.test.js diff --git a/packages/editor/src/components/post-title/index.js b/packages/editor/src/components/post-title/index.js index afd740538749ee..097f0b4afc3d4c 100644 --- a/packages/editor/src/components/post-title/index.js +++ b/packages/editor/src/components/post-title/index.js @@ -1,7 +1,6 @@ /** * External dependencies */ -import React from 'react'; import Textarea from 'react-autosize-textarea'; import classnames from 'classnames'; import { get } from 'lodash'; @@ -10,7 +9,7 @@ import { get } from 'lodash'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Component } from '@wordpress/element'; +import { Component, createRef } from '@wordpress/element'; import { decodeEntities } from '@wordpress/html-entities'; import { ENTER } from '@wordpress/keycodes'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -37,7 +36,7 @@ class PostTitle extends Component { this.onUnselect = this.onUnselect.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); this.redirectHistory = this.redirectHistory.bind( this ); - this.textareaRef = React.createRef(); + this.textareaRef = createRef(); this.state = { isSelected: false, diff --git a/test/e2e/specs/hello.test.js b/test/e2e/specs/hello.test.js deleted file mode 100644 index c43dd7ab313e28..00000000000000 --- a/test/e2e/specs/hello.test.js +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Internal dependencies - */ -import { newPost } from '../support/utils'; - -describe( 'hello', () => { - beforeAll( async () => { - await newPost(); - } ); - - it( 'Should show the New Post Page in Gutenberg', async () => { - expect( page.url() ).toEqual( expect.stringContaining( 'post-new.php' ) ); - // Should display the title. - const title = await page.$( '[placeholder="Add title"]' ); - expect( title ).not.toBeNull(); - // Should display the Preview button. - const postPreviewButton = await page.$( '.editor-post-preview.components-button' ); - expect( postPreviewButton ).not.toBeNull(); - // Should display the Post Formats UI. - const postFormatsUi = await page.$( '.editor-post-format' ); - expect( postFormatsUi ).not.toBeNull(); - } ); - - it( 'Should have no history', async () => { - const undoButton = await page.$( '.editor-history__undo:not( :disabled )' ); - const redoButton = await page.$( '.editor-history__redo:not( :disabled )' ); - - expect( undoButton ).toBeNull(); - expect( redoButton ).toBeNull(); - } ); -} ); diff --git a/test/e2e/specs/new-post.test.js b/test/e2e/specs/new-post.test.js index e33303d50ab2c2..14f87181d1e9dd 100644 --- a/test/e2e/specs/new-post.test.js +++ b/test/e2e/specs/new-post.test.js @@ -1,15 +1,34 @@ /** * Internal dependencies */ -import { - newPost, -} from '../support/utils'; +import { newPost } from '../support/utils'; -describe( 'new post', () => { - beforeEach( async () => { +describe( 'new editor state', () => { + beforeAll( async () => { await newPost(); } ); + it( 'should show the New Post page in Gutenberg', async () => { + expect( page.url() ).toEqual( expect.stringContaining( 'post-new.php' ) ); + // Should display the title. + const title = await page.$( '[placeholder="Add title"]' ); + expect( title ).not.toBeNull(); + // Should display the Preview button. + const postPreviewButton = await page.$( '.editor-post-preview.components-button' ); + expect( postPreviewButton ).not.toBeNull(); + // Should display the Post Formats UI. + const postFormatsUi = await page.$( '.editor-post-format' ); + expect( postFormatsUi ).not.toBeNull(); + } ); + + it( 'should have no history', async () => { + const undoButton = await page.$( '.editor-history__undo:not( :disabled )' ); + const redoButton = await page.$( '.editor-history__redo:not( :disabled )' ); + + expect( undoButton ).toBeNull(); + expect( redoButton ).toBeNull(); + } ); + it( 'should focus the title if the title is empty', async () => { const activeElementClasses = await page.evaluate( () => { return Object.values( document.activeElement.classList ); From ce5e741cc07d4066e2d11d73bde14be2ad244719 Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Thu, 6 Sep 2018 11:25:05 +0100 Subject: [PATCH 4/5] feat: Use autofocus and improve tests --- .../editor/src/components/post-title/index.js | 19 +++++++++--------- test/e2e/specs/new-post.test.js | 20 ++++++++++++++++++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/packages/editor/src/components/post-title/index.js b/packages/editor/src/components/post-title/index.js index 097f0b4afc3d4c..abc84860b8c648 100644 --- a/packages/editor/src/components/post-title/index.js +++ b/packages/editor/src/components/post-title/index.js @@ -43,14 +43,6 @@ class PostTitle extends Component { }; } - componentDidMount() { - // If there is a post title and it's empty, we should focus the title so - // the user can start typing the title right away. - if ( ! this.props.title || ! this.props.title.length ) { - this.textareaRef.current.textarea.focus(); - } - } - handleFocusOutside() { this.onUnselect(); } @@ -128,7 +120,13 @@ class PostTitle extends Component { onFocus={ this.onSelect } onKeyDown={ this.onKeyDown } onKeyPress={ this.onUnselect } - ref={ this.textareaRef } + /* + Only autofocus the title when the post is entirely empty. + This should only happen for a new post, which means we + focus the title on new post so the author can start typing + right away, without needing to click anything. + */ + autofocus={ this.props.isPostEmpty ? 'autofocus' : undefined } /> { isSelected && isPostTypeViewable && } @@ -140,12 +138,13 @@ class PostTitle extends Component { } const applyWithSelect = withSelect( ( select ) => { - const { getEditedPostAttribute, getEditorSettings } = select( 'core/editor' ); + const { getEditedPostAttribute, getEditorSettings, isEditedPostSaveable } = select( 'core/editor' ); const { getPostType } = select( 'core' ); const postType = getPostType( getEditedPostAttribute( 'type' ) ); const { titlePlaceholder, focusMode, hasFixedToolbar } = getEditorSettings(); return { + isPostEmpty: ! isEditedPostSaveable(), title: getEditedPostAttribute( 'title' ), isPostTypeViewable: get( postType, [ 'viewable' ], false ), placeholder: titlePlaceholder, diff --git a/test/e2e/specs/new-post.test.js b/test/e2e/specs/new-post.test.js index 14f87181d1e9dd..98da7080ae6248 100644 --- a/test/e2e/specs/new-post.test.js +++ b/test/e2e/specs/new-post.test.js @@ -30,15 +30,29 @@ describe( 'new editor state', () => { } ); it( 'should focus the title if the title is empty', async () => { + // We need to remove the tips to make sure they aren't clicked/removed + // during our check of the title `textarea`'s focus. + await page.evaluate( () => { + return wp.data.dispatch( 'core/nux' ).disableTips(); + } ); + + // And then reload the page to ensure we get a new page that should + // autofocus the title, without any NUX tips. + await page.reload(); + const activeElementClasses = await page.evaluate( () => { return Object.values( document.activeElement.classList ); } ); const activeElementTagName = await page.evaluate( () => { return document.activeElement.tagName.toLowerCase(); } ); + const titleAutofocus = await page.$eval( '.editor-post-title__input', ( element ) => { + return element.getAttribute( 'autofocus' ); + } ); expect( activeElementClasses ).toContain( 'editor-post-title__input' ); expect( activeElementTagName ).toEqual( 'textarea' ); + expect( titleAutofocus ).toEqual( 'autofocus' ); } ); it( 'should not focus the title if the title exists', async () => { @@ -56,10 +70,14 @@ describe( 'new editor state', () => { const activeElementTagName = await page.evaluate( () => { return document.activeElement.tagName.toLowerCase(); } ); + const titleAutofocus = await page.$eval( '.editor-post-title__input', ( element ) => { + return element.getAttribute( 'autofocus' ); + } ); expect( activeElementClasses ).not.toContain( 'editor-post-title__input' ); - // The document body should be the `activeElement`, because nothing is + // The document `body` should be the `activeElement`, because nothing is // focused by default when a post already has a title. expect( activeElementTagName ).toEqual( 'body' ); + expect( titleAutofocus ).toEqual( null ); } ); } ); From 13b6d4309a1fab7c1ce7ba8bf2a5e9e1057a9561 Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Fri, 7 Sep 2018 21:01:55 +0100 Subject: [PATCH 5/5] chore: Use autoFocus prop and isCleanNewPost selector --- docs/data/data-core-editor.md | 5 +---- .../editor/src/components/post-title/index.js | 21 +++++++++++++------ packages/editor/src/store/selectors.js | 3 --- test/e2e/specs/new-post.test.js | 8 ------- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/docs/data/data-core-editor.md b/docs/data/data-core-editor.md index df8e8f85be10f7..08e61c6fcdda06 100644 --- a/docs/data/data-core-editor.md +++ b/docs/data/data-core-editor.md @@ -54,9 +54,6 @@ Whether unsaved values exist. Returns true if there are no unsaved values for the current edit session and if the currently edited post is new (has never been saved before). -Note: This selector is not currently used by the editor package, but is made -available as an assumed-useful selector for external integrations. - *Parameters* * state: Global application state. @@ -1414,4 +1411,4 @@ Returns an action object used in signalling that the editor settings have been u *Parameters* - * settings: Updated settings \ No newline at end of file + * settings: Updated settings diff --git a/packages/editor/src/components/post-title/index.js b/packages/editor/src/components/post-title/index.js index abc84860b8c648..718784b7bd9f92 100644 --- a/packages/editor/src/components/post-title/index.js +++ b/packages/editor/src/components/post-title/index.js @@ -9,7 +9,7 @@ import { get } from 'lodash'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Component, createRef } from '@wordpress/element'; +import { Component } from '@wordpress/element'; import { decodeEntities } from '@wordpress/html-entities'; import { ENTER } from '@wordpress/keycodes'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -36,7 +36,6 @@ class PostTitle extends Component { this.onUnselect = this.onUnselect.bind( this ); this.onKeyDown = this.onKeyDown.bind( this ); this.redirectHistory = this.redirectHistory.bind( this ); - this.textareaRef = createRef(); this.state = { isSelected: false, @@ -89,7 +88,15 @@ class PostTitle extends Component { } render() { - const { title, placeholder, instanceId, isPostTypeViewable, isFocusMode, hasFixedToolbar } = this.props; + const { + hasFixedToolbar, + isCleanNewPost, + isFocusMode, + isPostTypeViewable, + instanceId, + placeholder, + title, + } = this.props; const { isSelected } = this.state; const className = classnames( 'editor-post-title__block', { 'is-selected': isSelected, @@ -126,7 +133,9 @@ class PostTitle extends Component { focus the title on new post so the author can start typing right away, without needing to click anything. */ - autofocus={ this.props.isPostEmpty ? 'autofocus' : undefined } + /* eslint-disable jsx-a11y/no-autofocus */ + autoFocus={ isCleanNewPost } + /* eslint-enable jsx-a11y/no-autofocus */ /> { isSelected && isPostTypeViewable && } @@ -138,13 +147,13 @@ class PostTitle extends Component { } const applyWithSelect = withSelect( ( select ) => { - const { getEditedPostAttribute, getEditorSettings, isEditedPostSaveable } = select( 'core/editor' ); + const { getEditedPostAttribute, getEditorSettings, isCleanNewPost } = select( 'core/editor' ); const { getPostType } = select( 'core' ); const postType = getPostType( getEditedPostAttribute( 'type' ) ); const { titlePlaceholder, focusMode, hasFixedToolbar } = getEditorSettings(); return { - isPostEmpty: ! isEditedPostSaveable(), + isCleanNewPost: isCleanNewPost(), title: getEditedPostAttribute( 'title' ), isPostTypeViewable: get( postType, [ 'viewable' ], false ), placeholder: titlePlaceholder, diff --git a/packages/editor/src/store/selectors.js b/packages/editor/src/store/selectors.js index c16b3dabd1632e..823fbe522f3638 100644 --- a/packages/editor/src/store/selectors.js +++ b/packages/editor/src/store/selectors.js @@ -102,9 +102,6 @@ export function isEditedPostDirty( state ) { * Returns true if there are no unsaved values for the current edit session and * if the currently edited post is new (has never been saved before). * - * Note: This selector is not currently used by the editor package, but is made - * available as an assumed-useful selector for external integrations. - * * @param {Object} state Global application state. * * @return {boolean} Whether new post and unsaved values exist. diff --git a/test/e2e/specs/new-post.test.js b/test/e2e/specs/new-post.test.js index 98da7080ae6248..61970a83f95361 100644 --- a/test/e2e/specs/new-post.test.js +++ b/test/e2e/specs/new-post.test.js @@ -46,13 +46,9 @@ describe( 'new editor state', () => { const activeElementTagName = await page.evaluate( () => { return document.activeElement.tagName.toLowerCase(); } ); - const titleAutofocus = await page.$eval( '.editor-post-title__input', ( element ) => { - return element.getAttribute( 'autofocus' ); - } ); expect( activeElementClasses ).toContain( 'editor-post-title__input' ); expect( activeElementTagName ).toEqual( 'textarea' ); - expect( titleAutofocus ).toEqual( 'autofocus' ); } ); it( 'should not focus the title if the title exists', async () => { @@ -70,14 +66,10 @@ describe( 'new editor state', () => { const activeElementTagName = await page.evaluate( () => { return document.activeElement.tagName.toLowerCase(); } ); - const titleAutofocus = await page.$eval( '.editor-post-title__input', ( element ) => { - return element.getAttribute( 'autofocus' ); - } ); expect( activeElementClasses ).not.toContain( 'editor-post-title__input' ); // The document `body` should be the `activeElement`, because nothing is // focused by default when a post already has a title. expect( activeElementTagName ).toEqual( 'body' ); - expect( titleAutofocus ).toEqual( null ); } ); } );