RichText: don't set focus when applying format (#19536)

ellatrix · web-flow · commit 2380f2d81b70 · 2020-01-13T11:39:21.000+01:00
* Return focus to original element

* Fix list e2e tests

* Fix annotation e2e tests

* Fix some rich text e2e tests

* Fix tests

* Focus after link

* Add onFocus

* Corrections
diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js
@@ -411,9 +411,9 @@ class RichTextWrapper extends Component {
 				start={ start }
 				reversed={ reversed }
 			>
-				{ ( { isSelected, value, onChange, Editable } ) =>
+				{ ( { isSelected, value, onChange, onFocus, Editable } ) =>
 					<>
-						{ children && children( { value, onChange } ) }
+						{ children && children( { value, onChange, onFocus } ) }
 						{ isSelected && hasFormats && ( <FormatToolbarContainer inline={ inlineToolbar } anchorRef={ forwardedRef.current } /> ) }
 						{ isSelected && <RemoveBrowserShortcuts /> }
 						<Autocomplete
diff --git a/packages/block-library/src/list/edit.js b/packages/block-library/src/list/edit.js
@@ -38,7 +38,7 @@ export default function ListEdit( {
 	const { ordered, values, type, reversed, start } = attributes;
 	const tagName = ordered ? 'ol' : 'ul';
 
-	const controls = ( { value, onChange } ) => (
+	const controls = ( { value, onChange, onFocus } ) => (
 		<>
 			{ ( isSelected && <>
 				<RichTextShortcut
@@ -79,6 +79,7 @@ export default function ListEdit( {
 							isActive: isActiveListType( value, 'ul', tagName ),
 							onClick() {
 								onChange( changeListType( value, { type: 'ul' } ) );
+								onFocus();
 
 								if ( isListRootSelected( value ) ) {
 									setAttributes( { ordered: false } );
@@ -91,6 +92,7 @@ export default function ListEdit( {
 							isActive: isActiveListType( value, 'ol', tagName ),
 							onClick() {
 								onChange( changeListType( value, { type: 'ol' } ) );
+								onFocus();
 
 								if ( isListRootSelected( value ) ) {
 									setAttributes( { ordered: true } );
@@ -104,6 +106,7 @@ export default function ListEdit( {
 							isDisabled: ! canOutdentListItems( value ),
 							onClick() {
 								onChange( outdentListItems( value ) );
+								onFocus();
 							},
 						},
 						{
@@ -113,6 +116,7 @@ export default function ListEdit( {
 							isDisabled: ! canIndentListItems( value ),
 							onClick() {
 								onChange( indentListItems( value, { type: tagName } ) );
+								onFocus();
 							},
 						},
 					] }
diff --git a/packages/e2e-tests/specs/editor/plugins/annotations.test.js b/packages/e2e-tests/specs/editor/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() );
 	}
 
 	/**
diff --git a/packages/format-library/src/bold/index.js b/packages/format-library/src/bold/index.js
@@ -13,8 +13,15 @@ export const bold = {
 	title,
 	tagName: 'strong',
 	className: null,
-	edit( { isActive, value, onChange } ) {
-		const onToggle = () => onChange( toggleFormat( value, { type: name } ) );
+	edit( { isActive, value, onChange, onFocus } ) {
+		function onToggle() {
+			onChange( toggleFormat( value, { type: name } ) );
+		}
+
+		function onClick() {
+			onToggle();
+			onFocus();
+		}
 
 		return (
 			<>
@@ -27,7 +34,7 @@ export const bold = {
 					name="bold"
 					icon="editor-bold"
 					title={ title }
-					onClick={ onToggle }
+					onClick={ onClick }
 					isActive={ isActive }
 					shortcutType="primary"
 					shortcutCharacter="b"
diff --git a/packages/format-library/src/code/index.js b/packages/format-library/src/code/index.js
@@ -43,14 +43,17 @@ export const code = {
 
 		return value;
 	},
-	edit( { value, onChange, isActive } ) {
-		const onToggle = () => onChange( toggleFormat( value, { type: name } ) );
+	edit( { value, onChange, onFocus, isActive } ) {
+		function onClick() {
+			onChange( toggleFormat( value, { type: name } ) );
+			onFocus();
+		}
 
 		return (
 			<RichTextToolbarButton
 				icon="editor-code"
 				title={ title }
-				onClick={ onToggle }
+				onClick={ onClick }
 				isActive={ isActive }
 			/>
 		);
diff --git a/packages/format-library/src/image/index.js b/packages/format-library/src/image/index.js
@@ -85,7 +85,7 @@ export const image = {
 		}
 
 		render() {
-			const { value, onChange, isObjectActive, activeObjectAttributes } = this.props;
+			const { value, onChange, onFocus, isObjectActive, activeObjectAttributes } = this.props;
 
 			return (
 				<MediaUploadCheck>
@@ -108,6 +108,7 @@ export const image = {
 									alt,
 								},
 							} ) );
+							onFocus();
 						} }
 						onClose={ this.closeModal }
 						render={ ( { open } ) => {
diff --git a/packages/format-library/src/italic/index.js b/packages/format-library/src/italic/index.js
@@ -13,8 +13,15 @@ export const italic = {
 	title,
 	tagName: 'em',
 	className: null,
-	edit( { isActive, value, onChange } ) {
-		const onToggle = () => onChange( toggleFormat( value, { type: name } ) );
+	edit( { isActive, value, onChange, onFocus } ) {
+		function onToggle() {
+			onChange( toggleFormat( value, { type: name } ) );
+		}
+
+		function onClick() {
+			onToggle();
+			onFocus();
+		}
 
 		return (
 			<>
@@ -27,7 +34,7 @@ export const italic = {
 					name="italic"
 					icon="editor-italic"
 					title={ title }
-					onClick={ onToggle }
+					onClick={ onClick }
 					isActive={ isActive }
 					shortcutType="primary"
 					shortcutCharacter="i"
diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js
@@ -91,7 +91,7 @@ export const link = {
 		}
 
 		render() {
-			const { isActive, activeAttributes, value, onChange } = this.props;
+			const { isActive, activeAttributes, value, onChange, onFocus } = this.props;
 
 			return (
 				<>
@@ -131,6 +131,7 @@ export const link = {
 						activeAttributes={ activeAttributes }
 						value={ value }
 						onChange={ onChange }
+						onFocus={ onFocus }
 					/>
 				</>
 			);
diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js
@@ -135,7 +135,7 @@ class InlineLinkUI extends Component {
 	}
 
 	submitLink( event ) {
-		const { isActive, value, onChange, speak } = this.props;
+		const { isActive, value, onChange, onFocus, speak } = this.props;
 		const { inputValue, opensInNewWindow } = this.state;
 		const url = prependHTTP( inputValue );
 		const selectedText = getTextContent( slice( value ) );
@@ -154,6 +154,8 @@ class InlineLinkUI extends Component {
 			onChange( applyFormat( value, format ) );
 		}
 
+		onFocus();
+
 		this.resetState();
 
 		if ( ! isValidHref( url ) ) {
diff --git a/packages/format-library/src/strikethrough/index.js b/packages/format-library/src/strikethrough/index.js
@@ -13,14 +13,17 @@ export const strikethrough = {
 	title,
 	tagName: 's',
 	className: null,
-	edit( { isActive, value, onChange } ) {
-		const onToggle = () => onChange( toggleFormat( value, { type: name } ) );
+	edit( { isActive, value, onChange, onFocus } ) {
+		function onClick() {
+			onChange( toggleFormat( value, { type: name } ) );
+			onFocus();
+		}
 
 		return (
 			<RichTextToolbarButton
 				icon="editor-strikethrough"
 				title={ title }
-				onClick={ onToggle }
+				onClick={ onClick }
 				isActive={ isActive }
 			/>
 		);
diff --git a/packages/rich-text/src/component/format-edit.js b/packages/rich-text/src/component/format-edit.js
@@ -26,6 +26,7 @@ const interactiveContentTags = new Set( [
 export default function FormatEdit( {
 	formatTypes,
 	onChange,
+	onFocus,
 	value,
 	allowedFormats,
 	withoutInteractiveFormatting,
@@ -68,6 +69,7 @@ export default function FormatEdit( {
 				}
 				value={ value }
 				onChange={ onChange }
+				onFocus={ onFocus }
 			/>
 		);
 	} );
diff --git a/packages/rich-text/src/component/index.js b/packages/rich-text/src/component/index.js
@@ -350,19 +350,29 @@ class RichText extends Component {
 			unstableOnFocus();
 		}
 
-		// 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 = EMPTY_ACTIVE_FORMATS;
-
-		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 = EMPTY_ACTIVE_FORMATS;
+
+			this.record = {
+				...this.record,
+				start: index,
+				end: index,
+				activeFormats,
+			};
+			this.props.onSelectionChange( index, index );
+			this.setState( { activeFormats } );
+		} else {
+			this.props.onSelectionChange( this.record.start, this.record.end );
+			this.setState( {
+				activeFormats: getActiveFormats( {
+					...this.record,
+					activeFormats: undefined,
+				}, EMPTY_ACTIVE_FORMATS ),
+			} );
+		}
 
 		// Update selection as soon as possible, which is at the next animation
 		// frame. The event listener for selection changes may be added too late
@@ -1059,6 +1069,11 @@ class RichText extends Component {
 		} = this.props;
 		const { activeFormats } = this.state;
 
+		const onFocus = () => {
+			forwardedRef.current.focus();
+			this.applyRecord( this.record );
+		};
+
 		return (
 			<>
 				<BoundaryStyle
@@ -1071,12 +1086,14 @@ class RichText extends Component {
 					withoutInteractiveFormatting={ withoutInteractiveFormatting }
 					value={ this.record }
 					onChange={ this.onChange }
+					onFocus={ onFocus }
 					formatTypes={ formatTypes }
 				/> }
 				{ children && children( {
 					isSelected,
 					value: this.record,
 					onChange: this.onChange,
+					onFocus,
 					Editable: this.Editable,
 				} ) }
 				{ ! children && <this.Editable /> }
diff --git 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();
 }

Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ describe( 'Using Plugins API', () => {`
`49`	`49`	`// Click add annotation button.`
`50`	`50`	`const addAnnotationButton = ( await page.$x( "//button[contains(text(), 'Add annotation')]" ) )[ 0 ];`
`51`	`51`	`await addAnnotationButton.click();`
	`52`	`+ await page.evaluate( () => document.querySelector( '[contenteditable]' ).focus() );`
`52`	`53`	`}`
`53`	`54`
`54`	`55`	`/**`
`@@ -60,6 +61,7 @@ describe( 'Using Plugins API', () => {`
`60`	`61`	`// Click remove annotations button.`
`61`	`62`	`const addAnnotationButton = ( await page.$x( "//button[contains(text(), 'Remove annotations')]" ) )[ 0 ];`
`62`	`63`	`await addAnnotationButton.click();`
	`64`	`+ await page.evaluate( () => document.querySelector( '[contenteditable]' ).focus() );`
`63`	`65`	`}`
`64`	`66`
`65`	`67`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ class InlineLinkUI extends Component {`
`135`	`135`	`}`
`136`	`136`
`137`	`137`	`submitLink( event ) {`
`138`		`- const { isActive, value, onChange, speak } = this.props;`
	`138`	`+ const { isActive, value, onChange, onFocus, speak } = this.props;`
`139`	`139`	`const { inputValue, opensInNewWindow } = this.state;`
`140`	`140`	`const url = prependHTTP( inputValue );`
`141`	`141`	`const selectedText = getTextContent( slice( value ) );`
`@@ -154,6 +154,8 @@ class InlineLinkUI extends Component {`
`154`	`154`	`onChange( applyFormat( value, format ) );`
`155`	`155`	`}`
`156`	`156`
	`157`	`+ onFocus();`
	`158`	`+`
`157`	`159`	`this.resetState();`
`158`	`160`
`159`	`161`	`if ( ! isValidHref( url ) ) {`