From 6061ecda75270930c33e204aaedc6a16d4a1b1b2 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 19 Dec 2019 11:51:10 -0300 Subject: [PATCH 01/46] SlotFill initial implementation --- packages/components/src/slot-fill2/README.md | 1 + packages/components/src/slot-fill2/context.js | 97 +++++++++++++++++++ packages/components/src/slot-fill2/fill.js | 23 +++++ packages/components/src/slot-fill2/index.js | 23 +++++ packages/components/src/slot-fill2/slot.js | 32 ++++++ .../src/slot-fill2/stories/index.js | 21 ++++ 6 files changed, 197 insertions(+) create mode 100644 packages/components/src/slot-fill2/README.md create mode 100644 packages/components/src/slot-fill2/context.js create mode 100644 packages/components/src/slot-fill2/fill.js create mode 100644 packages/components/src/slot-fill2/index.js create mode 100644 packages/components/src/slot-fill2/slot.js create mode 100644 packages/components/src/slot-fill2/stories/index.js diff --git a/packages/components/src/slot-fill2/README.md b/packages/components/src/slot-fill2/README.md new file mode 100644 index 00000000000000..de49dc4603d9dd --- /dev/null +++ b/packages/components/src/slot-fill2/README.md @@ -0,0 +1 @@ +# Slot Fill diff --git a/packages/components/src/slot-fill2/context.js b/packages/components/src/slot-fill2/context.js new file mode 100644 index 00000000000000..669ce4561f2603 --- /dev/null +++ b/packages/components/src/slot-fill2/context.js @@ -0,0 +1,97 @@ +/** + * WordPress dependencies + */ +import { + createContext, + useMemo, + useCallback, + useState, + useContext, +} from '@wordpress/element'; + +const SlotFillContext = createContext(); + +function useSlotRegistry() { + const [ slots, setSlots ] = useState( {} ); + + const register = useCallback( ( name, ref, fillProps = {} ) => { + setSlots( ( prevSlots ) => ( { + ...prevSlots, + [ name ]: { + ref, + fillProps, + }, + } ) ); + }, [] ); + + const update = useCallback( ( name, ref, fillProps ) => { + setSlots( ( prevSlots ) => ( { + ...prevSlots, + [ name ]: { + ref: ref || prevSlots[ name ].ref, + fillProps: fillProps || prevSlots[ name ].fillProps || {}, + }, + } ) ); + }, [] ); + + const unregister = useCallback( ( name ) => { + setSlots( ( prevSlots ) => { + // eslint-disable-next-line no-unused-vars + const { [ name ]: _, ...nextSlots } = prevSlots; + return nextSlots; + } ); + } ); + + const registry = useMemo( + () => ( { + slots, + register, + update, + unregister, + } ), + [ slots, register, update, unregister ] + ); + + return registry; +} + +export function useSlot( name ) { + const registry = useContext( SlotFillContext ); + + const { ref, fillProps } = registry.slots[ name ] || {}; + + const update = useCallback( + ( slotRef, slotFillProps ) => { + registry.update( name, slotRef, slotFillProps ); + }, + [ registry.update ] + ); + + const unregister = useCallback( () => { + registry.unregister( name ); + }, [ registry.unregister ] ); + + if ( ! registry.slots[ name ] ) { + return null; + } + + return { + ref, + fillProps, + update, + unregister, + }; +} + +export function useSlotFillContext() { + return useContext( SlotFillContext ); +} + +export default function SlotFillProvider( { children } ) { + const registry = useSlotRegistry(); + return ( + + { children } + + ); +} diff --git a/packages/components/src/slot-fill2/fill.js b/packages/components/src/slot-fill2/fill.js new file mode 100644 index 00000000000000..9ce8f9929d70bc --- /dev/null +++ b/packages/components/src/slot-fill2/fill.js @@ -0,0 +1,23 @@ +/** + * WordPress dependencies + */ +import { createPortal } from '@wordpress/element'; + +/** + * Internal dependencies + */ +import { useSlot } from './context'; + +export default function Fill( { name, children } ) { + const slot = useSlot( name ); + + if ( ! slot ) { + return null; + } + + if ( typeof children === 'function' ) { + children = children( slot.fillProps ); + } + + return createPortal( children, slot.ref.current ); +} diff --git a/packages/components/src/slot-fill2/index.js b/packages/components/src/slot-fill2/index.js new file mode 100644 index 00000000000000..c3042c31bb3f72 --- /dev/null +++ b/packages/components/src/slot-fill2/index.js @@ -0,0 +1,23 @@ +/** + * Internal dependencies + */ +import Slot from './slot'; +import Fill from './fill'; +import Provider from './context'; + +export { Slot }; +export { Fill }; +export { Provider }; + +export function createSlotFill( name ) { + const FillComponent = ( props ) => ; + FillComponent.displayName = name + 'Fill'; + + const SlotComponent = ( props ) => ; + SlotComponent.displayName = name + 'Slot'; + + return { + Fill: FillComponent, + Slot: SlotComponent, + }; +} diff --git a/packages/components/src/slot-fill2/slot.js b/packages/components/src/slot-fill2/slot.js new file mode 100644 index 00000000000000..c2f8800e0de859 --- /dev/null +++ b/packages/components/src/slot-fill2/slot.js @@ -0,0 +1,32 @@ +/** + * WordPress dependencies + */ +import { useEffect, useRef } from '@wordpress/element'; +import isShallowEqual from '@wordpress/is-shallow-equal'; + +/** + * Internal dependencies + */ +import { useSlotFillContext, useSlot } from './context'; + +export default function Slot( { name, fillProps = {}, ...props } ) { + const registry = useSlotFillContext(); + const ref = useRef(); + const slot = useSlot( name ); + const propsAreEqual = slot && isShallowEqual( slot.fillProps, fillProps ); + + useEffect( () => { + registry.register( name, ref, fillProps ); + return () => { + registry.unregister( name ); + }; + }, [ name ] ); + + useEffect( () => { + if ( slot && ! propsAreEqual ) { + registry.update( name, ref, fillProps ); + } + }, [ slot, propsAreEqual, fillProps ] ); + + return
; +} diff --git a/packages/components/src/slot-fill2/stories/index.js b/packages/components/src/slot-fill2/stories/index.js new file mode 100644 index 00000000000000..74a6551c062b5e --- /dev/null +++ b/packages/components/src/slot-fill2/stories/index.js @@ -0,0 +1,21 @@ +/** + * Internal dependencies + */ +import { Slot, Fill, Provider } from '../'; + +export default { title: 'Components|SlotFill', component: Slot }; + +export const _default = () => { + return ( + +
+ +
+
+ +
fill
+ +
+ + ); +}; From 7e38b5a60f2de4eb26cca3a9a46c8186f0bb0832 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 19 Dec 2019 11:51:40 -0300 Subject: [PATCH 02/46] Add manifest-devhub.json --- docs/manifest-devhub.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/manifest-devhub.json b/docs/manifest-devhub.json index 3c47fe380f8f40..cd076978e75790 100644 --- a/docs/manifest-devhub.json +++ b/docs/manifest-devhub.json @@ -953,6 +953,12 @@ "markdown_source": "../packages/components/src/slot-fill/README.md", "parent": "components" }, + { + "title": "SlotFill2", + "slug": "slot-fill2", + "markdown_source": "../packages/components/src/slot-fill2/README.md", + "parent": "components" + }, { "title": "Snackbar", "slug": "snackbar", From d032bee766427f31f793bf41a4a7dda4e92d2960 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 19 Dec 2019 19:39:08 -0300 Subject: [PATCH 03/46] Accept as prop on Slot --- packages/components/src/slot-fill2/slot.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/components/src/slot-fill2/slot.js b/packages/components/src/slot-fill2/slot.js index c2f8800e0de859..0c084b2a3c272a 100644 --- a/packages/components/src/slot-fill2/slot.js +++ b/packages/components/src/slot-fill2/slot.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { useEffect, useRef } from '@wordpress/element'; +import { useEffect, useRef, useLayoutEffect } from '@wordpress/element'; import isShallowEqual from '@wordpress/is-shallow-equal'; /** @@ -9,7 +9,12 @@ import isShallowEqual from '@wordpress/is-shallow-equal'; */ import { useSlotFillContext, useSlot } from './context'; -export default function Slot( { name, fillProps = {}, ...props } ) { +export default function Slot( { + name, + fillProps = {}, + as: Component = 'div', + ...props +} ) { const registry = useSlotFillContext(); const ref = useRef(); const slot = useSlot( name ); @@ -22,11 +27,11 @@ export default function Slot( { name, fillProps = {}, ...props } ) { }; }, [ name ] ); - useEffect( () => { + useLayoutEffect( () => { if ( slot && ! propsAreEqual ) { registry.update( name, ref, fillProps ); } }, [ slot, propsAreEqual, fillProps ] ); - return
; + return ; } From 13b60eff03aaed39bd743f3f80a176a0bb910766 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 19 Dec 2019 19:39:24 -0300 Subject: [PATCH 04/46] Update stories --- .../src/slot-fill2/stories/index.js | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/components/src/slot-fill2/stories/index.js b/packages/components/src/slot-fill2/stories/index.js index 74a6551c062b5e..621bd17200d296 100644 --- a/packages/components/src/slot-fill2/stories/index.js +++ b/packages/components/src/slot-fill2/stories/index.js @@ -1,3 +1,13 @@ +/** + * External dependencies + */ +import { text, number } from '@storybook/addon-knobs'; + +/** + * WordPress dependencies + */ +import { createContext, useContext } from '@wordpress/element'; + /** * Internal dependencies */ @@ -8,14 +18,46 @@ export default { title: 'Components|SlotFill', component: Slot }; export const _default = () => { return ( -
- -
-
- -
fill
- -
+

Profile

+

Name:

+

Age:

+ Grace + 33 + + ); +}; + +export const withFillProps = () => { + const name = text( 'name', 'Grace' ); + const age = number( 'age', 33 ); + return ( + +

Profile

+

Name:

+

Age:

+ { ( fillProps ) => fillProps.name } + { ( fillProps ) => fillProps.age } + + ); +}; + +export const withContext = () => { + const Context = createContext(); + const ContextFill = ( { name } ) => { + const value = useContext( Context ); + return { value }; + }; + return ( + +

Profile

+

Name:

+

Age:

+ + + + + + ); }; From 0c77a8a8415b013e549608ef4f1cd1c86ff4c924 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 20 Dec 2019 00:03:04 -0300 Subject: [PATCH 05/46] Update README.md --- packages/components/src/slot-fill2/README.md | 65 ++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/packages/components/src/slot-fill2/README.md b/packages/components/src/slot-fill2/README.md index de49dc4603d9dd..8609107cdb668b 100644 --- a/packages/components/src/slot-fill2/README.md +++ b/packages/components/src/slot-fill2/README.md @@ -1 +1,66 @@ # Slot Fill + +Slot and Fill are a pair of components which enable developers to render elsewhere in a React element tree, a pattern often referred to as "portal" rendering. It is a pattern for component extensibility, where a single Slot may be occupied by an indeterminate number of Fills elsewhere in the application. + +Slot Fill is heavily inspired by the [`react-slot-fill` library](https://github.com/camwest/react-slot-fill), but uses [React's own portal rendering API](https://reactjs.org/docs/portals.html). + +## Usage + +At the root of your application, you must render a `SlotFillProvider` which coordinates Slot and Fill rendering. + +Then, render a Slot component anywhere in your application, giving it a name. + +Any Fill will automatically occupy this Slot space, even if rendered elsewhere in the application. + +You can either use the Fill component directly, or a wrapper component type as in the below example to abstract the slot name from consumer awareness. + +```jsx +import { SlotFillProvider, Slot, Fill, Panel, PanelBody } from '@wordpress/components'; + +const MySlotFillProvider = () => { + const MyPanelSlot = () => ( + + + + + + ); + + MyPanelSlot.Content = () => ( + + Panel body + + ); + + return ( + + + + + ); +}; +``` + +There is also `createSlotFill` helper method which was created to simplify the process of matching the corresponding `Slot` and `Fill` components: + +```jsx +const { Fill, Slot } = createSlotFill( 'Toolbar' ); + +const ToolbarItem = () => ( + + My item + +); + +const Toolbar = () => ( +
+ +
+); +``` + +## Props + +The `SlotFillProvider` component does not accept any props. + +Both `Slot` and `Fill` accept a `name` string prop, where a `Slot` with a given `name` will render the `children` of any associated `Fill`s. From bd84603c73506b75c20e1c96d7a56a5de9c6d5c3 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 20 Dec 2019 00:03:29 -0300 Subject: [PATCH 06/46] Update code --- packages/components/src/slot-fill2/context.js | 9 ++-- packages/components/src/slot-fill2/index.js | 4 +- packages/components/src/slot-fill2/slot.js | 19 +++++--- .../src/slot-fill2/stories/index.js | 14 +++--- storybook/test/__snapshots__/index.js.snap | 48 +++++++++++++++++++ 5 files changed, 72 insertions(+), 22 deletions(-) diff --git a/packages/components/src/slot-fill2/context.js b/packages/components/src/slot-fill2/context.js index 669ce4561f2603..aefe94b67d4544 100644 --- a/packages/components/src/slot-fill2/context.js +++ b/packages/components/src/slot-fill2/context.js @@ -9,7 +9,7 @@ import { useContext, } from '@wordpress/element'; -const SlotFillContext = createContext(); +export const SlotFillContext = createContext(); function useSlotRegistry() { const [ slots, setSlots ] = useState( {} ); @@ -40,8 +40,9 @@ function useSlotRegistry() { const { [ name ]: _, ...nextSlots } = prevSlots; return nextSlots; } ); - } ); + }, [] ); + // Memoizing the return value so it can be directly passed to Provider value const registry = useMemo( () => ( { slots, @@ -83,10 +84,6 @@ export function useSlot( name ) { }; } -export function useSlotFillContext() { - return useContext( SlotFillContext ); -} - export default function SlotFillProvider( { children } ) { const registry = useSlotRegistry(); return ( diff --git a/packages/components/src/slot-fill2/index.js b/packages/components/src/slot-fill2/index.js index c3042c31bb3f72..528f8a009b5266 100644 --- a/packages/components/src/slot-fill2/index.js +++ b/packages/components/src/slot-fill2/index.js @@ -3,11 +3,11 @@ */ import Slot from './slot'; import Fill from './fill'; -import Provider from './context'; +import SlotFillProvider from './context'; export { Slot }; export { Fill }; -export { Provider }; +export { SlotFillProvider }; export function createSlotFill( name ) { const FillComponent = ( props ) => ; diff --git a/packages/components/src/slot-fill2/slot.js b/packages/components/src/slot-fill2/slot.js index 0c084b2a3c272a..ebe9e89b72d816 100644 --- a/packages/components/src/slot-fill2/slot.js +++ b/packages/components/src/slot-fill2/slot.js @@ -1,13 +1,13 @@ /** * WordPress dependencies */ -import { useEffect, useRef, useLayoutEffect } from '@wordpress/element'; +import { useEffect, useRef, useLayoutEffect, useContext } from '@wordpress/element'; import isShallowEqual from '@wordpress/is-shallow-equal'; /** * Internal dependencies */ -import { useSlotFillContext, useSlot } from './context'; +import { SlotFillContext, useSlot } from './context'; export default function Slot( { name, @@ -15,23 +15,28 @@ export default function Slot( { as: Component = 'div', ...props } ) { - const registry = useSlotFillContext(); + const registry = useContext( SlotFillContext ); const ref = useRef(); const slot = useSlot( name ); - const propsAreEqual = slot && isShallowEqual( slot.fillProps, fillProps ); useEffect( () => { registry.register( name, ref, fillProps ); return () => { registry.unregister( name ); }; - }, [ name ] ); + // We are not including fillProps in the deps because we don't want to + // unregister and register the slot whenever fillProps change, which would + // cause the fill to be re-mounted. We are only considering the initial value + // of fillProps. + }, [ registry.register, registry.unregister, name ] ); + // fillProps may be an update that interact with the layout, so + // we useLayoutEffect useLayoutEffect( () => { - if ( slot && ! propsAreEqual ) { + if ( slot && ! isShallowEqual( slot.fillProps, fillProps ) ) { registry.update( name, ref, fillProps ); } - }, [ slot, propsAreEqual, fillProps ] ); + } ); return ; } diff --git a/packages/components/src/slot-fill2/stories/index.js b/packages/components/src/slot-fill2/stories/index.js index 621bd17200d296..bd8dfef54463d5 100644 --- a/packages/components/src/slot-fill2/stories/index.js +++ b/packages/components/src/slot-fill2/stories/index.js @@ -11,19 +11,19 @@ import { createContext, useContext } from '@wordpress/element'; /** * Internal dependencies */ -import { Slot, Fill, Provider } from '../'; +import { Slot, Fill, SlotFillProvider } from '../'; export default { title: 'Components|SlotFill', component: Slot }; export const _default = () => { return ( - +

Profile

Name:

Age:

Grace 33 - + ); }; @@ -31,13 +31,13 @@ export const withFillProps = () => { const name = text( 'name', 'Grace' ); const age = number( 'age', 33 ); return ( - +

Profile

Name:

Age:

{ ( fillProps ) => fillProps.name } { ( fillProps ) => fillProps.age } - + ); }; @@ -48,7 +48,7 @@ export const withContext = () => { return { value }; }; return ( - +

Profile

Name:

Age:

@@ -58,6 +58,6 @@ export const withContext = () => { - + ); }; diff --git a/storybook/test/__snapshots__/index.js.snap b/storybook/test/__snapshots__/index.js.snap index a15111a15621e4..4cb76f84387c72 100644 --- a/storybook/test/__snapshots__/index.js.snap +++ b/storybook/test/__snapshots__/index.js.snap @@ -3444,6 +3444,54 @@ exports[`Storyshots Components|ScrollLock Default 1`] = `
`; +exports[`Storyshots Components|SlotFill Default 1`] = ` +Array [ +

+ Profile +

, +

+ Name: + +

, +

+ Age: + +

, +] +`; + +exports[`Storyshots Components|SlotFill With Context 1`] = ` +Array [ +

+ Profile +

, +

+ Name: + +

, +

+ Age: + +

, +] +`; + +exports[`Storyshots Components|SlotFill With Fill Props 1`] = ` +Array [ +

+ Profile +

, +

+ Name: + +

, +

+ Age: + +

, +] +`; + exports[`Storyshots Components|Snackbar Default 1`] = `
Date: Fri, 20 Dec 2019 00:21:41 -0300 Subject: [PATCH 07/46] Add slot-fill2 entries to components index file --- packages/components/src/index.js | 7 +++++++ packages/components/src/index.native.js | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 02a2f62492ae2c..14b19a6bfa1475 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -85,6 +85,13 @@ export { Consumer as __experimentalSlotFillConsumer, } from './slot-fill'; +export { + createSlotFill as __experimentalCreateSlotFill, + Slot as __experimentalSlot, + Fill as __experimentalFill, + SlotFillProvider as __experimentalSlotFillProvider, +} from './slot-fill2'; + // Higher-Order Components export { default as navigateRegions } from './higher-order/navigate-regions'; export { diff --git a/packages/components/src/index.native.js b/packages/components/src/index.native.js index 66e5e609cb7539..924ad475e9d4f7 100644 --- a/packages/components/src/index.native.js +++ b/packages/components/src/index.native.js @@ -11,6 +11,12 @@ export { default as Icon } from './icon'; export { default as IconButton } from './icon-button'; export { default as Spinner } from './spinner'; export { createSlotFill, Slot, Fill, Provider as SlotFillProvider } from './slot-fill'; +export { + createSlotFill as __experimentalCreateSlotFill, + Slot as __experimentalSlot, + Fill as __experimentalFill, + SlotFillProvider as __experimentalSlotFillProvider, +} from './slot-fill2'; export { default as BaseControl } from './base-control'; export { default as TextareaControl } from './textarea-control'; export { default as PanelBody } from './panel/body'; From 229d432b364751950109ec9f6f6d55a5ed93c4ed Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 20 Dec 2019 13:35:08 -0300 Subject: [PATCH 08/46] Add unit tests --- .../components/src/slot-fill2/test/index.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 packages/components/src/slot-fill2/test/index.js diff --git a/packages/components/src/slot-fill2/test/index.js b/packages/components/src/slot-fill2/test/index.js new file mode 100644 index 00000000000000..6bd4c3a7c9247e --- /dev/null +++ b/packages/components/src/slot-fill2/test/index.js @@ -0,0 +1,28 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; + +/** + * Internal dependencies + */ +import { createSlotFill, Fill, Slot } from '../'; + +describe( 'createSlotFill', () => { + const SLOT_NAME = 'MySlotFill'; + const MySlotFill = createSlotFill( SLOT_NAME ); + + test( 'should match snapshot for Fill', () => { + const wrapper = shallow( ); + + expect( wrapper.type() ).toBe( Fill ); + expect( wrapper.prop( 'name' ) ).toBe( SLOT_NAME ); + } ); + + test( 'should match snapshot for Slot', () => { + const wrapper = shallow( ); + + expect( wrapper.type() ).toBe( Slot ); + expect( wrapper.prop( 'name' ) ).toBe( SLOT_NAME ); + } ); +} ); From e8bbb9b8bfe71fe04b6744b1b6217831e6cb3036 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Jan 2020 16:18:09 -0300 Subject: [PATCH 09/46] Fix git conflicts --- .../src/components/provider/index.native.js | 104 +++++++++++------- packages/block-library/src/paragraph/edit.js | 6 +- packages/edit-post/src/editor.js | 6 +- .../edit-site/src/components/editor/index.js | 31 ++++-- .../src/components/layout/index.js | 2 + 5 files changed, 96 insertions(+), 53 deletions(-) diff --git a/packages/block-editor/src/components/provider/index.native.js b/packages/block-editor/src/components/provider/index.native.js index 1d3d46467288c6..a9dfbf11430c60 100644 --- a/packages/block-editor/src/components/provider/index.native.js +++ b/packages/block-editor/src/components/provider/index.native.js @@ -1,40 +1,28 @@ +/** + * External dependencies + */ +import { last, noop } from 'lodash'; + /** * WordPress dependencies */ import { Component } from '@wordpress/element'; -import { SlotFillProvider } from '@wordpress/components'; -import { withDispatch, RegistryConsumer } from '@wordpress/data'; -import { createHigherOrderComponent, compose } from '@wordpress/compose'; - -/** @typedef {import('@wordpress/data').WPDataRegistry} WPDataRegistry */ +import { withDispatch } from '@wordpress/data'; +import { compose } from '@wordpress/compose'; /** - * Higher-order component which renders the original component with the current - * registry context passed as its `registry` prop. - * - * @param {WPComponent} OriginalComponent Original component. - * - * @return {WPComponent} Enhanced component. + * Internal dependencies */ -const withRegistry = createHigherOrderComponent( - ( OriginalComponent ) => ( props ) => ( - - { ( registry ) => ( - - ) } - - ), - 'withRegistry' -); +import withRegistryProvider from './with-registry-provider'; + +/** @typedef {import('@wordpress/data').WPDataRegistry} WPDataRegistry */ class BlockEditorProvider extends Component { componentDidMount() { this.props.updateSettings( this.props.settings ); this.props.resetBlocks( this.props.value ); this.attachChangeObserver( this.props.registry ); + this.isSyncingOutcomingValue = []; } componentDidUpdate( prevProps ) { @@ -43,6 +31,9 @@ class BlockEditorProvider extends Component { updateSettings, value, resetBlocks, + selectionStart, + selectionEnd, + resetSelection, registry, } = this.props; @@ -54,11 +45,28 @@ class BlockEditorProvider extends Component { this.attachChangeObserver( registry ); } - if ( this.isSyncingOutcomingValue ) { - this.isSyncingOutcomingValue = false; + if ( this.isSyncingOutcomingValue.includes( value ) ) { + // Skip block reset if the value matches expected outbound sync + // triggered by this component by a preceding change detection. + // Only skip if the value matches expectation, since a reset should + // still occur if the value is modified (not equal by reference), + // to allow that the consumer may apply modifications to reflect + // back on the editor. + if ( last( this.isSyncingOutcomingValue ) === value ) { + this.isSyncingOutcomingValue = []; + } } else if ( value !== prevProps.value ) { - this.isSyncingIncomingValue = true; + // Reset changing value in all other cases than the sync described + // above. Since this can be reached in an update following an out- + // bound sync, unset the outbound value to avoid considering it in + // subsequent renders. + this.isSyncingOutcomingValue = []; + this.isSyncingIncomingValue = value; resetBlocks( value ); + + if ( selectionStart && selectionEnd ) { + resetSelection( selectionStart, selectionEnd ); + } } } @@ -87,7 +95,10 @@ class BlockEditorProvider extends Component { const { getBlocks, + getSelectionStart, + getSelectionEnd, isLastBlockChangePersistent, + __unstableIsLastBlockChangeIgnored, } = registry.select( 'core/block-editor' ); let blocks = getBlocks(); @@ -95,13 +106,20 @@ class BlockEditorProvider extends Component { this.unsubscribe = registry.subscribe( () => { const { - onChange, - onInput, + onChange = noop, + onInput = noop, } = this.props; + const newBlocks = getBlocks(); const newIsPersistent = isLastBlockChangePersistent(); - if ( newBlocks !== blocks && this.isSyncingIncomingValue ) { - this.isSyncingIncomingValue = false; + + if ( + newBlocks !== blocks && ( + this.isSyncingIncomingValue || + __unstableIsLastBlockChangeIgnored() + ) + ) { + this.isSyncingIncomingValue = null; blocks = newBlocks; isPersistent = newIsPersistent; return; @@ -112,14 +130,22 @@ class BlockEditorProvider extends Component { // This happens when a previous input is explicitely marked as persistent. ( newIsPersistent && ! isPersistent ) ) { + // When knowing the blocks value is changing, assign instance + // value to skip reset in subsequent `componentDidUpdate`. + if ( newBlocks !== blocks ) { + this.isSyncingOutcomingValue.push( newBlocks ); + } + blocks = newBlocks; isPersistent = newIsPersistent; - this.isSyncingOutcomingValue = true; + const selectionStart = getSelectionStart(); + const selectionEnd = getSelectionEnd(); + if ( isPersistent ) { - onChange( blocks ); + onChange( blocks, { selectionStart, selectionEnd } ); } else { - onInput( blocks ); + onInput( blocks, { selectionStart, selectionEnd } ); } } } ); @@ -128,25 +154,23 @@ class BlockEditorProvider extends Component { render() { const { children } = this.props; - return ( - - { children } - - ); + return children; } } export default compose( [ + withRegistryProvider, withDispatch( ( dispatch ) => { const { updateSettings, resetBlocks, + resetSelection, } = dispatch( 'core/block-editor' ); return { updateSettings, resetBlocks, + resetSelection, }; } ), - withRegistry, ] )( BlockEditorProvider ); diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js index d53b6483e626b8..983b92a1a15097 100644 --- a/packages/block-library/src/paragraph/edit.js +++ b/packages/block-library/src/paragraph/edit.js @@ -134,9 +134,9 @@ function ParagraphBlock( { label={ __( 'Drop Cap' ) } checked={ !! dropCap } onChange={ () => setAttributes( { dropCap: ! dropCap } ) } - help={ dropCap - ? __( 'Showing large initial letter.' ) - : __( 'Toggle to show a large initial letter.' ) + help={ dropCap ? + __( 'Showing large initial letter.' ) : + __( 'Toggle to show a large initial letter.' ) } /> diff --git a/packages/edit-post/src/editor.js b/packages/edit-post/src/editor.js index 88835b9203e17a..eb6754e7cf665c 100644 --- a/packages/edit-post/src/editor.js +++ b/packages/edit-post/src/editor.js @@ -63,9 +63,9 @@ class Editor extends Component { // anything other than `true` (where `true` is equivalent to allow // all block types). const defaultAllowedBlockTypes = ( - true === settings.allowedBlockTypes - ? map( blockTypes, 'name' ) - : ( settings.allowedBlockTypes || [] ) + true === settings.allowedBlockTypes ? + map( blockTypes, 'name' ) : + ( settings.allowedBlockTypes || [] ) ); settings.allowedBlockTypes = without( diff --git a/packages/edit-site/src/components/editor/index.js b/packages/edit-site/src/components/editor/index.js index f7f0ef0286b10c..83a0f498a35fca 100644 --- a/packages/edit-site/src/components/editor/index.js +++ b/packages/edit-site/src/components/editor/index.js @@ -1,12 +1,14 @@ /** * WordPress dependencies */ +import { useSelect } from '@wordpress/data'; import { SlotFillProvider, DropZoneProvider, Popover, navigateRegions, } from '@wordpress/components'; +import { EntityProvider } from '@wordpress/core-data'; /** * Internal dependencies @@ -17,17 +19,32 @@ import Sidebar from '../sidebar'; import BlockEditor from '../block-editor'; function Editor( { settings } ) { - return ( + const template = useSelect( + ( select ) => + select( 'core' ).getEntityRecord( + 'postType', + 'wp_template', + settings.templateId + ), + [] + ); + return template ? ( - -
- - - + + +
+ + + + - ); + ) : null; } export default navigateRegions( Editor ); diff --git a/packages/edit-widgets/src/components/layout/index.js b/packages/edit-widgets/src/components/layout/index.js index 9011f8fff87f7c..15480f21f77a4e 100644 --- a/packages/edit-widgets/src/components/layout/index.js +++ b/packages/edit-widgets/src/components/layout/index.js @@ -9,6 +9,7 @@ import { SlotFillProvider, } from '@wordpress/components'; import { useState } from '@wordpress/element'; +import { BlockEditorKeyboardShortcuts } from '@wordpress/block-editor'; /** * Internal dependencies @@ -23,6 +24,7 @@ function Layout( { blockEditorSettings } ) { return ( +
From 861e8ecb2e45fcb1e8bea26d3e3ad75582028b0f Mon Sep 17 00:00:00 2001 From: Haz Date: Mon, 3 Feb 2020 13:37:26 -0300 Subject: [PATCH 10/46] Convert fixed toolbar buttons into ToolbarButton --- .../block-alignment-toolbar/index.js | 8 +- .../src/components/block-controls/index.js | 32 ++- .../components/block-format-controls/index.js | 27 ++- .../src/components/block-list/style.scss | 5 + .../src/components/block-mover/index.js | 17 +- .../components/block-settings-menu/index.js | 190 +++++++++--------- .../src/components/block-switcher/index.js | 84 +++----- .../src/components/block-toolbar/index.js | 10 +- .../src/components/block-toolbar/style.scss | 7 + .../src/components/navigable-toolbar/index.js | 35 +++- .../src/components/rich-text/style.scss | 1 + .../src/components/url-input/style.scss | 1 + packages/components/src/index.js | 1 + packages/components/src/index.native.js | 1 + .../components/src/toolbar-button/index.js | 1 + .../toolbar-group/toolbar-group-collapsed.js | 2 +- packages/components/src/toolbar-item/index.js | 24 +-- .../src/toolbar-item/index.native.js | 18 -- .../toolbar-item/toolbar-item-container.js | 27 +++ packages/components/src/toolbar/index.js | 11 +- .../header/header-toolbar/style.scss | 2 + .../src/components/sidebar/style.scss | 2 + packages/edit-post/src/editor.js | 39 ++-- .../edit-site/src/components/editor/index.js | 29 +-- .../src/components/layout/index.js | 47 +++-- storybook/stories/playground/index.js | 43 ++-- 26 files changed, 371 insertions(+), 293 deletions(-) delete mode 100644 packages/components/src/toolbar-item/index.native.js create mode 100644 packages/components/src/toolbar-item/toolbar-item-container.js diff --git a/packages/block-editor/src/components/block-alignment-toolbar/index.js b/packages/block-editor/src/components/block-alignment-toolbar/index.js index 84a7c6160d34d5..d7d1d1b4ebeaae 100644 --- a/packages/block-editor/src/components/block-alignment-toolbar/index.js +++ b/packages/block-editor/src/components/block-alignment-toolbar/index.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Toolbar } from '@wordpress/components'; +import { ToolbarGroup } from '@wordpress/components'; import { withSelect } from '@wordpress/data'; import { compose } from '@wordpress/compose'; import { @@ -58,16 +58,14 @@ export function BlockAlignmentToolbar( { const enabledControls = wideControlsEnabled ? controls - : controls.filter( - ( control ) => WIDE_CONTROLS.indexOf( control ) === -1 - ); + : controls.filter( ( control ) => WIDE_CONTROLS.indexOf( control ) === -1 ); const activeAlignmentControl = BLOCK_ALIGNMENTS_CONTROLS[ value ]; const defaultAlignmentControl = BLOCK_ALIGNMENTS_CONTROLS[ DEFAULT_CONTROL ]; return ( - ( - - - { children } - -); +function BlockControlsSlot( props ) { + const accessibleToolbarState = useContext( ToolbarContext ); + return ; +} + +function BlockControlsFill( { controls, children } ) { + return ( + + { ( fillProps ) => ( + + + { children } + + ) } + + ); +} const BlockControls = ifBlockEditSelected( BlockControlsFill ); -BlockControls.Slot = Slot; +BlockControls.Slot = BlockControlsSlot; export default BlockControls; diff --git a/packages/block-editor/src/components/block-format-controls/index.js b/packages/block-editor/src/components/block-format-controls/index.js index 2ab83843846ad4..58d5757be9b153 100644 --- a/packages/block-editor/src/components/block-format-controls/index.js +++ b/packages/block-editor/src/components/block-format-controls/index.js @@ -1,7 +1,11 @@ /** * WordPress dependencies */ -import { createSlotFill } from '@wordpress/components'; +import { useContext } from '@wordpress/element'; +import { + __experimentalCreateSlotFill as createSlotFill, + __experimentalToolbarContext as ToolbarContext, +} from '@wordpress/components'; /** * Internal dependencies @@ -10,8 +14,25 @@ import { ifBlockEditSelected } from '../block-edit/context'; const { Fill, Slot } = createSlotFill( 'BlockFormatControls' ); -const BlockFormatControls = ifBlockEditSelected( Fill ); +function BlockFormatControlsSlot( props ) { + const accessibleToolbarState = useContext( ToolbarContext ); + return ; +} -BlockFormatControls.Slot = Slot; +function BlockFormatControlsFill( props ) { + return ( + + { ( fillProps ) => ( + + { props.children } + + ) } + + ); +} + +const BlockFormatControls = ifBlockEditSelected( BlockFormatControlsFill ); + +BlockFormatControls.Slot = BlockFormatControlsSlot; export default BlockFormatControls; diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss index 739aa957b266d8..57cd8dc678fb4b 100644 --- a/packages/block-editor/src/components/block-list/style.scss +++ b/packages/block-editor/src/components/block-list/style.scss @@ -1,3 +1,7 @@ +.block-editor-block-contextual-toolbar { + display: block; +} + .block-editor-block-list__layout .block-editor-block-list__block.is-selected { // Needs specificity to override inherited styles. // While block is being dragged, dim the slot dragged from, and hide some UI. &.is-dragging { @@ -492,6 +496,7 @@ line-height: 1; z-index: z-index(".block-editor-block-list__breadcrumb"); + .components-toolbar-group, .components-toolbar { display: flex; background: $white; diff --git a/packages/block-editor/src/components/block-mover/index.js b/packages/block-editor/src/components/block-mover/index.js index 7e29326458fd13..2be2a30a91a51e 100644 --- a/packages/block-editor/src/components/block-mover/index.js +++ b/packages/block-editor/src/components/block-mover/index.js @@ -8,7 +8,7 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { Button, ToolbarGroup } from '@wordpress/components'; +import { Button, ToolbarGroup, ToolbarButton } from '@wordpress/components'; import { getBlockType } from '@wordpress/blocks'; import { Component } from '@wordpress/element'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -107,15 +107,12 @@ export class BlockMover extends Component { 'is-horizontal': orientation === 'horizontal', } ) } > -
{ shouldShowVisualToolbar && ! isMultiToolbar && ( <> - diff --git a/packages/block-editor/src/components/block-toolbar/style.scss b/packages/block-editor/src/components/block-toolbar/style.scss index 758ee67a22e6ec..9c24d1715f2516 100644 --- a/packages/block-editor/src/components/block-toolbar/style.scss +++ b/packages/block-editor/src/components/block-toolbar/style.scss @@ -158,14 +158,14 @@ } } - .components-toolbar-group { - border-color: $light-gray-800; - border-left: 0; - margin: 0; - } + // .components-toolbar-group { + // border-color: $light-gray-800; + // border-left: 0; + // margin: 0; + // } // Single buttons should remain 48px. - .components-toolbar-group div:first-child:last-child > .components-button, + // .components-toolbar-group div:first-child:last-child > .components-button, .components-toolbar div:first-child:last-child > .components-button { min-width: $block-toolbar-height; padding-left: $grid-unit-15; diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 1323c0dcc1cccc..3811648c741336 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -7,13 +7,17 @@ import { focus } from '@wordpress/dom'; import { useShortcut } from '@wordpress/keyboard-shortcuts'; function useShouldDisplayAccessibleToolbar( ref ) { - const [ shouldDisplayAccessibleToolbar, setShouldDisplayAccessibleToolbar ] = useState( false ); + const [ + shouldDisplayAccessibleToolbar, + setShouldDisplayAccessibleToolbar, + ] = useState( false ); useEffect( () => { const wrapper = ref.current; window.requestAnimationFrame( () => { const focusables = focus.focusable.find( wrapper ); const notToolbarItem = focusables.find( - ( focusable ) => ! ( 'experimentalToolbarItem' in focusable.dataset ) + ( focusable ) => + ! ( 'experimentalToolbarItem' in focusable.dataset ) ); if ( ! notToolbarItem ) { setShouldDisplayAccessibleToolbar( true ); @@ -25,7 +29,9 @@ function useShouldDisplayAccessibleToolbar( ref ) { function NavigableToolbar( { children, focusOnMount, ...props } ) { const wrapper = useRef(); - const shouldDisplayAccessibleToolbar = useShouldDisplayAccessibleToolbar( wrapper ); + const shouldDisplayAccessibleToolbar = useShouldDisplayAccessibleToolbar( + wrapper + ); const focusToolbar = useCallback( () => { const tabbables = focus.tabbable.find( wrapper.current ); diff --git a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js index d487d98fbfa9a1..e4bfdeed5aec59 100644 --- a/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js +++ b/packages/components/src/slot-fill/bubbles-virtually/slot-fill-provider.js @@ -13,14 +13,17 @@ function useSlotRegistry() { const [ fills, setFills ] = useState( {} ); const registerSlot = useCallback( ( name, ref, fillProps ) => { - setSlots( ( prevSlots ) => ( { - ...prevSlots, - [ name ]: { - ...prevSlots[ name ], - ref: ref || prevSlots[ name ].ref, - fillProps: fillProps || prevSlots[ name ].fillProps || {}, - }, - } ) ); + setSlots( ( prevSlots ) => { + const currentSlot = prevSlots[ name ] || {}; + return { + ...prevSlots, + [ name ]: { + ...currentSlot, + ref: ref || currentSlot.ref, + fillProps: fillProps || currentSlot.fillProps || {}, + }, + }; + } ); }, [] ); const unregisterSlot = useCallback( ( name, ref ) => { From ec33aa010ae77812d1e234aed8af5802a48a7be1 Mon Sep 17 00:00:00 2001 From: Haz Date: Mon, 2 Mar 2020 14:03:59 -0300 Subject: [PATCH 14/46] Temporarily fix toolbar buttons styles --- .../src/components/block-list/style.scss | 2 +- .../src/components/block-mover/index.js | 6 +-- .../src/components/block-mover/style.scss | 4 ++ .../src/components/block-switcher/index.js | 19 ++++--- .../src/components/block-switcher/style.scss | 6 --- .../src/components/block-toolbar/style.scss | 27 ++++++---- .../components/src/toolbar-button/index.js | 53 ++++++++++--------- 7 files changed, 65 insertions(+), 52 deletions(-) diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss index 9acddaa26a4aee..ad705871d3c02e 100644 --- a/packages/block-editor/src/components/block-list/style.scss +++ b/packages/block-editor/src/components/block-list/style.scss @@ -466,7 +466,7 @@ display: block; z-index: z-index(".block-editor-block-list__breadcrumb"); - // .components-toolbar-group, + .components-toolbar-group, .components-toolbar { display: flex; border: none; diff --git a/packages/block-editor/src/components/block-mover/index.js b/packages/block-editor/src/components/block-mover/index.js index 874ae0d000f5cf..7fed3c29a68cda 100644 --- a/packages/block-editor/src/components/block-mover/index.js +++ b/packages/block-editor/src/components/block-mover/index.js @@ -8,7 +8,7 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { ToolbarGroup, Button } from '@wordpress/components'; +import { ToolbarGroup, ToolbarButton } from '@wordpress/components'; import { getBlockType } from '@wordpress/blocks'; import { Component } from '@wordpress/element'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -114,7 +114,7 @@ export class BlockMover extends Component { onDragEnd={ onDraggableEnd } > - + ) } ); diff --git a/packages/components/src/toolbar-item/index.js b/packages/components/src/toolbar-item/index.js index d7cf30bea0165c..dcfdca4ea46bc9 100644 --- a/packages/components/src/toolbar-item/index.js +++ b/packages/components/src/toolbar-item/index.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { ToolbarItem as BaseToolbarItem } from 'reakit/Toolbar'; + /** * WordPress dependencies */ @@ -8,7 +13,6 @@ import warning from '@wordpress/warning'; * Internal dependencies */ import ToolbarContext from '../toolbar-context'; -import ToolbarItemContainer from './toolbar-item-container'; function ToolbarItem( { children, ...props }, ref ) { const accessibleToolbarState = useContext( ToolbarContext ); @@ -27,9 +31,9 @@ function ToolbarItem( { children, ...props }, ref ) { } return ( - + { children } - + ); } diff --git a/packages/components/src/toolbar-item/toolbar-item-container.js b/packages/components/src/toolbar-item/toolbar-item-container.js deleted file mode 100644 index 72284f57b71ea6..00000000000000 --- a/packages/components/src/toolbar-item/toolbar-item-container.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * External dependencies - */ -import { useToolbarItem } from 'reakit/Toolbar'; - -/** - * WordPress dependencies - */ -import { forwardRef, useContext } from '@wordpress/element'; - -/** - * Internal dependencies - */ -import ToolbarContext from '../toolbar-context'; - -function ToolbarItemContainer( { children, ...props }, ref ) { - const accessibleToolbarState = useContext( ToolbarContext ); - // https://reakit.io/docs/composition/#props-hooks - const itemProps = useToolbarItem( accessibleToolbarState, { - ...props, - ref, - } ); - - return children( itemProps ); -} - -export default forwardRef( ToolbarItemContainer ); diff --git a/packages/components/src/toolbar/style.scss b/packages/components/src/toolbar/style.scss index fdbcccb3f20294..ae147df18885c4 100644 --- a/packages/components/src/toolbar/style.scss +++ b/packages/components/src/toolbar/style.scss @@ -4,7 +4,7 @@ border-radius: $radius-block-ui; flex-shrink: 0; - .components-toolbar-group:last-child { + & > .components-toolbar-group:last-child { border-right: none; } } diff --git a/packages/interface/src/components/complementary-area/style.scss b/packages/interface/src/components/complementary-area/style.scss index 469e6605d13d5f..30ba90853102b8 100644 --- a/packages/interface/src/components/complementary-area/style.scss +++ b/packages/interface/src/components/complementary-area/style.scss @@ -65,6 +65,7 @@ margin: 1.5em 0; } + div.components-toolbar-group, div.components-toolbar { box-shadow: none; margin-bottom: 1.5em; From 2999c85f786352f7505f90b0971d7d9c02ad7404 Mon Sep 17 00:00:00 2001 From: Haz Date: Tue, 28 Apr 2020 22:53:13 -0300 Subject: [PATCH 21/46] Fix unit tests --- .../block-editor/src/components/alignment-toolbar/index.js | 4 ++-- .../alignment-toolbar/test/__snapshots__/index.js.snap | 4 ++-- .../test/__snapshots__/index.js.snap | 2 +- .../block-switcher/test/__snapshots__/index.js.snap | 6 +++--- .../src/components/block-switcher/test/index.js | 6 ++---- .../components/block-vertical-alignment-toolbar/index.js | 4 ++-- .../test/__snapshots__/index.js.snap | 2 +- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/block-editor/src/components/alignment-toolbar/index.js b/packages/block-editor/src/components/alignment-toolbar/index.js index 2536e2e7e79b96..00f5ea7438fe1f 100644 --- a/packages/block-editor/src/components/alignment-toolbar/index.js +++ b/packages/block-editor/src/components/alignment-toolbar/index.js @@ -7,7 +7,7 @@ import { find } from 'lodash'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Toolbar } from '@wordpress/components'; +import { ToolbarGroup } from '@wordpress/components'; import { alignLeft, alignRight, alignCenter } from '@wordpress/icons'; const DEFAULT_ALIGNMENT_CONTROLS = [ @@ -52,7 +52,7 @@ export function AlignmentToolbar( props ) { ); return ( - - + - + `; exports[`BlockSwitcher should render enabled block switcher with multi block when transforms exist 1`] = ` diff --git a/packages/block-editor/src/components/block-switcher/test/index.js b/packages/block-editor/src/components/block-switcher/test/index.js index dea53770ca7d07..75755bcd561d4d 100644 --- a/packages/block-editor/src/components/block-switcher/test/index.js +++ b/packages/block-editor/src/components/block-switcher/test/index.js @@ -188,9 +188,7 @@ describe( 'BlockSwitcher', () => { isOpen: false, } ) ); - const iconButtonClosed = toggleClosed.find( - 'ForwardRef(Button)' - ); + const iconButtonClosed = toggleClosed.find( 'ToolbarButton' ); iconButtonClosed.simulate( 'keydown', mockKeyDown ); @@ -206,7 +204,7 @@ describe( 'BlockSwitcher', () => { isOpen: true, } ) ); - const iconButtonOpen = toggleOpen.find( 'ForwardRef(Button)' ); + const iconButtonOpen = toggleOpen.find( 'ToolbarButton' ); iconButtonOpen.simulate( 'keydown', mockKeyDown ); diff --git a/packages/block-editor/src/components/block-vertical-alignment-toolbar/index.js b/packages/block-editor/src/components/block-vertical-alignment-toolbar/index.js index ee48eae9ee5ede..e19de08f818292 100644 --- a/packages/block-editor/src/components/block-vertical-alignment-toolbar/index.js +++ b/packages/block-editor/src/components/block-vertical-alignment-toolbar/index.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { _x } from '@wordpress/i18n'; -import { Toolbar } from '@wordpress/components'; +import { ToolbarGroup } from '@wordpress/components'; /** * Internal dependencies @@ -48,7 +48,7 @@ export function BlockVerticalAlignmentToolbar( { BLOCK_ALIGNMENTS_CONTROLS[ DEFAULT_CONTROL ]; return ( - Date: Wed, 29 Apr 2020 01:02:21 -0300 Subject: [PATCH 22/46] Fix some e2e tests --- packages/components/src/toolbar-button/index.js | 16 ++++++++++++---- .../various/keyboard-navigable-blocks.test.js | 8 ++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/components/src/toolbar-button/index.js b/packages/components/src/toolbar-button/index.js index c88057899b16cd..7e84e89cc99315 100644 --- a/packages/components/src/toolbar-button/index.js +++ b/packages/components/src/toolbar-button/index.js @@ -21,6 +21,9 @@ function ToolbarButton( { className, extraProps, children, + title, + isActive, + isDisabled, ...props } ) { const accessibleToolbarState = useContext( ToolbarContext ); @@ -32,7 +35,7 @@ function ToolbarButton( { ) } diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js index 2f54b616ceef1d..3f590f4741f12b 100644 --- a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js +++ b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js @@ -29,7 +29,6 @@ const tabThroughParagraphBlock = async ( paragraphText ) => { await page.keyboard.press( 'Tab' ); await expect( await getActiveLabel() ).toBe( 'Add block' ); - await tabThroughBlockMoverControl(); await tabThroughBlockToolbar(); await page.keyboard.press( 'Tab' ); @@ -42,15 +41,13 @@ const tabThroughParagraphBlock = async ( paragraphText ) => { await expect( await getActiveLabel() ).toBe( 'Open document settings' ); }; -const tabThroughBlockMoverControl = async () => { +const tabThroughBlockToolbar = async () => { await page.keyboard.press( 'Tab' ); await expect( await getActiveLabel() ).toBe( 'Move up' ); await page.keyboard.press( 'ArrowRight' ); await expect( await getActiveLabel() ).toBe( 'Move down' ); -}; -const tabThroughBlockToolbar = async () => { await page.keyboard.press( 'ArrowRight' ); await expect( await getActiveLabel() ).toBe( 'Change block type or style' ); @@ -71,6 +68,9 @@ const tabThroughBlockToolbar = async () => { await page.keyboard.press( 'ArrowRight' ); await expect( await getActiveLabel() ).toBe( 'More options' ); + + await page.keyboard.press( 'ArrowRight' ); + await expect( await getActiveLabel() ).toBe( 'Move up' ); }; describe( 'Order of block keyboard navigation', () => { From 11b310410ac914f84e9635847519acd917d6d681 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 03:51:57 -0300 Subject: [PATCH 23/46] Update NavigableToolbar --- .../src/components/navigable-toolbar/index.js | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 3811648c741336..4c977f389672fe 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -2,7 +2,13 @@ * WordPress dependencies */ import { NavigableMenu, Toolbar } from '@wordpress/components'; -import { useState, useRef, useEffect, useCallback } from '@wordpress/element'; +import { + useState, + useRef, + useLayoutEffect, + useEffect, + useCallback, +} from '@wordpress/element'; import { focus } from '@wordpress/dom'; import { useShortcut } from '@wordpress/keyboard-shortcuts'; @@ -11,22 +17,20 @@ function useShouldDisplayAccessibleToolbar( ref ) { shouldDisplayAccessibleToolbar, setShouldDisplayAccessibleToolbar, ] = useState( false ); - useEffect( () => { + useLayoutEffect( () => { const wrapper = ref.current; - window.requestAnimationFrame( () => { - const focusables = focus.focusable.find( wrapper ); - const notToolbarItem = focusables.find( - ( focusable ) => - ! ( 'experimentalToolbarItem' in focusable.dataset ) - ); - if ( ! notToolbarItem ) { - setShouldDisplayAccessibleToolbar( true ); - } - } ); + const tabbables = focus.tabbable.find( wrapper ); + const notToolbarItem = tabbables.some( + ( tabbable ) => ! ( 'experimentalToolbarItem' in tabbable.dataset ) + ); + if ( ! notToolbarItem ) { + setShouldDisplayAccessibleToolbar( true ); + } } ); return shouldDisplayAccessibleToolbar; } +// TODO: Opening Change Alignment Text makes it not be the last child function NavigableToolbar( { children, focusOnMount, ...props } ) { const wrapper = useRef(); const shouldDisplayAccessibleToolbar = useShouldDisplayAccessibleToolbar( @@ -34,15 +38,19 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { ); const focusToolbar = useCallback( () => { - const tabbables = focus.tabbable.find( wrapper.current ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } + window.requestAnimationFrame( () => { + const tabbables = focus.tabbable.find( wrapper.current ); + if ( tabbables.length ) { + tabbables[ 0 ].focus(); + } + } ); }, [] ); + useShortcut( 'core/block-editor/focus-toolbar', focusToolbar, { bindGlobal: true, eventName: 'keydown', } ); + useEffect( () => { if ( focusOnMount ) { focusToolbar(); From c87d1f23e875abcaf9aa4eac842061c3c996b6a8 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 06:19:26 -0300 Subject: [PATCH 24/46] Attempt to fix e2e tests --- .../block-editor/src/components/navigable-toolbar/index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 4c977f389672fe..c9099a44218a2d 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -40,8 +40,15 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { const focusToolbar = useCallback( () => { window.requestAnimationFrame( () => { const tabbables = focus.tabbable.find( wrapper.current ); + // TODO: Oh! Find another way to do this if ( tabbables.length ) { tabbables[ 0 ].focus(); + window.requestAnimationFrame( () => { + tabbables[ 0 ].focus(); + window.requestAnimationFrame( () => { + tabbables[ 0 ].focus(); + } ); + } ); } } ); }, [] ); From 379409186de7cdd5c18a8ae6431b3fa3b57c66de Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 06:34:40 -0300 Subject: [PATCH 25/46] Revert Attempt to fix e2e tests --- .../block-editor/src/components/navigable-toolbar/index.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index c9099a44218a2d..4c977f389672fe 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -40,15 +40,8 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { const focusToolbar = useCallback( () => { window.requestAnimationFrame( () => { const tabbables = focus.tabbable.find( wrapper.current ); - // TODO: Oh! Find another way to do this if ( tabbables.length ) { tabbables[ 0 ].focus(); - window.requestAnimationFrame( () => { - tabbables[ 0 ].focus(); - window.requestAnimationFrame( () => { - tabbables[ 0 ].focus(); - } ); - } ); } } ); }, [] ); From 51b11539380295f83121bb24e40dfdee8b2acd0a Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 06:40:13 -0300 Subject: [PATCH 26/46] Fix navigable-toolbar tests --- .../src/components/navigable-toolbar/index.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 4c977f389672fe..b4bdb03c806227 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -38,12 +38,10 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { ); const focusToolbar = useCallback( () => { - window.requestAnimationFrame( () => { - const tabbables = focus.tabbable.find( wrapper.current ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } - } ); + const tabbables = focus.focusable.find( wrapper.current ); + if ( tabbables.length ) { + tabbables[ 0 ].focus(); + } }, [] ); useShortcut( 'core/block-editor/focus-toolbar', focusToolbar, { @@ -55,7 +53,7 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { if ( focusOnMount ) { focusToolbar(); } - }, [] ); + }, [ shouldDisplayAccessibleToolbar ] ); if ( shouldDisplayAccessibleToolbar ) { return ( From c8f4773c65bfd13d5a5b82b175b3c20ba498aa93 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 07:19:16 -0300 Subject: [PATCH 27/46] Upgrade reakit --- package-lock.json | 52 +++++++++---------- .../src/components/navigable-toolbar/index.js | 2 +- packages/components/package.json | 2 +- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/package-lock.json b/package-lock.json index 149f6c5168c811..87d04fefc01e7d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6198,9 +6198,9 @@ } }, "@popperjs/core": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.1.1.tgz", - "integrity": "sha512-sLqWxCzC5/QHLhziXSCAksBxHfOnQlhPRVgPK0egEw+ktWvG75T2k+aYWVjVh9+WKeT3tlG3ZNbZQvZLmfuOIw==" + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/@popperjs/core/-/core-2.4.0.tgz", + "integrity": "sha512-NMrDy6EWh9TPdSRiHmHH2ye1v5U0gBD7pRYwSwJvomx7Bm4GG04vu63dYiVzebLOx2obPpJugew06xVP0Nk7hA==" }, "@reach/router": { "version": "1.2.1", @@ -9987,7 +9987,7 @@ "re-resizable": "^6.0.0", "react-dates": "^17.1.1", "react-spring": "^8.0.20", - "reakit": "^1.0.0-rc.0", + "reakit": "^1.0.0-rc.2", "rememo": "^3.0.0", "tinycolor2": "^1.4.1", "uuid": "^7.0.2" @@ -13564,9 +13564,9 @@ } }, "body-scroll-lock": { - "version": "2.7.1", - "resolved": "https://registry.npmjs.org/body-scroll-lock/-/body-scroll-lock-2.7.1.tgz", - "integrity": "sha512-hS53SQ8RhM0e4DsQ3PKz6Gr2O7Kpdh59TWU98GHjaQznL7y4dFycEPk7pFQAikqBaUSCArkc5E3pe7CWIt2fZA==" + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/body-scroll-lock/-/body-scroll-lock-3.0.2.tgz", + "integrity": "sha512-PtItUun94iIupKry8J/h6SfRCLWZnly77KbPsTSKALmxfR282L8R0Ujkv7bydSZvLxAJS4sBJ3y/E6X8gYkGrQ==" }, "boolbase": { "version": "1.0.0", @@ -37125,36 +37125,36 @@ } }, "reakit": { - "version": "1.0.0-rc.0", - "resolved": "https://registry.npmjs.org/reakit/-/reakit-1.0.0-rc.0.tgz", - "integrity": "sha512-jG9RfLE9DX3XP6xiUmindu8dJmd4rLs+ohQ2xppF9LVYQ/7Qa9B4kz8mNYbe42u8muE3nMM78T2RfXz+c/ZMsQ==", + "version": "1.0.0-rc.2", + "resolved": "https://registry.npmjs.org/reakit/-/reakit-1.0.0-rc.2.tgz", + "integrity": "sha512-+Snfc9Y/VMozEDXld3CJtvqgTyFCE67FXBmaDcvBuO3z2U/ZAHiAOBprF+ZN/QhsiVNMk1faj791vDBTokxS4w==", "requires": { - "@popperjs/core": "^2.1.0", - "body-scroll-lock": "^2.6.4", - "reakit-system": "^0.10.0", - "reakit-utils": "^0.10.0", - "reakit-warning": "^0.1.0" + "@popperjs/core": "^2.3.3", + "body-scroll-lock": "^3.0.1", + "reakit-system": "^0.12.0", + "reakit-utils": "^0.12.0", + "reakit-warning": "^0.3.0" } }, "reakit-system": { - "version": "0.10.0", - "resolved": "https://registry.npmjs.org/reakit-system/-/reakit-system-0.10.0.tgz", - "integrity": "sha512-73ZI50NB2A6WAF3OsPJEEz73fax5cFiMoGMx3KxPT/AcS39rPqlBW6QkawtZC1HUebQXlsLxwZWicoFt8UubmQ==", + "version": "0.12.0", + "resolved": "https://registry.npmjs.org/reakit-system/-/reakit-system-0.12.0.tgz", + "integrity": "sha512-lqesr7jmOEDEqKtPhEilCJNfYiLCeXOeU2lpPLBzO+CB2lsfF+A/NtrSpb3kuLu2mfIUSTTBxgaKdOBbfANmWA==", "requires": { - "reakit-utils": "^0.10.0" + "reakit-utils": "^0.12.0" } }, "reakit-utils": { - "version": "0.10.0", - "resolved": "https://registry.npmjs.org/reakit-utils/-/reakit-utils-0.10.0.tgz", - "integrity": "sha512-s1+nqLYrHo54U38iETdY86+VD+CZBTqF9rxMmphuft1Iz1i+L+OqOVJMq5sviBkTiEz8zRMhrNLcjBERFiPnkA==" + "version": "0.12.0", + "resolved": "https://registry.npmjs.org/reakit-utils/-/reakit-utils-0.12.0.tgz", + "integrity": "sha512-B0KUjRDu0GFDTi+FQApm4gynJGn18DuDdgCtcUytkN/AIJdKGaqHJ6FpeE1zMr1KAAUzZKrRqq/x93MrcQtvfQ==" }, "reakit-warning": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/reakit-warning/-/reakit-warning-0.1.0.tgz", - "integrity": "sha512-nfujYGWoZ1lh6eAFTVQc2aNjrAEf30PHffJw8Q8tiJJY4Knoy7eLA4jQGHTl3gOjhA9+Yd8KSmiLoOPlr6A0kA==", + "version": "0.3.0", + "resolved": "https://registry.npmjs.org/reakit-warning/-/reakit-warning-0.3.0.tgz", + "integrity": "sha512-sJhgKQl6b4BZOo8jAXpneYFuAkx4wuftGl5KiIDAQZWg+e8YfB41QayjqM2eh0mQkG13hbc4pBOAyR5oFZxK0w==", "requires": { - "reakit-utils": "^0.10.0" + "reakit-utils": "^0.12.0" } }, "realpath-native": { diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index b4bdb03c806227..fa4baa20ce4f98 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -38,7 +38,7 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { ); const focusToolbar = useCallback( () => { - const tabbables = focus.focusable.find( wrapper.current ); + const tabbables = focus.tabbable.find( wrapper.current ); if ( tabbables.length ) { tabbables[ 0 ].focus(); } diff --git a/packages/components/package.json b/packages/components/package.json index 4beac832581982..7bdd28dcbc1814 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -53,7 +53,7 @@ "re-resizable": "^6.0.0", "react-dates": "^17.1.1", "react-spring": "^8.0.20", - "reakit": "^1.0.0-rc.0", + "reakit": "^1.0.0-rc.2", "rememo": "^3.0.0", "tinycolor2": "^1.4.1", "uuid": "^7.0.2" From 1bb519c83dd4195a4aaf130307a0bddf4734a52f Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 08:22:37 -0300 Subject: [PATCH 28/46] Fix e2e tests --- .../src/components/block-toolbar/utils.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-toolbar/utils.js b/packages/block-editor/src/components/block-toolbar/utils.js index 2ee4a65b0d43f5..638e2facde790f 100644 --- a/packages/block-editor/src/components/block-toolbar/utils.js +++ b/packages/block-editor/src/components/block-toolbar/utils.js @@ -8,7 +8,12 @@ import { noop } from 'lodash'; import { useDispatch } from '@wordpress/data'; import { useState, useRef, useEffect, useCallback } from '@wordpress/element'; -const { clearTimeout, requestAnimationFrame, setTimeout } = window; +const { + clearTimeout, + requestAnimationFrame, + cancelAnimationFrame, + setTimeout, +} = window; const DEBOUNCE_TIMEOUT = 250; /** @@ -153,6 +158,8 @@ export function useShowMoversGestures( { }; } +let requestAnimationFrameId; + /** * Hook that toggles the highlight (outline) state of a block * @@ -171,10 +178,14 @@ export function useToggleBlockHighlight( clientId ) { ); useEffect( () => { + // TODO: Explain + if ( requestAnimationFrameId ) { + cancelAnimationFrame( requestAnimationFrameId ); + } return () => { // Sequences state change to enable editor updates (e.g. cursor // position) to render correctly. - requestAnimationFrame( () => { + requestAnimationFrameId = requestAnimationFrame( () => { updateBlockHighlight( false ); } ); }; From 83f7eba558066a717c3ca10894268ab11859080b Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 29 Apr 2020 08:48:15 -0300 Subject: [PATCH 29/46] Update styles --- packages/edit-navigation/src/components/menu-editor/style.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/edit-navigation/src/components/menu-editor/style.scss b/packages/edit-navigation/src/components/menu-editor/style.scss index aa7a3d98b33f55..8b76eeb8f1d6ed 100644 --- a/packages/edit-navigation/src/components/menu-editor/style.scss +++ b/packages/edit-navigation/src/components/menu-editor/style.scss @@ -19,6 +19,7 @@ border: 1px solid #e2e4e7; // Borders around toolbar segments. + .components-toolbar-group, .components-toolbar { background: none; // IE11 has thick paddings without this. From 750914c85dc0ec4c0aaa548045ef5e90eb045d90 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 01:13:10 -0300 Subject: [PATCH 30/46] Update ToolbarGroup styles --- packages/components/src/toolbar-group/style.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/toolbar-group/style.scss b/packages/components/src/toolbar-group/style.scss index a804663d4ef793..6882c190e1a775 100644 --- a/packages/components/src/toolbar-group/style.scss +++ b/packages/components/src/toolbar-group/style.scss @@ -104,7 +104,7 @@ div.components-toolbar { } // Single buttons should remain 48px. -.components-accessible-toolbar .components-toolbar-group > .components-button:first-child:last-child.has-icon, +.components-accessible-toolbar .components-toolbar-group > .components-button:first-of-type:last-of-type, .components-toolbar div:first-child:last-child > .components-button.has-icon { min-width: $block-toolbar-height; padding-left: $grid-unit-15; From 47ceb493c75769d55b7888e4e9a05042c428edf0 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 01:13:30 -0300 Subject: [PATCH 31/46] Update NavigableToolbar --- .../src/components/navigable-toolbar/index.js | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index fa4baa20ce4f98..d9c460a5ab04ef 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -12,33 +12,30 @@ import { import { focus } from '@wordpress/dom'; import { useShortcut } from '@wordpress/keyboard-shortcuts'; -function useShouldDisplayAccessibleToolbar( ref ) { - const [ - shouldDisplayAccessibleToolbar, - setShouldDisplayAccessibleToolbar, - ] = useState( false ); +function useIsAccessibleToolbar( ref ) { + // By default, it's gonna render NavigableMenu. If all the tabbable elements + // inside the toolbar are ToolbarItem components (or derived components like + // ToolbarButton), then we can wrap them with the accessible Toolbar + // component. + const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState( false ); useLayoutEffect( () => { - const wrapper = ref.current; - const tabbables = focus.tabbable.find( wrapper ); + const tabbables = focus.tabbable.find( ref.current ); const notToolbarItem = tabbables.some( ( tabbable ) => ! ( 'experimentalToolbarItem' in tabbable.dataset ) ); if ( ! notToolbarItem ) { - setShouldDisplayAccessibleToolbar( true ); + setIsAccessibleToolbar( true ); } } ); - return shouldDisplayAccessibleToolbar; + return isAccessibleToolbar; } -// TODO: Opening Change Alignment Text makes it not be the last child -function NavigableToolbar( { children, focusOnMount, ...props } ) { - const wrapper = useRef(); - const shouldDisplayAccessibleToolbar = useShouldDisplayAccessibleToolbar( - wrapper - ); +function useToolbarFocus( ref, focusOnMount, isAccessibleToolbar ) { + // Make sure we don't use modified versions of this prop + const [ initialFocusOnMount ] = useState( focusOnMount ); const focusToolbar = useCallback( () => { - const tabbables = focus.tabbable.find( wrapper.current ); + const tabbables = focus.tabbable.find( ref.current ); if ( tabbables.length ) { tabbables[ 0 ].focus(); } @@ -50,12 +47,19 @@ function NavigableToolbar( { children, focusOnMount, ...props } ) { } ); useEffect( () => { - if ( focusOnMount ) { + if ( initialFocusOnMount ) { focusToolbar(); } - }, [ shouldDisplayAccessibleToolbar ] ); + }, [ isAccessibleToolbar, initialFocusOnMount, focusToolbar ] ); +} + +function NavigableToolbar( { children, focusOnMount, ...props } ) { + const wrapper = useRef(); + const isAccessibleToolbar = useIsAccessibleToolbar( wrapper ); + + useToolbarFocus( wrapper, focusOnMount, isAccessibleToolbar ); - if ( shouldDisplayAccessibleToolbar ) { + if ( isAccessibleToolbar ) { return ( Date: Thu, 30 Apr 2020 01:32:34 -0300 Subject: [PATCH 32/46] Refactor Block Controlls Fill/Slot code --- .../src/components/block-controls/index.js | 27 ++++++++++++------- .../components/block-format-controls/index.js | 24 ++++++++++------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/block-editor/src/components/block-controls/index.js b/packages/block-editor/src/components/block-controls/index.js index c2c5601bc8970e..31b18be0e203b5 100644 --- a/packages/block-editor/src/components/block-controls/index.js +++ b/packages/block-editor/src/components/block-controls/index.js @@ -1,10 +1,15 @@ +/** + * External dependencies + */ +import { isEmpty } from 'lodash'; + /** * WordPress dependencies */ import { useContext } from '@wordpress/element'; import { - createSlotFill, __experimentalToolbarContext as ToolbarContext, + createSlotFill, ToolbarGroup, } from '@wordpress/components'; @@ -29,14 +34,18 @@ function BlockControlsSlot( props ) { function BlockControlsFill( { controls, children } ) { return ( - { ( fillProps ) => ( - - - { children } - - ) } + { ( fillProps ) => { + // Children passed to BlockControlsFill will not have access to any + // React Context whose Provider is part of the BlockControlsSlot tree. + // So we re-create the Provider in this subtree. + const value = ! isEmpty( fillProps ) ? fillProps : null; + return ( + + + { children } + + ); + } } ); } diff --git a/packages/block-editor/src/components/block-format-controls/index.js b/packages/block-editor/src/components/block-format-controls/index.js index 218e5d38a1970e..cffddea6c7249a 100644 --- a/packages/block-editor/src/components/block-format-controls/index.js +++ b/packages/block-editor/src/components/block-format-controls/index.js @@ -1,10 +1,15 @@ +/** + * External dependencies + */ +import { isEmpty } from 'lodash'; + /** * WordPress dependencies */ import { useContext } from '@wordpress/element'; import { - createSlotFill, __experimentalToolbarContext as ToolbarContext, + createSlotFill, } from '@wordpress/components'; /** @@ -27,14 +32,15 @@ function BlockFormatControlsSlot( props ) { function BlockFormatControlsFill( props ) { return ( - - { ( fillProps ) => ( - - { props.children } - - ) } + + { ( fillProps ) => { + const value = ! isEmpty( fillProps ) ? fillProps : null; + return ( + + { props.children } + + ); + } } ); } From 12477f962af248aae5b73f6c5a9e8761ada5105f Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 02:05:02 -0300 Subject: [PATCH 33/46] Update NavigableToolbar --- .../src/components/navigable-toolbar/index.js | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index d9c460a5ab04ef..ae21e8a6b95341 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -12,21 +12,42 @@ import { import { focus } from '@wordpress/dom'; import { useShortcut } from '@wordpress/keyboard-shortcuts'; +function hasOnlyToolbarItem( elements ) { + const dataProp = 'experimentalToolbarItem'; + return ! elements.some( ( element ) => ! ( dataProp in element.dataset ) ); +} + +function applyOnNextRepaint( callback ) { + const id = window.requestAnimationFrame( callback ); + return () => window.cancelAnimationFrame( id ); +} + +function focusFirstTabbableIn( container ) { + const [ firstTabbable ] = focus.tabbable.find( container ); + if ( firstTabbable ) { + firstTabbable.focus(); + } +} + function useIsAccessibleToolbar( ref ) { // By default, it's gonna render NavigableMenu. If all the tabbable elements // inside the toolbar are ToolbarItem components (or derived components like // ToolbarButton), then we can wrap them with the accessible Toolbar // component. const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState( false ); - useLayoutEffect( () => { + + const determineIsAccessibleToolbar = useCallback( () => { const tabbables = focus.tabbable.find( ref.current ); - const notToolbarItem = tabbables.some( - ( tabbable ) => ! ( 'experimentalToolbarItem' in tabbable.dataset ) - ); - if ( ! notToolbarItem ) { - setIsAccessibleToolbar( true ); - } - } ); + setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) ); + }, [] ); + + useLayoutEffect( () => { + determineIsAccessibleToolbar(); + // We do an additional check on re-paint because some custom block toolbar + // buttons aren't there in the first paint. + return applyOnNextRepaint( determineIsAccessibleToolbar ); + }, [] ); + return isAccessibleToolbar; } @@ -35,10 +56,7 @@ function useToolbarFocus( ref, focusOnMount, isAccessibleToolbar ) { const [ initialFocusOnMount ] = useState( focusOnMount ); const focusToolbar = useCallback( () => { - const tabbables = focus.tabbable.find( ref.current ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } + focusFirstTabbableIn( ref.current ); }, [] ); useShortcut( 'core/block-editor/focus-toolbar', focusToolbar, { From 67289f707546cd0b52bf211060aac0a1996a9597 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 02:05:21 -0300 Subject: [PATCH 34/46] Update styles --- packages/block-editor/src/components/block-list/style.scss | 4 ---- packages/block-editor/src/components/block-mover/style.scss | 5 ----- 2 files changed, 9 deletions(-) diff --git a/packages/block-editor/src/components/block-list/style.scss b/packages/block-editor/src/components/block-list/style.scss index e843451fa2fc87..01cbe5f1bf1dd0 100644 --- a/packages/block-editor/src/components/block-list/style.scss +++ b/packages/block-editor/src/components/block-list/style.scss @@ -1,7 +1,3 @@ -// .block-editor-block-contextual-toolbar { -// display: block; -// } - .block-editor-block-list__layout .block-editor-block-list__block { // Needs specificity to override inherited styles. // While block is being dragged, dim the slot dragged from, and hide some UI. &.is-dragging { diff --git a/packages/block-editor/src/components/block-mover/style.scss b/packages/block-editor/src/components/block-mover/style.scss index d03bf648879155..eedcb1344dbcef 100644 --- a/packages/block-editor/src/components/block-mover/style.scss +++ b/packages/block-editor/src/components/block-mover/style.scss @@ -8,7 +8,6 @@ } .block-editor-block-mover__control.has-icon { - height: auto; padding: 0; } @@ -20,10 +19,6 @@ flex: 1; } - .components-toolbar > div { - flex: 1; - } - &.is-horizontal .components-toolbar-group, &.is-horizontal .components-toolbar { flex-direction: row; From 2d01cda47085d7f08eafbdc85f9e136cc361cd36 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 02:12:54 -0300 Subject: [PATCH 35/46] Add comment on block-toolbar/utils.js --- packages/block-editor/src/components/block-toolbar/utils.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/block-toolbar/utils.js b/packages/block-editor/src/components/block-toolbar/utils.js index 638e2facde790f..a33c6240b2f958 100644 --- a/packages/block-editor/src/components/block-toolbar/utils.js +++ b/packages/block-editor/src/components/block-toolbar/utils.js @@ -178,7 +178,9 @@ export function useToggleBlockHighlight( clientId ) { ); useEffect( () => { - // TODO: Explain + // On mount, we make sure to cancel any pending animation frame request + // that hasn't been completed yet. Components like NavigableToolbar may + // mount and unmount quickly. if ( requestAnimationFrameId ) { cancelAnimationFrame( requestAnimationFrameId ); } From 70937da71e8a44a71a82e7814e6eefe7b6104c8e Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 02:19:09 -0300 Subject: [PATCH 36/46] Refactor format-toolbar code --- .../src/components/rich-text/format-toolbar/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/format-toolbar/index.js b/packages/block-editor/src/components/rich-text/format-toolbar/index.js index a7d9d49d8cc353..e0c35b11cdae50 100644 --- a/packages/block-editor/src/components/rich-text/format-toolbar/index.js +++ b/packages/block-editor/src/components/rich-text/format-toolbar/index.js @@ -10,10 +10,10 @@ import { orderBy } from 'lodash'; import { __ } from '@wordpress/i18n'; import { - ToolbarGroup, __experimentalToolbarItem as ToolbarItem, - Slot, + ToolbarGroup, DropdownMenu, + Slot, } from '@wordpress/components'; import { chevronDown } from '@wordpress/icons'; From 5bb5f499fbf990d6ceedc53043e93f0b4c96ca2c Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 23:19:14 -0300 Subject: [PATCH 37/46] Fix styles on horizontal movers --- .../src/components/block-mover/style.scss | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/block-mover/style.scss b/packages/block-editor/src/components/block-mover/style.scss index eedcb1344dbcef..dbf1b05dcab611 100644 --- a/packages/block-editor/src/components/block-mover/style.scss +++ b/packages/block-editor/src/components/block-mover/style.scss @@ -29,7 +29,8 @@ width: $block-toolbar-height; padding: 0 !important; - // Focys style. + // Focus style. + // Overrides .components-toolbar-group styles &::before { left: $grid-unit-10 !important; right: $grid-unit-10 !important; @@ -67,8 +68,9 @@ &.is-horizontal { .block-editor-block-mover__control.has-icon { height: $block-toolbar-height; + // Overrides .components-toolbar-group styles width: $block-toolbar-height/2 !important; - min-width: $grid-unit-30; + min-width: $grid-unit-30 !important; padding-left: 0; padding-right: 0; @@ -91,9 +93,10 @@ } // Focus style. + // Overrides .components-toolbar-group styles &::before { - left: $grid-unit-10; - right: 0; + left: $grid-unit-10 !important; + right: 0 !important; } } @@ -105,9 +108,10 @@ } // Focus style. + // Overrides .components-toolbar-group styles &::before { - left: 0; - right: $grid-unit-10; + left: 0 !important; + right: $grid-unit-10 !important; } } } From 9b600751f9c18a2da371ae415eccda23c838df7b Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 30 Apr 2020 23:36:56 -0300 Subject: [PATCH 38/46] Update trailing dropdown toolbar button styles --- packages/components/src/toolbar-group/style.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/components/src/toolbar-group/style.scss b/packages/components/src/toolbar-group/style.scss index 6882c190e1a775..fa1f90b957d011 100644 --- a/packages/components/src/toolbar-group/style.scss +++ b/packages/components/src/toolbar-group/style.scss @@ -79,6 +79,7 @@ div.components-toolbar { // First button in a group. .components-accessible-toolbar .components-toolbar-group > .components-button:first-child.has-icon, +.components-accessible-toolbar .components-toolbar-group > div:first-child > .components-button.has-icon, .components-toolbar div:first-child .components-button.has-icon { min-width: $block-toolbar-height - $grid-unit-15 / 2; padding-left: $grid-unit-15 - $border-width; @@ -92,6 +93,7 @@ div.components-toolbar { // Last button in a group. .components-accessible-toolbar .components-toolbar-group > .components-button:last-child.has-icon, +.components-accessible-toolbar .components-toolbar-group > div:last-child > .components-button.has-icon, .components-toolbar div:last-child .components-button.has-icon { min-width: $block-toolbar-height - $grid-unit-15 / 2; padding-left: $grid-unit-15 / 2; @@ -104,7 +106,8 @@ div.components-toolbar { } // Single buttons should remain 48px. -.components-accessible-toolbar .components-toolbar-group > .components-button:first-of-type:last-of-type, +.components-accessible-toolbar .components-toolbar-group > .components-button:first-of-type:last-of-type.has-icon, +.components-accessible-toolbar .components-toolbar-group > div:first-child:last-child > .components-button.has-icon, .components-toolbar div:first-child:last-child > .components-button.has-icon { min-width: $block-toolbar-height; padding-left: $grid-unit-15; From 0709c0419d1cabbff5ad84954e764369db134bf6 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 1 May 2020 00:13:51 -0300 Subject: [PATCH 39/46] Use MutationObserver instead of relying on the next repaint --- .../src/components/navigable-toolbar/index.js | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index ae21e8a6b95341..8c8e94a23d658e 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -17,11 +17,6 @@ function hasOnlyToolbarItem( elements ) { return ! elements.some( ( element ) => ! ( dataProp in element.dataset ) ); } -function applyOnNextRepaint( callback ) { - const id = window.requestAnimationFrame( callback ); - return () => window.cancelAnimationFrame( id ); -} - function focusFirstTabbableIn( container ) { const [ firstTabbable ] = focus.tabbable.find( container ); if ( firstTabbable ) { @@ -36,17 +31,16 @@ function useIsAccessibleToolbar( ref ) { // component. const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState( false ); - const determineIsAccessibleToolbar = useCallback( () => { - const tabbables = focus.tabbable.find( ref.current ); - setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) ); - }, [] ); - useLayoutEffect( () => { - determineIsAccessibleToolbar(); - // We do an additional check on re-paint because some custom block toolbar - // buttons aren't there in the first paint. - return applyOnNextRepaint( determineIsAccessibleToolbar ); - }, [] ); + // Toolbar buttons may be rendered asynchronously, so we use + // MutationObserver to check if the toolbar subtree has been modified + const observer = new window.MutationObserver( () => { + const tabbables = focus.tabbable.find( ref.current ); + setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) ); + } ); + observer.observe( ref.current, { childList: true, subtree: true } ); + return () => observer.disconnect(); + }, [ isAccessibleToolbar ] ); return isAccessibleToolbar; } From ed633cc20cf79c54df5e03c2817ed8b0cf3d8270 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 1 May 2020 01:31:21 -0300 Subject: [PATCH 40/46] Fix drag and drop --- packages/components/src/toolbar-item/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/components/src/toolbar-item/index.js b/packages/components/src/toolbar-item/index.js index dcfdca4ea46bc9..a475eb5e8e3c79 100644 --- a/packages/components/src/toolbar-item/index.js +++ b/packages/components/src/toolbar-item/index.js @@ -32,7 +32,11 @@ function ToolbarItem( { children, ...props }, ref ) { return ( - { children } + { ( htmlProps ) => + // Overwriting BaseToolbarItem's onMouseDown since it disables drag + // and drop + children( { ...htmlProps, onMouseDown: allProps.onMouseDown } ) + } ); } From 2399dbd272a37bc329798e9d27989ba4d7e73348 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 1 May 2020 01:51:34 -0300 Subject: [PATCH 41/46] useUpdateLayoutEffect for MutationObserver --- .../src/components/navigable-toolbar/index.js | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 8c8e94a23d658e..fa63839248267e 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -12,6 +12,16 @@ import { import { focus } from '@wordpress/dom'; import { useShortcut } from '@wordpress/keyboard-shortcuts'; +function useUpdateLayoutEffect( effect, deps ) { + const mounted = useRef( false ); + useLayoutEffect( () => { + if ( mounted.current ) { + return effect(); + } + mounted.current = true; + }, deps ); +} + function hasOnlyToolbarItem( elements ) { const dataProp = 'experimentalToolbarItem'; return ! elements.some( ( element ) => ! ( dataProp in element.dataset ) ); @@ -31,13 +41,19 @@ function useIsAccessibleToolbar( ref ) { // component. const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState( false ); - useLayoutEffect( () => { + const determineIsAccessibleToolbar = useCallback( () => { + const tabbables = focus.tabbable.find( ref.current ); + setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) ); + }, [] ); + + useLayoutEffect( determineIsAccessibleToolbar, [] ); + + useUpdateLayoutEffect( () => { // Toolbar buttons may be rendered asynchronously, so we use // MutationObserver to check if the toolbar subtree has been modified - const observer = new window.MutationObserver( () => { - const tabbables = focus.tabbable.find( ref.current ); - setIsAccessibleToolbar( hasOnlyToolbarItem( tabbables ) ); - } ); + const observer = new window.MutationObserver( + determineIsAccessibleToolbar + ); observer.observe( ref.current, { childList: true, subtree: true } ); return () => observer.disconnect(); }, [ isAccessibleToolbar ] ); From 790eb222f0044c9d5356d05774c2b35c77d1d714 Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 1 May 2020 02:08:49 -0300 Subject: [PATCH 42/46] Update NavigableToolbar code --- packages/block-editor/src/components/navigable-toolbar/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index fa63839248267e..86896109b901f4 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -56,7 +56,7 @@ function useIsAccessibleToolbar( ref ) { ); observer.observe( ref.current, { childList: true, subtree: true } ); return () => observer.disconnect(); - }, [ isAccessibleToolbar ] ); + }, [ isAccessibleToolbar, determineIsAccessibleToolbar ] ); return isAccessibleToolbar; } From d5378c534db42885e25f8c56785ebbac8d413f8d Mon Sep 17 00:00:00 2001 From: Haz Date: Fri, 1 May 2020 02:10:06 -0300 Subject: [PATCH 43/46] Update NavigableToolbar code --- packages/block-editor/src/components/navigable-toolbar/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index 86896109b901f4..fa63839248267e 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -56,7 +56,7 @@ function useIsAccessibleToolbar( ref ) { ); observer.observe( ref.current, { childList: true, subtree: true } ); return () => observer.disconnect(); - }, [ isAccessibleToolbar, determineIsAccessibleToolbar ] ); + }, [ isAccessibleToolbar ] ); return isAccessibleToolbar; } From 369f2fedf55ff6e39e109b01fb9b218cda47f063 Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 6 May 2020 05:47:53 -0300 Subject: [PATCH 44/46] Add comment on Toolbar --- packages/components/src/toolbar/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/components/src/toolbar/index.js b/packages/components/src/toolbar/index.js index 523207c37b6b6d..228f73a7e5e453 100644 --- a/packages/components/src/toolbar/index.js +++ b/packages/components/src/toolbar/index.js @@ -41,7 +41,10 @@ function Toolbar( /> ); } - + // When the __experimentalAccessibilityLabel prop is not passed, Toolbar will + // fallback to ToolbarGroup. This should be deprecated as soon as the new API + // gets stable. + // See https://github.com/WordPress/gutenberg/pull/20008#issuecomment-624503410 return ; } From ca40ffd856c5edd80a5c1ee1f83d1de23ad2c6ec Mon Sep 17 00:00:00 2001 From: Haz Date: Wed, 6 May 2020 07:52:19 -0300 Subject: [PATCH 45/46] Move bubblesVirtually prop so React Native still works --- .../src/components/block-controls/index.js | 8 +------- .../src/components/block-format-controls/index.js | 8 +------- .../block-editor/src/components/block-toolbar/index.js | 10 ++++++++-- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/block-controls/index.js b/packages/block-editor/src/components/block-controls/index.js index 31b18be0e203b5..f81c481c1bbba4 100644 --- a/packages/block-editor/src/components/block-controls/index.js +++ b/packages/block-editor/src/components/block-controls/index.js @@ -22,13 +22,7 @@ const { Fill, Slot } = createSlotFill( 'BlockControls' ); function BlockControlsSlot( props ) { const accessibleToolbarState = useContext( ToolbarContext ); - return ( - - ); + return ; } function BlockControlsFill( { controls, children } ) { diff --git a/packages/block-editor/src/components/block-format-controls/index.js b/packages/block-editor/src/components/block-format-controls/index.js index cffddea6c7249a..89f2df3bee260f 100644 --- a/packages/block-editor/src/components/block-format-controls/index.js +++ b/packages/block-editor/src/components/block-format-controls/index.js @@ -21,13 +21,7 @@ const { Fill, Slot } = createSlotFill( 'BlockFormatControls' ); function BlockFormatControlsSlot( props ) { const accessibleToolbarState = useContext( ToolbarContext ); - return ( - - ); + return ; } function BlockFormatControlsFill( props ) { diff --git a/packages/block-editor/src/components/block-toolbar/index.js b/packages/block-editor/src/components/block-toolbar/index.js index 9fb3d0cf7dc30f..d2dc21fff63c20 100644 --- a/packages/block-editor/src/components/block-toolbar/index.js +++ b/packages/block-editor/src/components/block-toolbar/index.js @@ -123,8 +123,14 @@ export default function BlockToolbar( { hideDragHandle } ) {
{ shouldShowVisualToolbar && ! isMultiToolbar && ( <> - - + + ) } From 2b5c5f0933e61e9f4c56bea5aba66f5e03814d39 Mon Sep 17 00:00:00 2001 From: Haz Date: Thu, 7 May 2020 09:59:24 -0300 Subject: [PATCH 46/46] Remove missing comment from styles --- packages/block-editor/src/components/block-mover/style.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-editor/src/components/block-mover/style.scss b/packages/block-editor/src/components/block-mover/style.scss index 4962404bb8e394..427e41189c8073 100644 --- a/packages/block-editor/src/components/block-mover/style.scss +++ b/packages/block-editor/src/components/block-mover/style.scss @@ -24,7 +24,6 @@ flex-direction: row; } - // .block-editor-block-mover__control.has-icon { .block-editor-block-mover-button { height: $block-toolbar-height/2; width: $block-toolbar-height;