From 1fb8658025ab2ebbdd55348429709d3f0dfffcc1 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Sun, 5 Jul 2020 17:19:36 +0300 Subject: [PATCH 1/4] Inserter: fix line to show again --- .../components/block-list/insertion-point.js | 107 ++++++++++-------- 1 file changed, 62 insertions(+), 45 deletions(-) diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js index b1cab6c6d5e00..10305093aa74f 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -33,7 +33,7 @@ function Indicator( { clientId } ) { return ( isBlockInsertionPointVisible() && insertionPoint.index === blockIndex && - insertionPoint.rootClientId === rootClientId + ( insertionPoint.rootClientId || '' ) === rootClientId ); }, [ clientId ] @@ -59,20 +59,30 @@ export default function InsertionPoint( { const [ inserterElement, setInserterElement ] = useState( null ); const [ inserterClientId, setInserterClientId ] = useState( null ); const ref = useRef(); - const { multiSelectedBlockClientIds, isMultiSelecting } = useSelect( - ( select ) => { - const { - getMultiSelectedBlockClientIds, - isMultiSelecting: _isMultiSelecting, - } = select( 'core/block-editor' ); - - return { - isMultiSelecting: _isMultiSelecting(), - multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(), - }; - }, - [] - ); + const { + multiSelectedBlockClientIds, + isMultiSelecting, + isInserterVisible, + selectedClientId, + } = useSelect( ( select ) => { + const { + getMultiSelectedBlockClientIds, + isMultiSelecting: _isMultiSelecting, + isBlockInsertionPointVisible, + getBlockInsertionPoint, + getBlockOrder, + } = select( 'core/block-editor' ); + + const insertionPoint = getBlockInsertionPoint(); + const order = getBlockOrder( insertionPoint.rootClientId ); + + return { + isMultiSelecting: _isMultiSelecting(), + multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(), + isInserterVisible: isBlockInsertionPointVisible(), + selectedClientId: order[ insertionPoint.index ], + }; + }, [] ); function onMouseMove( event ) { if ( @@ -145,14 +155,17 @@ export default function InsertionPoint( { const isInserterHidden = hasMultiSelection ? multiSelectedBlockClientIds.includes( inserterClientId ) : inserterClientId === selectedBlockClientId; + const isVisible = isInserterShown || isInserterForced || isInserterVisible; + const selectedElement = + inserterElement || getBlockDOMNode( selectedClientId ); return ( <> - { ! isMultiSelecting && ( isInserterShown || isInserterForced ) && ( + { ! isMultiSelecting && isVisible && (
- - { /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ } -
setIsInserterForced( true ) } - onBlur={ () => setIsInserterForced( false ) } - onClick={ focusClosestTabbable } - // While ideally it would be enough to capture the - // bubbling focus event from the Inserter, due to the - // characteristics of click focusing of `button`s in - // Firefox and Safari, it is not reliable. - // - // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus - tabIndex={ -1 } - className={ classnames( - 'block-editor-block-list__insertion-point-inserter', - { - 'is-inserter-hidden': isInserterHidden, - } - ) } - > - -
+ + { ( isInserterShown || isInserterForced ) && ( + /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ +
setIsInserterForced( true ) } + onBlur={ () => setIsInserterForced( false ) } + onClick={ focusClosestTabbable } + // While ideally it would be enough to capture the + // bubbling focus event from the Inserter, due to the + // characteristics of click focusing of `button`s in + // Firefox and Safari, it is not reliable. + // + // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus + tabIndex={ -1 } + className={ classnames( + 'block-editor-block-list__insertion-point-inserter', + { + 'is-inserter-hidden': isInserterHidden, + } + ) } + > + +
+ ) }
) } From a79df8e310d096e2a925af33be2c6ddf8eac4da2 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Sun, 5 Jul 2020 21:16:03 +0300 Subject: [PATCH 2/4] Fix client id --- .../src/components/block-list/insertion-point.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js index 10305093aa74f..95e761b37249a 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -30,6 +30,7 @@ function Indicator( { clientId } ) { const rootClientId = getBlockRootClientId( clientId ); const blockIndex = getBlockIndex( clientId, rootClientId ); const insertionPoint = getBlockInsertionPoint(); + return ( isBlockInsertionPointVisible() && insertionPoint.index === blockIndex && @@ -156,8 +157,9 @@ export default function InsertionPoint( { ? multiSelectedBlockClientIds.includes( inserterClientId ) : inserterClientId === selectedBlockClientId; const isVisible = isInserterShown || isInserterForced || isInserterVisible; - const selectedElement = - inserterElement || getBlockDOMNode( selectedClientId ); + const selectedElement = selectedClientId + ? getBlockDOMNode( selectedClientId ) + : inserterElement; return ( <> @@ -176,7 +178,7 @@ export default function InsertionPoint( { style={ { width: selectedElement.offsetWidth } } > { ( isInserterShown || isInserterForced ) && ( /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ From d4bc33eb87bba06c560b0c3ac598fd9ce21bc9c4 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Mon, 20 Jul 2020 16:39:24 +0300 Subject: [PATCH 3/4] Split component --- .../components/block-list/insertion-point.js | 160 ++++++++++-------- 1 file changed, 88 insertions(+), 72 deletions(-) diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js index 95e761b37249a..3e5d7a0eee8f8 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -49,6 +49,84 @@ function Indicator( { clientId } ) { ); } +function InsertionPointPopover( { + clientId, + isInserterShown, + isInserterForced, + setIsInserterForced, + containerRef, + isInserterHidden, +} ) { + const ref = useRef(); + const element = getBlockDOMNode( clientId ); + + function focusClosestTabbable( event ) { + const { clientX, clientY, target } = event; + + // Only handle click on the wrapper specifically, and not an event + // bubbled from the inserter itself. + if ( target !== ref.current ) { + return; + } + + const targetRect = target.getBoundingClientRect(); + const isReverse = clientY < targetRect.top + targetRect.height / 2; + const container = isReverse ? containerRef.current : element; + const closest = + getClosestTabbable( element, true, container ) || element; + const rect = new window.DOMRect( clientX, clientY, 0, 16 ); + + placeCaretAtVerticalEdge( closest, isReverse, rect, false ); + } + + return ( + +
+ + { ( isInserterShown || isInserterForced ) && ( + /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ +
setIsInserterForced( true ) } + onBlur={ () => setIsInserterForced( false ) } + onClick={ focusClosestTabbable } + // While ideally it would be enough to capture the + // bubbling focus event from the Inserter, due to the + // characteristics of click focusing of `button`s in + // Firefox and Safari, it is not reliable. + // + // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus + tabIndex={ -1 } + className={ classnames( + 'block-editor-block-list__insertion-point-inserter', + { + 'is-inserter-hidden': isInserterHidden, + } + ) } + > + +
+ ) } +
+ + ); +} + export default function InsertionPoint( { hasMultiSelection, selectedBlockClientId, @@ -57,9 +135,7 @@ export default function InsertionPoint( { } ) { const [ isInserterShown, setIsInserterShown ] = useState( false ); const [ isInserterForced, setIsInserterForced ] = useState( false ); - const [ inserterElement, setInserterElement ] = useState( null ); const [ inserterClientId, setInserterClientId ] = useState( null ); - const ref = useRef(); const { multiSelectedBlockClientIds, isMultiSelecting, @@ -128,88 +204,28 @@ export default function InsertionPoint( { } setIsInserterShown( true ); - setInserterElement( element ); setInserterClientId( clientId ); } - function focusClosestTabbable( event ) { - const { clientX, clientY, target } = event; - - // Only handle click on the wrapper specifically, and not an event - // bubbled from the inserter itself. - if ( target !== ref.current ) { - return; - } - - const targetRect = target.getBoundingClientRect(); - const isReverse = clientY < targetRect.top + targetRect.height / 2; - const blockNode = getBlockDOMNode( inserterClientId ); - const container = isReverse ? containerRef.current : blockNode; - const closest = - getClosestTabbable( blockNode, true, container ) || blockNode; - const rect = new window.DOMRect( clientX, clientY, 0, 16 ); - - placeCaretAtVerticalEdge( closest, isReverse, rect, false ); - } - // Hide the inserter above the selected block and during multi-selection. const isInserterHidden = hasMultiSelection ? multiSelectedBlockClientIds.includes( inserterClientId ) : inserterClientId === selectedBlockClientId; const isVisible = isInserterShown || isInserterForced || isInserterVisible; - const selectedElement = selectedClientId - ? getBlockDOMNode( selectedClientId ) - : inserterElement; return ( <> { ! isMultiSelecting && isVisible && ( - -
- - { ( isInserterShown || isInserterForced ) && ( - /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ -
setIsInserterForced( true ) } - onBlur={ () => setIsInserterForced( false ) } - onClick={ focusClosestTabbable } - // While ideally it would be enough to capture the - // bubbling focus event from the Inserter, due to the - // characteristics of click focusing of `button`s in - // Firefox and Safari, it is not reliable. - // - // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus - tabIndex={ -1 } - className={ classnames( - 'block-editor-block-list__insertion-point-inserter', - { - 'is-inserter-hidden': isInserterHidden, - } - ) } - > - -
- ) } -
- + ) }
Date: Mon, 20 Jul 2020 17:21:34 +0300 Subject: [PATCH 4/4] Reorganise --- .../components/block-list/insertion-point.js | 187 +++++++++--------- .../components/block-list/root-container.js | 6 +- .../src/components/block-list/style.scss | 2 +- 3 files changed, 91 insertions(+), 104 deletions(-) diff --git a/packages/block-editor/src/components/block-list/insertion-point.js b/packages/block-editor/src/components/block-list/insertion-point.js index 3e5d7a0eee8f8..c7eb0e8d6ee80 100644 --- a/packages/block-editor/src/components/block-list/insertion-point.js +++ b/packages/block-editor/src/components/block-list/insertion-point.js @@ -18,48 +18,30 @@ import Inserter from '../inserter'; import { getClosestTabbable } from '../writing-flow'; import { getBlockDOMNode } from '../../utils/dom'; -function Indicator( { clientId } ) { - const showInsertionPoint = useSelect( +function InsertionPointInserter( { + clientId, + setIsInserterForced, + containerRef, +} ) { + const ref = useRef(); + // Hide the inserter above the selected block and during multi-selection. + const isInserterHidden = useSelect( ( select ) => { const { - getBlockIndex, - getBlockInsertionPoint, - isBlockInsertionPointVisible, - getBlockRootClientId, + getMultiSelectedBlockClientIds, + getSelectedBlockClientId, + hasMultiSelection, } = select( 'core/block-editor' ); - const rootClientId = getBlockRootClientId( clientId ); - const blockIndex = getBlockIndex( clientId, rootClientId ); - const insertionPoint = getBlockInsertionPoint(); + const multiSelectedBlockClientIds = getMultiSelectedBlockClientIds(); + const selectedBlockClientId = getSelectedBlockClientId(); - return ( - isBlockInsertionPointVisible() && - insertionPoint.index === blockIndex && - ( insertionPoint.rootClientId || '' ) === rootClientId - ); + return hasMultiSelection() + ? multiSelectedBlockClientIds.includes( clientId ) + : clientId === selectedBlockClientId; }, [ clientId ] ); - if ( ! showInsertionPoint ) { - return null; - } - - return ( -
- ); -} - -function InsertionPointPopover( { - clientId, - isInserterShown, - isInserterForced, - setIsInserterForced, - containerRef, - isInserterHidden, -} ) { - const ref = useRef(); - const element = getBlockDOMNode( clientId ); - function focusClosestTabbable( event ) { const { clientX, clientY, target } = event; @@ -71,14 +53,55 @@ function InsertionPointPopover( { const targetRect = target.getBoundingClientRect(); const isReverse = clientY < targetRect.top + targetRect.height / 2; - const container = isReverse ? containerRef.current : element; + const blockNode = getBlockDOMNode( clientId ); + const container = isReverse ? containerRef.current : blockNode; const closest = - getClosestTabbable( element, true, container ) || element; + getClosestTabbable( blockNode, true, container ) || blockNode; const rect = new window.DOMRect( clientX, clientY, 0, 16 ); placeCaretAtVerticalEdge( closest, isReverse, rect, false ); } + return ( + /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ +
setIsInserterForced( true ) } + onBlur={ () => setIsInserterForced( false ) } + onClick={ focusClosestTabbable } + // While ideally it would be enough to capture the + // bubbling focus event from the Inserter, due to the + // characteristics of click focusing of `button`s in + // Firefox and Safari, it is not reliable. + // + // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus + tabIndex={ -1 } + className={ classnames( + 'block-editor-block-list__insertion-point-inserter', + { + 'is-inserter-hidden': isInserterHidden, + } + ) } + > + +
+ ); +} + +function InsertionPointPopover( { + clientId, + isInserterShown, + isInserterForced, + setIsInserterForced, + containerRef, + showInsertionPoint, +} ) { + const element = getBlockDOMNode( clientId ); + return ( - + { showInsertionPoint && ( +
+ ) } { ( isInserterShown || isInserterForced ) && ( - /* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */ -
setIsInserterForced( true ) } - onBlur={ () => setIsInserterForced( false ) } - onClick={ focusClosestTabbable } - // While ideally it would be enough to capture the - // bubbling focus event from the Inserter, due to the - // characteristics of click focusing of `button`s in - // Firefox and Safari, it is not reliable. - // - // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus - tabIndex={ -1 } - className={ classnames( - 'block-editor-block-list__insertion-point-inserter', - { - 'is-inserter-hidden': isInserterHidden, - } - ) } - > - -
+ ) }
); } -export default function InsertionPoint( { - hasMultiSelection, - selectedBlockClientId, - children, - containerRef, -} ) { +export default function InsertionPoint( { children, containerRef } ) { const [ isInserterShown, setIsInserterShown ] = useState( false ); const [ isInserterForced, setIsInserterForced ] = useState( false ); const [ inserterClientId, setInserterClientId ] = useState( null ); - const { - multiSelectedBlockClientIds, - isMultiSelecting, - isInserterVisible, - selectedClientId, - } = useSelect( ( select ) => { - const { - getMultiSelectedBlockClientIds, - isMultiSelecting: _isMultiSelecting, - isBlockInsertionPointVisible, - getBlockInsertionPoint, - getBlockOrder, - } = select( 'core/block-editor' ); - - const insertionPoint = getBlockInsertionPoint(); - const order = getBlockOrder( insertionPoint.rootClientId ); - - return { - isMultiSelecting: _isMultiSelecting(), - multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(), - isInserterVisible: isBlockInsertionPointVisible(), - selectedClientId: order[ insertionPoint.index ], - }; - }, [] ); + const { isMultiSelecting, isInserterVisible, selectedClientId } = useSelect( + ( select ) => { + const { + isMultiSelecting: _isMultiSelecting, + isBlockInsertionPointVisible, + getBlockInsertionPoint, + getBlockOrder, + } = select( 'core/block-editor' ); + + const insertionPoint = getBlockInsertionPoint(); + const order = getBlockOrder( insertionPoint.rootClientId ); + + return { + isMultiSelecting: _isMultiSelecting(), + isInserterVisible: isBlockInsertionPointVisible(), + selectedClientId: order[ insertionPoint.index ], + }; + }, + [] + ); function onMouseMove( event ) { if ( @@ -207,10 +202,6 @@ export default function InsertionPoint( { setInserterClientId( clientId ); } - // Hide the inserter above the selected block and during multi-selection. - const isInserterHidden = hasMultiSelection - ? multiSelectedBlockClientIds.includes( inserterClientId ) - : inserterClientId === selectedBlockClientId; const isVisible = isInserterShown || isInserterForced || isInserterVisible; return ( @@ -224,7 +215,7 @@ export default function InsertionPoint( { isInserterForced={ isInserterForced } setIsInserterForced={ setIsInserterForced } containerRef={ containerRef } - isInserterHidden={ isInserterHidden } + showInsertionPoint={ isInserterVisible } /> ) }
+