From f69af55567f4d4a77313b2b0e57bceaccd99bbbb Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 26 Sep 2019 20:43:26 +0300 Subject: [PATCH 01/10] RichText: return focus after setting selection --- packages/rich-text/src/to-dom.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js index 9d7aed3cfdfd0..3864bc802491d 100644 --- a/packages/rich-text/src/to-dom.js +++ b/packages/rich-text/src/to-dom.js @@ -278,10 +278,7 @@ export function applySelection( { startPath, endPath }, current ) { range.setStart( startContainer, startOffset ); range.setEnd( endContainer, endOffset ); - // Set back focus if focus is lost. - if ( ownerDocument.activeElement !== current ) { - current.focus(); - } + const { activeElement } = ownerDocument; if ( selection.rangeCount > 0 ) { // If the to be added range and the live range are the same, there's no @@ -294,4 +291,5 @@ export function applySelection( { startPath, endPath }, current ) { } selection.addRange( range ); + activeElement.focus(); } From 1ab4854cb7949be162ea965c18a7ef630a2173ed Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 26 Sep 2019 21:24:54 +0300 Subject: [PATCH 02/10] Fix rich text e2e tests --- packages/e2e-tests/specs/rich-text.test.js | 4 ++++ packages/rich-text/src/component/index.js | 14 -------------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/e2e-tests/specs/rich-text.test.js b/packages/e2e-tests/specs/rich-text.test.js index 13acc998e3946..238593345b79b 100644 --- a/packages/e2e-tests/specs/rich-text.test.js +++ b/packages/e2e-tests/specs/rich-text.test.js @@ -7,6 +7,7 @@ import { insertBlock, clickBlockAppender, pressKeyWithModifier, + pressKeyTimes, } from '@wordpress/e2e-test-utils'; describe( 'RichText', () => { @@ -83,10 +84,12 @@ describe( 'RichText', () => { await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); + await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( 'bold' ); await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); + await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( '.' ); expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -238,6 +241,7 @@ describe( 'RichText', () => { it( 'should handle Home and End keys', async () => { await page.keyboard.press( 'Enter' ); + await page.evaluate( () => new Promise( window.requestAnimationFrame ) ); await pressKeyWithModifier( 'primary', 'b' ); await page.keyboard.type( '12' ); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index a4f4fd8f22005..319bd1028add1 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -282,20 +282,6 @@ class RichText extends Component { this.recalculateBoundaryStyle(); - // We know for certain that on focus, the old selection is invalid. It - // will be recalculated on the next mouseup, keyup, or touchend event. - const index = undefined; - const activeFormats = undefined; - - this.record = { - ...this.record, - start: index, - end: index, - activeFormats, - }; - this.props.onSelectionChange( index, index ); - this.setState( { activeFormats } ); - // Update selection as soon as possible, which is at the next animation // frame. The event listener for selection changes may be added too late // at this point, but this focus event is still too early to calculate From 483735b70a11dcf446ae20bed262f228b61c9f94 Mon Sep 17 00:00:00 2001 From: iseulde Date: Thu, 26 Sep 2019 21:30:48 +0300 Subject: [PATCH 03/10] Fix list e2e tests --- packages/e2e-tests/specs/blocks/list.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/e2e-tests/specs/blocks/list.test.js b/packages/e2e-tests/specs/blocks/list.test.js index 5e04dbbcfeba0..b3a9c302a038f 100644 --- a/packages/e2e-tests/specs/blocks/list.test.js +++ b/packages/e2e-tests/specs/blocks/list.test.js @@ -182,6 +182,7 @@ describe( 'List', () => { await page.keyboard.type( 'one' ); await page.keyboard.press( 'Enter' ); await clickBlockToolbarButton( 'Indent list item' ); + await pressKeyTimes( 'Tab', 6 ); await page.keyboard.type( 'two' ); await transformBlockTo( 'Paragraph' ); @@ -264,6 +265,7 @@ describe( 'List', () => { await page.keyboard.type( 'one' ); await page.keyboard.press( 'Enter' ); await clickBlockToolbarButton( 'Indent list item' ); + await pressKeyTimes( 'Tab', 6 ); await page.keyboard.type( 'two' ); await page.keyboard.press( 'Enter' ); await page.keyboard.type( 'three' ); From 92ed8062e31094859f164d59e52ae6a04ed15aeb Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 10:30:01 +0300 Subject: [PATCH 04/10] Revert "Fix rich text e2e tests" This reverts commit 1ab4854cb7949be162ea965c18a7ef630a2173ed. --- packages/e2e-tests/specs/rich-text.test.js | 4 ---- packages/rich-text/src/component/index.js | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/rich-text.test.js b/packages/e2e-tests/specs/rich-text.test.js index 238593345b79b..13acc998e3946 100644 --- a/packages/e2e-tests/specs/rich-text.test.js +++ b/packages/e2e-tests/specs/rich-text.test.js @@ -7,7 +7,6 @@ import { insertBlock, clickBlockAppender, pressKeyWithModifier, - pressKeyTimes, } from '@wordpress/e2e-test-utils'; describe( 'RichText', () => { @@ -84,12 +83,10 @@ describe( 'RichText', () => { await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); - await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( 'bold' ); await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); - await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( '.' ); expect( await getEditedPostContent() ).toMatchSnapshot(); @@ -241,7 +238,6 @@ describe( 'RichText', () => { it( 'should handle Home and End keys', async () => { await page.keyboard.press( 'Enter' ); - await page.evaluate( () => new Promise( window.requestAnimationFrame ) ); await pressKeyWithModifier( 'primary', 'b' ); await page.keyboard.type( '12' ); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 319bd1028add1..a4f4fd8f22005 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -282,6 +282,20 @@ class RichText extends Component { this.recalculateBoundaryStyle(); + // We know for certain that on focus, the old selection is invalid. It + // will be recalculated on the next mouseup, keyup, or touchend event. + const index = undefined; + const activeFormats = undefined; + + this.record = { + ...this.record, + start: index, + end: index, + activeFormats, + }; + this.props.onSelectionChange( index, index ); + this.setState( { activeFormats } ); + // Update selection as soon as possible, which is at the next animation // frame. The event listener for selection changes may be added too late // at this point, but this focus event is still too early to calculate From 7a1b8141559e84430e154d1d6f36f08d28d82549 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 10:36:09 +0300 Subject: [PATCH 05/10] Fix some rich text e2e tests --- packages/e2e-tests/specs/rich-text.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/e2e-tests/specs/rich-text.test.js b/packages/e2e-tests/specs/rich-text.test.js index 13acc998e3946..2d91edf8f9b80 100644 --- a/packages/e2e-tests/specs/rich-text.test.js +++ b/packages/e2e-tests/specs/rich-text.test.js @@ -7,6 +7,7 @@ import { insertBlock, clickBlockAppender, pressKeyWithModifier, + pressKeyTimes, } from '@wordpress/e2e-test-utils'; describe( 'RichText', () => { @@ -83,10 +84,12 @@ describe( 'RichText', () => { await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); + await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( 'bold' ); await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); + await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( '.' ); expect( await getEditedPostContent() ).toMatchSnapshot(); From f31d816969297510e1b6e48befa1747bbf0f446c Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 10:45:50 +0300 Subject: [PATCH 06/10] Only reset selection on focus if previously not selected --- packages/rich-text/src/component/index.js | 28 ++++++++++++----------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index a4f4fd8f22005..deb8f3017ebdd 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -282,19 +282,21 @@ class RichText extends Component { this.recalculateBoundaryStyle(); - // We know for certain that on focus, the old selection is invalid. It - // will be recalculated on the next mouseup, keyup, or touchend event. - const index = undefined; - const activeFormats = undefined; - - this.record = { - ...this.record, - start: index, - end: index, - activeFormats, - }; - this.props.onSelectionChange( index, index ); - this.setState( { activeFormats } ); + if ( ! this.props.__unstableIsSelected ) { + // We know for certain that on focus, the old selection is invalid. It + // will be recalculated on the next mouseup, keyup, or touchend event. + const index = undefined; + const activeFormats = undefined; + + this.record = { + ...this.record, + start: index, + end: index, + activeFormats, + }; + this.props.onSelectionChange( index, index ); + this.setState( { activeFormats } ); + } // Update selection as soon as possible, which is at the next animation // frame. The event listener for selection changes may be added too late From fc138bf31f2c1688a93689be2cdbf61cde95e387 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 10:57:33 +0300 Subject: [PATCH 07/10] Fix annotation e2e tests --- packages/e2e-tests/specs/plugins/annotations.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/e2e-tests/specs/plugins/annotations.test.js b/packages/e2e-tests/specs/plugins/annotations.test.js index 5bab673a41d80..4425e8c7e8f53 100644 --- a/packages/e2e-tests/specs/plugins/annotations.test.js +++ b/packages/e2e-tests/specs/plugins/annotations.test.js @@ -49,6 +49,7 @@ describe( 'Using Plugins API', () => { // Click add annotation button. const addAnnotationButton = ( await page.$x( "//button[contains(text(), 'Add annotation')]" ) )[ 0 ]; await addAnnotationButton.click(); + await page.evaluate( () => document.querySelector( '[contenteditable]' ).focus() ); } /** @@ -60,6 +61,7 @@ describe( 'Using Plugins API', () => { // Click remove annotations button. const addAnnotationButton = ( await page.$x( "//button[contains(text(), 'Remove annotations')]" ) )[ 0 ]; await addAnnotationButton.click(); + await page.evaluate( () => document.querySelector( '[contenteditable]' ).focus() ); } /** From 032d0ddd374eeb1a5b92318b42f2fa817873a422 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 11:06:08 +0300 Subject: [PATCH 08/10] Allow focus to be set through onChange --- packages/format-library/src/link/inline.js | 4 ++-- packages/rich-text/src/component/index.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 815a8fcaae1d3..37f3db42464c0 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -149,9 +149,9 @@ class InlineLinkUI extends Component { if ( isCollapsed( value ) && ! isActive ) { const toInsert = applyFormat( create( { text: url } ), format, 0, url.length ); - onChange( insert( value, toInsert ) ); + onChange( insert( value, toInsert ), { focus: true } ); } else { - onChange( applyFormat( value, format ) ); + onChange( applyFormat( value, format ), { focus: true } ); } this.resetState(); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index deb8f3017ebdd..5d699e95f380c 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -527,7 +527,11 @@ class RichText extends Component { * @param {boolean} $2.withoutHistory If true, no undo level will be * created. */ - onChange( record, { withoutHistory } = {} ) { + onChange( record, { withoutHistory, focus } = {} ) { + if ( focus ) { + this.editableRef.focus(); + } + this.applyRecord( record ); const { start, end, activeFormats = [] } = record; From 9de752097c9d2419d9e6412bb21cde7199fa45df Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 13:35:43 +0300 Subject: [PATCH 09/10] Set focus back to rich text after mere button click --- .../components/rich-text/toolbar-button.js | 24 ++++++++++++++++++- .../__snapshots__/rich-text.test.js.snap | 14 +++++++---- packages/e2e-tests/specs/rich-text.test.js | 24 ++++++++++++++++++- packages/rich-text/src/component/index.js | 2 ++ 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/toolbar-button.js b/packages/block-editor/src/components/rich-text/toolbar-button.js index 2febd82e717c5..8c1adafca1550 100644 --- a/packages/block-editor/src/components/rich-text/toolbar-button.js +++ b/packages/block-editor/src/components/rich-text/toolbar-button.js @@ -3,8 +3,17 @@ */ import { Fill, ToolbarButton } from '@wordpress/components'; import { displayShortcut } from '@wordpress/keycodes'; +import { useRef } from '@wordpress/element'; + +export function RichTextToolbarButton( { + name, + shortcutType, + shortcutCharacter, + onClick, + ...props +} ) { + const activeElement = useRef( null ); -export function RichTextToolbarButton( { name, shortcutType, shortcutCharacter, ...props } ) { let shortcut; let fillName = 'RichText.ToolbarControls'; @@ -21,6 +30,19 @@ export function RichTextToolbarButton( { name, shortcutType, shortcutCharacter, { + activeElement.current = document.activeElement; + }, + } } + onClick={ () => { + onClick( ...arguments ); + + if ( activeElement.current ) { + activeElement.current.focus(); + activeElement.current = null; + } + } } /> ); diff --git a/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap b/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap index 78b78101a59a5..c237c2049ef67 100644 --- a/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap +++ b/packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap @@ -48,6 +48,12 @@ exports[`RichText should not lose selection direction 1`] = ` " `; +exports[`RichText should not return focus when tabbing to formatting button 1`] = ` +" +

Some bold.

+" +`; + exports[`RichText should not undo backtick transform with backspace after selection change 1`] = `""`; exports[`RichText should not undo backtick transform with backspace after typing 1`] = `""`; @@ -76,14 +82,14 @@ exports[`RichText should transform backtick to code 2`] = ` " `; -exports[`RichText should update internal selection after fresh focus 1`] = ` +exports[`RichText should undo backtick transform with backspace 1`] = ` " -

12

+

\`a\`

" `; -exports[`RichText should undo backtick transform with backspace 1`] = ` +exports[`RichText should update internal selection after fresh focus 1`] = ` " -

\`a\`

+

12

" `; diff --git a/packages/e2e-tests/specs/rich-text.test.js b/packages/e2e-tests/specs/rich-text.test.js index 2d91edf8f9b80..19db0e2a8e34a 100644 --- a/packages/e2e-tests/specs/rich-text.test.js +++ b/packages/e2e-tests/specs/rich-text.test.js @@ -84,11 +84,33 @@ describe( 'RichText', () => { await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); - await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( 'bold' ); await page.mouse.move( 0, 0 ); await page.mouse.move( 10, 10 ); await page.click( '[aria-label="Bold"]' ); + await page.keyboard.type( '.' ); + + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); + + it( 'should not return focus when tabbing to formatting button', async () => { + await clickBlockAppender(); + await page.keyboard.type( 'Some ' ); + await pressKeyWithModifier( 'alt', 'F10' ); + // Wait for button to receive focus. + await page.waitForFunction( () => + document.activeElement.nodeName === 'BUTTON' + ); + await pressKeyTimes( 'Tab', 2 ); + await page.keyboard.press( 'Space' ); + await pressKeyTimes( 'Tab', 5 ); + await page.keyboard.type( 'bold' ); + await pressKeyWithModifier( 'alt', 'F10' ); + await page.waitForFunction( () => + document.activeElement.nodeName === 'BUTTON' + ); + await pressKeyTimes( 'Tab', 2 ); + await page.keyboard.press( 'Space' ); await pressKeyTimes( 'Tab', 5 ); await page.keyboard.type( '.' ); diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 5d699e95f380c..395fa9d72f86c 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -526,6 +526,8 @@ class RichText extends Component { * @param {Object} $2 Named options. * @param {boolean} $2.withoutHistory If true, no undo level will be * created. + * @param {boolean} $2.focus If true, the rich text element will + * receive focus. */ onChange( record, { withoutHistory, focus } = {} ) { if ( focus ) { From 8468062aab7c459f4d9def9a44d2c16c9d9943c7 Mon Sep 17 00:00:00 2001 From: iseulde Date: Fri, 27 Sep 2019 15:37:46 +0300 Subject: [PATCH 10/10] Ignore focus when applying selection --- packages/rich-text/src/component/index.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js index 395fa9d72f86c..3a160df4674d1 100644 --- a/packages/rich-text/src/component/index.js +++ b/packages/rich-text/src/component/index.js @@ -170,6 +170,8 @@ class RichText extends Component { applyRecord( record, { domOnly } = {} ) { const { __unstableMultilineTag: multilineTag } = this.props; + this.ignoreFocus = true; + apply( { value: record, current: this.editableRef, @@ -179,6 +181,8 @@ class RichText extends Component { __unstableDomOnly: domOnly, placeholder: this.props.placeholder, } ); + + this.ignoreFocus = false; } /** @@ -276,6 +280,10 @@ class RichText extends Component { onFocus() { const { unstableOnFocus } = this.props; + if ( this.ignoreFocus ) { + return; + } + if ( unstableOnFocus ) { unstableOnFocus(); }