From 3af3643b781bf9d15e84bb1d580be0c99c8c83f6 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Wed, 27 Nov 2019 11:28:49 +0100 Subject: [PATCH 1/4] Popover: consistently adjust position on scroll --- .../rich-text/format-toolbar-container.js | 29 +-- .../src/components/rich-text/index.js | 2 +- packages/components/src/autocomplete/index.js | 11 +- packages/components/src/popover/index.js | 172 ++++++++++++++---- packages/format-library/src/image/index.js | 28 +-- packages/format-library/src/link/inline.js | 20 +- packages/nux/src/components/dot-tip/index.js | 16 +- .../dot-tip/test/__snapshots__/index.js.snap | 2 +- 8 files changed, 164 insertions(+), 116 deletions(-) diff --git a/packages/block-editor/src/components/rich-text/format-toolbar-container.js b/packages/block-editor/src/components/rich-text/format-toolbar-container.js index fc857311706db3..15fb664ded11d4 100644 --- a/packages/block-editor/src/components/rich-text/format-toolbar-container.js +++ b/packages/block-editor/src/components/rich-text/format-toolbar-container.js @@ -9,31 +9,7 @@ import { Popover } from '@wordpress/components'; import BlockFormatControls from '../block-format-controls'; import FormatToolbar from './format-toolbar'; -function getAnchorRect( anchorObj ) { - const { current } = anchorObj; - const rect = current.getBoundingClientRect(); - - // Add some space. - const buffer = 6; - - // Subtract padding if any. - let { paddingTop } = window.getComputedStyle( current ); - - paddingTop = parseInt( paddingTop, 10 ); - - return { - x: rect.left, - y: rect.top + paddingTop - buffer, - width: rect.width, - height: rect.height - paddingTop + buffer, - left: rect.left, - right: rect.right, - top: rect.top + paddingTop - buffer, - bottom: rect.bottom, - }; -} - -const FormatToolbarContainer = ( { inline, anchorObj } ) => { +const FormatToolbarContainer = ( { inline, anchorRef } ) => { if ( inline ) { // Render in popover return ( @@ -41,7 +17,8 @@ const FormatToolbarContainer = ( { inline, anchorObj } ) => { noArrow position="top center" focusOnMount={ false } - getAnchorRect={ () => getAnchorRect( anchorObj ) } + anchorVerticalBuffer={ 6 } + anchorRef={ anchorRef } className="block-editor-rich-text__inline-format-toolbar" > diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js index 63ceffb8032325..ca047c74673085 100644 --- a/packages/block-editor/src/components/rich-text/index.js +++ b/packages/block-editor/src/components/rich-text/index.js @@ -404,7 +404,7 @@ class RichTextWrapper extends Component { { ( { isSelected, value, onChange, Editable } ) => <> { children && children( { value, onChange } ) } - { isSelected && hasFormats && ( ) } + { isSelected && hasFormats && ( ) } { isSelected && }
{ - if ( ! anchorRef.current ) { - return; - } + let newAnchor = computeAnchorRect( + anchorRefFallback, + anchorRect, + getAnchorRect, + anchorRef, + anchorIncludePadding + ); - let newAnchor; - if ( anchorRect ) { - newAnchor = anchorRect; - } else if ( getAnchorRect ) { - newAnchor = getAnchorRect( anchorRef.current ); - } else { - const rect = anchorRef.current.parentNode.getBoundingClientRect(); - // subtract padding - const { paddingTop, paddingBottom } = window.getComputedStyle( anchorRef.current.parentNode ); - const topPad = parseInt( paddingTop, 10 ); - const bottomPad = parseInt( paddingBottom, 10 ); - newAnchor = { - x: rect.left, - y: rect.top + topPad, - width: rect.width, - height: rect.height - topPad - bottomPad, - left: rect.left, - right: rect.right, - top: rect.top + topPad, - bottom: rect.bottom - bottomPad, - }; - } + newAnchor = addBuffer( + newAnchor, + anchorVerticalBuffer, + anchorHorizontalBuffer + ); - const didAnchorRectChange = ! isShallowEqual( newAnchor, anchor ); - if ( didAnchorRectChange ) { + if ( ! isShallowEqual( newAnchor, anchor ) ) { setAnchor( newAnchor ); } }; @@ -259,6 +346,10 @@ const Popover = ( { position = 'top', range, focusOnMount = 'firstElement', + anchorRef, + anchorIncludePadding, + anchorVerticalBuffer, + anchorHorizontalBuffer, anchorRect, getAnchorRect, expandOnMobile, @@ -268,14 +359,23 @@ const Popover = ( { /* eslint-enable no-unused-vars */ ...contentProps } ) => { - const anchorRef = useRef( null ); + const anchorRefFallback = useRef( null ); const contentRef = useRef( null ); // Animation const [ isReadyToAnimate, setIsReadyToAnimate ] = useState( false ); // Anchor position - const anchor = useAnchor( anchorRef, contentRef, anchorRect, getAnchorRect ); + const anchor = useAnchor( + anchorRefFallback, + contentRef, + anchorRect, + getAnchorRect, + anchorRef, + anchorIncludePadding, + anchorVerticalBuffer, + anchorHorizontalBuffer + ); // Content size const contentSize = useInitialContentSize( contentRef ); @@ -438,7 +538,7 @@ const Popover = ( { } return ( - + { content } { popoverPosition.isMobile && expandOnMobile && } diff --git a/packages/format-library/src/image/index.js b/packages/format-library/src/image/index.js index 15cea7ecee4e33..c7a46a804b6367 100644 --- a/packages/format-library/src/image/index.js +++ b/packages/format-library/src/image/index.js @@ -3,11 +3,10 @@ */ import { Path, SVG, TextControl, Popover, IconButton } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { Component, useMemo } from '@wordpress/element'; +import { Component } from '@wordpress/element'; import { insertObject } from '@wordpress/rich-text'; import { MediaUpload, RichTextToolbarButton, MediaUploadCheck } from '@wordpress/block-editor'; import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; -import { computeCaretRect } from '@wordpress/dom'; const ALLOWED_MEDIA_TYPES = [ 'image' ]; @@ -16,16 +15,10 @@ const title = __( 'Inline image' ); const stopKeyPropagation = ( event ) => event.stopPropagation(); -const PopoverAtImage = ( { dependencies, ...props } ) => { - return ( - computeCaretRect(), dependencies ) } - { ...props } - /> - ); -}; +function getRange() { + const selection = window.getSelection(); + return selection.rangeCount ? selection.getRangeAt( 0 ) : null; +} export const image = { name, @@ -93,7 +86,6 @@ export const image = { render() { const { value, onChange, isObjectActive, activeObjectAttributes } = this.props; - const { style } = activeObjectAttributes; return ( @@ -124,10 +116,10 @@ export const image = { } } /> } { isObjectActive && - { // Disable reason: KeyPress must be suppressed so the block doesn't hide the toolbar /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ } @@ -165,7 +157,7 @@ export const image = { { /* eslint-enable jsx-a11y/no-noninteractive-element-interactions */ } - + } ); diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index bf1784db2132d7..34c05ad432c0cf 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -8,7 +8,6 @@ import { withSpokenMessages, } from '@wordpress/components'; import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; -import { getRectangleFromRange } from '@wordpress/dom'; import { prependHTTP } from '@wordpress/url'; import { create, @@ -32,15 +31,17 @@ function isShowingInput( props, state ) { } const URLPopoverAtLink = ( { isActive, addingLink, value, ...props } ) => { - const anchorRect = useMemo( () => { + const anchorRef = useMemo( () => { const selection = window.getSelection(); - const range = selection.rangeCount > 0 ? selection.getRangeAt( 0 ) : null; - if ( ! range ) { + + if ( ! selection.rangeCount ) { return; } + const range = selection.getRangeAt( 0 ); + if ( addingLink ) { - return getRectangleFromRange( range ); + return range; } let element = range.startContainer; @@ -52,17 +53,14 @@ const URLPopoverAtLink = ( { isActive, addingLink, value, ...props } ) => { element = element.parentNode; } - const closest = element.closest( 'a' ); - if ( closest ) { - return closest.getBoundingClientRect(); - } + return element.closest( 'a' ); }, [ isActive, addingLink, value.start, value.end ] ); - if ( ! anchorRect ) { + if ( ! anchorRef ) { return null; } - return ; + return ; }; class InlineLinkUI extends Component { diff --git a/packages/nux/src/components/dot-tip/index.js b/packages/nux/src/components/dot-tip/index.js index ed8119cb0d1158..48bad4d0ae0c71 100644 --- a/packages/nux/src/components/dot-tip/index.js +++ b/packages/nux/src/components/dot-tip/index.js @@ -7,13 +7,6 @@ import { __ } from '@wordpress/i18n'; import { withSelect, withDispatch } from '@wordpress/data'; import { useCallback, useRef } from '@wordpress/element'; -function getAnchorRect( anchor ) { - // The default getAnchorRect() excludes an element's top and bottom padding - // from its calculation. We want tips to point to the outer margin of an - // element, so we override getAnchorRect() to include all padding. - return anchor.parentNode.getBoundingClientRect(); -} - function onClick( event ) { // Tips are often nested within buttons. We stop propagation so that clicking // on a tip doesn't result in the button being clicked. @@ -29,13 +22,6 @@ export function DotTip( { onDisable, } ) { const anchorParent = useRef( null ); - const getAnchorRectCallback = useCallback( - ( anchor ) => { - anchorParent.current = anchor.parentNode; - return getAnchorRect( anchor ); - }, - [ anchorParent ] - ); const onFocusOutsideCallback = useCallback( ( event ) => { if ( ! anchorParent.current ) { @@ -58,7 +44,7 @@ export function DotTip( { position={ position } noArrow focusOnMount="container" - getAnchorRect={ getAnchorRectCallback } + anchorIncludePadding role="dialog" aria-label={ __( 'Editor tips' ) } onClick={ onClick } diff --git a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap index c7e04b79aaf56b..844af24efddb4c 100644 --- a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap +++ b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap @@ -2,10 +2,10 @@ exports[`DotTip should render correctly 1`] = ` Date: Wed, 27 Nov 2019 15:28:08 +0100 Subject: [PATCH 2/4] Fix docs --- packages/components/src/popover/index.js | 42 +++++++++---------- packages/nux/src/components/dot-tip/index.js | 2 +- .../dot-tip/test/__snapshots__/index.js.snap | 2 +- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 790a6a84b2fe26..e796ff0df56f27 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -67,7 +67,7 @@ function computeAnchorRect( anchorRect, getAnchorRect, anchorRef = false, - anchorIncludePadding + shouldAnchorIncludePadding ) { if ( anchorRect ) { return anchorRect; @@ -92,7 +92,7 @@ function computeAnchorRect( const rect = anchorRef.getBoundingClientRect(); - if ( anchorIncludePadding ) { + if ( shouldAnchorIncludePadding ) { return rect; } @@ -106,7 +106,7 @@ function computeAnchorRect( const { parentNode } = anchorRefFallback.current; const rect = parentNode.getBoundingClientRect(); - if ( anchorIncludePadding ) { + if ( shouldAnchorIncludePadding ) { return rect; } @@ -153,14 +153,14 @@ function withoutPadding( rect, element ) { /** * Hook used to compute and update the anchor position properly. * - * @param {Object} anchorRefFallback Reference to the popover anchor fallback element. - * @param {Object} contentRef Reference to the popover content element. - * @param {Object} anchorRect Anchor Rect prop used to override the computed value. - * @param {Function} getAnchorRect Function used to override the anchor value computation algorithm. - * @param {Object} anchorRef Reference to the popover anchor fallback element. - * @param {Object} anchorIncludePadding Whether to include the anchor padding. - * @param {Object} anchorVerticalBuffer Vertical buffer for the anchor. - * @param {Object} anchorHorizontalBuffer Horizontal buffer for the anchor. + * @param {Object} anchorRefFallback Reference to the popover anchor fallback element. + * @param {Object} contentRef Reference to the popover content element. + * @param {Object} anchorRect Anchor Rect prop used to override the computed value. + * @param {Function} getAnchorRect Function used to override the anchor value computation algorithm. + * @param {Element|Range} anchorRef Reference to the popover anchor fallback element. + * @param {boolean} shouldAnchorIncludePadding Whether to include the anchor padding. + * @param {number} anchorVerticalBuffer Vertical buffer for the anchor. + * @param {number} anchorHorizontalBuffer Horizontal buffer for the anchor. * * @return {Object} Anchor position. */ @@ -170,7 +170,7 @@ function useAnchor( anchorRect, getAnchorRect, anchorRef, - anchorIncludePadding, + shouldAnchorIncludePadding, anchorVerticalBuffer, anchorHorizontalBuffer ) { @@ -181,7 +181,7 @@ function useAnchor( anchorRect, getAnchorRect, anchorRef, - anchorIncludePadding + shouldAnchorIncludePadding ); newAnchor = addBuffer( @@ -241,11 +241,11 @@ function useInitialContentSize( ref ) { * Hook used to compute and update the position of the popover * based on the anchor position and the content size. * - * @param {Object} anchor Anchor Position. - * @param {Object} contentSize Content Size. - * @param {string} position Position prop. - * @param {boolean} expandOnMobile Whether to show the popover full width on mobile. - * @param {Object} contentRef Reference to the popover content element. + * @param {Object} anchor Anchor Position. + * @param {Object} contentSize Content Size. + * @param {string} position Position prop. + * @param {boolean} expandOnMobile Whether to show the popover full width on mobile. + * @param {Object} contentRef Reference to the popover content element. * * @return {Object} Popover position. */ @@ -293,7 +293,7 @@ function usePopoverPosition( anchor, contentSize, position, expandOnMobile, cont * Hook used to focus the first tabbable element on mount. * * @param {boolean|string} focusOnMount Focus on mount mode. - * @param {Object} contentRef Reference to the popover content element. + * @param {Object} contentRef Reference to the popover content element. */ function useFocusContentOnMount( focusOnMount, contentRef ) { // Focus handling @@ -347,7 +347,7 @@ const Popover = ( { range, focusOnMount = 'firstElement', anchorRef, - anchorIncludePadding, + shouldAnchorIncludePadding, anchorVerticalBuffer, anchorHorizontalBuffer, anchorRect, @@ -372,7 +372,7 @@ const Popover = ( { anchorRect, getAnchorRect, anchorRef, - anchorIncludePadding, + shouldAnchorIncludePadding, anchorVerticalBuffer, anchorHorizontalBuffer ); diff --git a/packages/nux/src/components/dot-tip/index.js b/packages/nux/src/components/dot-tip/index.js index 48bad4d0ae0c71..5279ef41a1a60b 100644 --- a/packages/nux/src/components/dot-tip/index.js +++ b/packages/nux/src/components/dot-tip/index.js @@ -44,7 +44,7 @@ export function DotTip( { position={ position } noArrow focusOnMount="container" - anchorIncludePadding + shouldAnchorIncludePadding role="dialog" aria-label={ __( 'Editor tips' ) } onClick={ onClick } diff --git a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap index 844af24efddb4c..77341e829d7ee0 100644 --- a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap +++ b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap @@ -2,7 +2,7 @@ exports[`DotTip should render correctly 1`] = ` Date: Wed, 27 Nov 2019 15:31:36 +0100 Subject: [PATCH 3/4] Fix anchorRef doc --- packages/components/src/popover/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index e796ff0df56f27..0f5f16b1bc8e1f 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -157,7 +157,7 @@ function withoutPadding( rect, element ) { * @param {Object} contentRef Reference to the popover content element. * @param {Object} anchorRect Anchor Rect prop used to override the computed value. * @param {Function} getAnchorRect Function used to override the anchor value computation algorithm. - * @param {Element|Range} anchorRef Reference to the popover anchor fallback element. + * @param {Element|Range} anchorRef A live element or range reference. * @param {boolean} shouldAnchorIncludePadding Whether to include the anchor padding. * @param {number} anchorVerticalBuffer Vertical buffer for the anchor. * @param {number} anchorHorizontalBuffer Horizontal buffer for the anchor. From 9fd86c7b6d21c320a35b7a6aa4ac48f406f675dd Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Wed, 27 Nov 2019 17:09:21 +0100 Subject: [PATCH 4/4] Fix unit test --- .../nux/src/components/dot-tip/test/__snapshots__/index.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap index 77341e829d7ee0..5c5917cb54f525 100644 --- a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap +++ b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap @@ -2,7 +2,6 @@ exports[`DotTip should render correctly 1`] = `

It looks like you’re writing a letter. Would you like help?