RichText: remove wrapper (and use Popover for inline toolbar) (WordPr…

…ess#17607) * wip * Calculate inline toolbar position without wrapper element * Fix e2e tests * Use Popover, use createRef * Position Popover to editable element * Revert packages/block-library/src/image/edit.js * Simplify * Fix switcher e2e tests * Adjust anchorRect: subtract padding and add buffer * Remove toolbar padding * Remove dead CSS * Clean up * Increase toolbar buffer by 2 * Remove more dead CSS * Remove more dead CSS
geriux · Oct 2, 2019 · a220eba · a220eba
1 parent 1aa09ac
commit a220eba
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 139 deletions.
diff --git a/packages/block-editor/src/components/rich-text/format-toolbar/style.scss b/packages/block-editor/src/components/rich-text/format-toolbar/style.scss
@@ -1,13 +1,3 @@
-.block-editor-format-toolbar {
-	display: flex;
-	flex-shrink: 0;
-}
-
-.block-editor-format-toolbar__selection-position {
-	position: absolute;
-	transform: translateX(-50%);
-}
-
 .block-editor-format-toolbar .components-dropdown-menu__toggle .components-dropdown-menu__indicator::after {
 	margin: 7px;
 }
diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js
@@ -7,7 +7,7 @@ import { omit } from 'lodash';
 /**
  * WordPress dependencies
  */
-import { RawHTML, Component } from '@wordpress/element';
+import { RawHTML, Component, createRef } from '@wordpress/element';
 import { withDispatch, withSelect } from '@wordpress/data';
 import { pasteHandler, children as childrenSource, getBlockTransforms, findTransform } from '@wordpress/blocks';
 import { withInstanceId, compose } from '@wordpress/compose';
@@ -25,7 +25,7 @@ import {
 	toHTMLString,
 	slice,
 } from '@wordpress/rich-text';
-import { withFilters, IsolatedEventContainer } from '@wordpress/components';
+import { withFilters, Popover } from '@wordpress/components';
 import { createBlobURL } from '@wordpress/blob';
 import deprecated from '@wordpress/deprecated';
 import { isURL } from '@wordpress/url';
@@ -60,11 +60,13 @@ function getMultilineTag( multiline ) {
 class RichTextWrapper extends Component {
 	constructor() {
 		super( ...arguments );
+		this.ref = createRef();
 		this.onEnter = this.onEnter.bind( this );
 		this.onSplit = this.onSplit.bind( this );
 		this.onPaste = this.onPaste.bind( this );
 		this.onDelete = this.onDelete.bind( this );
 		this.inputRule = this.inputRule.bind( this );
+		this.getAnchorRect = this.getAnchorRect.bind( this );
 	}
 
 	onEnter( { value, onChange, shiftKey } ) {
@@ -301,6 +303,30 @@ class RichTextWrapper extends Component {
 		return formattingControls.map( ( name ) => `core/${ name }` );
 	}
 
+	getAnchorRect() {
+		const { current } = this.ref;
+		const rect = current.getBoundingClientRect();
+
+		// Add some space.
+		const buffer = 6;
+
+		// Subtract padding if any.
+		let { paddingTop } = window.getComputedStyle( current );
+
+		paddingTop = parseInt( paddingTop, 10 );
+
+		return {
+			x: rect.left,
+			y: rect.top + paddingTop - buffer,
+			width: rect.width,
+			height: rect.height - paddingTop + buffer,
+			left: rect.left,
+			right: rect.right,
+			top: rect.top + paddingTop - buffer,
+			bottom: rect.bottom,
+		};
+	}
+
 	render() {
 		const {
 			children,
@@ -367,6 +393,7 @@ class RichTextWrapper extends Component {
 		const content = (
 			<RichText
 				{ ...experimentalProps }
+				ref={ this.ref }
 				value={ adjustedValue }
 				onChange={ adjustedOnChange }
 				selectionStart={ selectionStart }
@@ -403,11 +430,15 @@ class RichTextWrapper extends Component {
 							</BlockFormatControls>
 						) }
 						{ isSelected && inlineToolbar && hasFormats && (
-							<IsolatedEventContainer
-								className="editor-rich-text__inline-toolbar block-editor-rich-text__inline-toolbar"
+							<Popover
+								noArrow
+								position="top center"
+								focusOnMount={ false }
+								getAnchorRect={ this.getAnchorRect }
+								className="block-editor-rich-text__inline-format-toolbar"
 							>
 								<FormatToolbar />
-							</IsolatedEventContainer>
+							</Popover>
 						) }
 						{ isSelected && <RemoveBrowserShortcuts /> }
 						<Autocomplete
@@ -433,6 +464,10 @@ class RichTextWrapper extends Component {
 			</RichText>
 		);
 
+		if ( ! wrapperClassName ) {
+			return content;
+		}
+
 		return (
 			<div className={ classnames( wrapperClasses, wrapperClassName ) }>
 				{ content }

diff --git a/packages/block-editor/src/components/rich-text/style.scss b/packages/block-editor/src/components/rich-text/style.scss
@@ -1,8 +1,3 @@
-.block-editor-rich-text {
-	// This is needed to position the formatting toolbar.
-	position: relative;
-}
-
 .block-editor-rich-text__editable {
 	> p:first-child {
 		margin-top: 0;
@@ -57,17 +52,17 @@ figcaption.block-editor-rich-text__editable [data-rich-text-placeholder]::before
 	opacity: 0.8;
 }
 
-.block-editor-rich-text__inline-toolbar {
-	display: flex;
-	justify-content: center;
-	position: absolute;
-	top: -$block-controls-height - 4px;
-	line-height: 0;
-	left: 0;
-	right: 0;
-	z-index: 1;
+.components-popover.block-editor-rich-text__inline-format-toolbar {
+	// Set z-index as if it's displayed on the bottom, otherwise the block
+	// switcher popover might overlap if displayed on the bottom.
+	z-index: z-index(".components-popover:not(.is-mobile).is-bottom");
+
+	.components-popover__content {
+		min-width: auto;
+	}
 
-	ul.components-toolbar {
-		box-shadow: $shadow-toolbar;
+	.components-toolbar {
+		// The popover already provides a border.
+		border: none;
 	}
 }
diff --git a/packages/block-library/src/cover/editor.scss b/packages/block-library/src/cover/editor.scss
@@ -5,11 +5,6 @@
 		color: inherit;
 	}
 
-	&.has-right-content .block-editor-rich-text__inline-toolbar,
-	&.has-left-content .block-editor-rich-text__inline-toolbar {
-		display: inline-block;
-	}
-
 	.block-editor-block-list__layout {
 		width: 100%;
 	}

diff --git a/packages/block-library/src/gallery/editor.scss b/packages/block-library/src/gallery/editor.scss
@@ -32,33 +32,6 @@
 		opacity: 0.3;
 	}
 
-	.block-editor-rich-text {
-		position: absolute;
-		bottom: 0;
-		width: 100%;
-		max-height: 100%;
-		overflow-y: auto;
-	}
-
-	.is-selected .block-editor-rich-text {
-		// IE calculates this incorrectly, so leave it to modern browsers.
-		@supports (position: sticky) {
-			right: 0;
-			left: 0;
-			margin-top: -4px;
-		}
-
-		// Override negative margins so this toolbar isn't hidden by overflow. Overflow is needed for long captions.
-		.block-editor-rich-text__inline-toolbar {
-			top: 0;
-		}
-
-		// Make extra space for the inline toolbar.
-		figcaption {
-			padding-top: 48px;
-		}
-	}
-
 	.is-selected .block-library-gallery-item__move-menu,
 	.is-selected .block-library-gallery-item__inline-menu {
 		background: $white;
@@ -96,15 +69,6 @@
 			color: inherit;
 		}
 	}
-
-	.block-editor-rich-text figcaption {
-		position: relative;
-		overflow: hidden;
-
-		a {
-			color: $white;
-		}
-	}
 }
 
 .block-library-gallery-item__move-menu,

diff --git a/packages/block-library/src/image/editor.scss b/packages/block-library/src/image/editor.scss
@@ -95,13 +95,6 @@
 	.block-editor-block-list__block-edit {
 		figure {
 			margin: 0;
-			display: table;
-		}
-
-		// This maps to the figcaption on the frontend.
-		.block-editor-rich-text {
-			display: table-caption;
-			caption-side: bottom;
 		}
 	}
 }
@@ -116,11 +109,4 @@
 // This is similar to above but for resized unfloated images only, where the markup is different.
 [data-type="core/image"] .block-editor-block-list__block-edit figure.is-resized {
 	margin: 0;
-	display: table;
-
-	// This maps to the figcaption on the frontend.
-	.block-editor-rich-text {
-		display: table-caption;
-		caption-side: bottom;
-	}
 }
diff --git a/packages/e2e-tests/specs/editor-modes.test.js b/packages/e2e-tests/specs/editor-modes.test.js
@@ -17,7 +17,7 @@ describe( 'Editing modes (visual/HTML)', () => {
 
 	it( 'should switch between visual and HTML modes', async () => {
 		// This block should be in "visual" mode by default.
-		let visualBlock = await page.$$( '.block-editor-block-list__layout .block-editor-block-list__block .block-editor-rich-text' );
+		let visualBlock = await page.$$( '.block-editor-block-list__layout .block-editor-block-list__block .rich-text' );
 		expect( visualBlock ).toHaveLength( 1 );
 
 		// Move the mouse to show the block toolbar
@@ -43,7 +43,7 @@ describe( 'Editing modes (visual/HTML)', () => {
 		await changeModeButton.click();
 
 		// This block should be in "visual" mode by default.
-		visualBlock = await page.$$( '.block-editor-block-list__layout .block-editor-block-list__block .block-editor-rich-text' );
+		visualBlock = await page.$$( '.block-editor-block-list__layout .block-editor-block-list__block .rich-text' );
 		expect( visualBlock ).toHaveLength( 1 );
 	} );
 

diff --git a/packages/e2e-tests/specs/reusable-blocks.test.js b/packages/e2e-tests/specs/reusable-blocks.test.js
@@ -143,7 +143,7 @@ describe( 'Reusable Blocks', () => {
 
 		// Check that its content is up to date
 		const text = await page.$eval(
-			'.block-editor-block-list__block[data-type="core/block"] .block-editor-rich-text',
+			'.block-editor-block-list__block[data-type="core/block"] p',
 			( element ) => element.innerText
 		);
 		expect( text ).toMatch( 'Oh! Hello there!' );
@@ -166,7 +166,7 @@ describe( 'Reusable Blocks', () => {
 
 		// Check that its content is up to date
 		const text = await page.$eval(
-			'.block-editor-block-list__block[data-type="core/paragraph"] .block-editor-rich-text',
+			'.block-editor-block-list__block[data-type="core/paragraph"] p',
 			( element ) => element.innerText
 		);
 		expect( text ).toMatch( 'Oh! Hello there!' );

diff --git a/packages/rich-text/src/component/editable.js b/packages/rich-text/src/component/editable.js
@@ -6,19 +6,14 @@ import { isEqual } from 'lodash';
 /**
  * WordPress dependencies
  */
-import { Component, createElement } from '@wordpress/element';
+import { Component, createElement, forwardRef } from '@wordpress/element';
 
 /**
  * Internal dependencies
  */
 import { diffAriaProps } from './aria';
 
-export default class Editable extends Component {
-	constructor() {
-		super();
-		this.bindEditorNode = this.bindEditorNode.bind( this );
-	}
-
+class Editable extends Component {
 	// We must prevent rerenders because the browser will modify the DOM. React
 	// will rerender the DOM fine, but we're losing selection and it would be
 	// more expensive to do so as it would just set the inner HTML through
@@ -29,52 +24,48 @@ export default class Editable extends Component {
 	// update the attributes on the wrapper nodes here. `componentDidUpdate`
 	// will never be called.
 	shouldComponentUpdate( nextProps ) {
+		const element = this.props.forwardedRef.current;
+
 		if ( ! isEqual( this.props.style, nextProps.style ) ) {
-			this.editorNode.setAttribute( 'style', '' );
-			Object.assign( this.editorNode.style, {
+			element.setAttribute( 'style', '' );
+			Object.assign( element.style, {
 				...( nextProps.style || {} ),
 				whiteSpace: 'pre-wrap',
 			} );
 		}
 
 		if ( ! isEqual( this.props.className, nextProps.className ) ) {
-			this.editorNode.className = nextProps.className;
+			element.className = nextProps.className;
 		}
 
 		if ( this.props.start !== nextProps.start ) {
-			this.editorNode.setAttribute( 'start', nextProps.start );
+			element.setAttribute( 'start', nextProps.start );
 		}
 
 		if ( this.props.reversed !== nextProps.reversed ) {
-			this.editorNode.reversed = nextProps.reversed;
+			element.reversed = nextProps.reversed;
 		}
 
 		const { removedKeys, updatedKeys } = diffAriaProps( this.props, nextProps );
 		removedKeys.forEach( ( key ) =>
-			this.editorNode.removeAttribute( key ) );
+			element.removeAttribute( key ) );
 		updatedKeys.forEach( ( key ) =>
-			this.editorNode.setAttribute( key, nextProps[ key ] ) );
+			element.setAttribute( key, nextProps[ key ] ) );
 
 		return false;
 	}
 
-	bindEditorNode( editorNode ) {
-		this.editorNode = editorNode;
-		this.props.setRef( editorNode );
-	}
-
 	render() {
 		const {
 			tagName = 'div',
 			style = {},
 			record,
 			valueToEditableHTML,
 			className,
+			forwardedRef,
 			...remainingProps
 		} = this.props;
 
-		delete remainingProps.setRef;
-
 		// In HTML, leading and trailing spaces are not visible, and multiple
 		// spaces elsewhere are visually reduced to one space. This rule
 		// prevents spaces from collapsing so all space is visible in the editor
@@ -101,7 +92,7 @@ export default class Editable extends Component {
 			'aria-multiline': true,
 			className,
 			contentEditable: true,
-			ref: this.bindEditorNode,
+			ref: forwardedRef,
 			style: {
 				...style,
 				whiteSpace,
@@ -112,3 +103,7 @@ export default class Editable extends Component {
 		} );
 	}
 }
+
+export default forwardRef( ( props, ref ) => {
+	return <Editable { ...props } forwardedRef={ ref } />;
+} );