From 1906f42870232b1135084a28c759195486fe7ddd Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Sun, 11 Apr 2021 21:33:34 -0700 Subject: [PATCH 1/9] =?UTF-8?q?Fix=20Dropdown=E2=80=99s=20closing=20behavi?= =?UTF-8?q?or=20when=20toggle=20is=20pressed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/components/src/dropdown/index.js | 32 ++++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/components/src/dropdown/index.js b/packages/components/src/dropdown/index.js index c5775fdaf09d9..67949c28106c6 100644 --- a/packages/components/src/dropdown/index.js +++ b/packages/components/src/dropdown/index.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { useRef, useEffect, useState } from '@wordpress/element'; +import { useRef, useEffect, useState, cloneElement } from '@wordpress/element'; /** * Internal dependencies @@ -56,16 +56,15 @@ export default function Dropdown( { } /** - * Closes the dropdown if a focus leaves the dropdown wrapper. This is - * intentionally distinct from `onClose` since focus loss from the popover - * is expected to occur when using the Dropdown's toggle button, in which - * case the correct behavior is to keep the dropdown closed. The same applies - * in case when focus is moved to the modal dialog. + * Closes the dropdown if focus leaves its popover unless its toggle was + * pressed or focus was moved to within a modal dialog. The former is to + * let the toggle act to close the dropdown and the latter is to enable + * the focus to return after the dialog is dismissed. */ function closeIfFocusOutside() { const { ownerDocument } = containerRef.current; if ( - ! containerRef.current.contains( ownerDocument.activeElement ) && + ! isPressingToggle.current && ! ownerDocument.activeElement.closest( '[role="dialog"]' ) ) { close(); @@ -81,17 +80,30 @@ export default function Dropdown( { const args = { isOpen, onToggle: toggle, onClose: close }; + const isPressingToggle = useRef(); + let toggleElement = renderToggle( args ); + const { props } = toggleElement; + const pressHandlers = { + onMouseDown: ( event ) => { + isPressingToggle.current = true; + props.onMouseDown?.( event ); + }, + onMouseUp: ( event ) => { + isPressingToggle.current = false; + props.onMouseUp?.( event ); + }, + }; + toggleElement = cloneElement( toggleElement, pressHandlers ); return (
- { renderToggle( args ) } + { toggleElement } { isOpen && ( Date: Sun, 11 Apr 2021 22:04:03 -0700 Subject: [PATCH 2/9] Remove tooltip wrapper from button in ButtonBlockAppender --- .../src/components/button-block-appender/index.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/block-editor/src/components/button-block-appender/index.js b/packages/block-editor/src/components/button-block-appender/index.js index eecd1bf51af17..59e3838cca3a2 100644 --- a/packages/block-editor/src/components/button-block-appender/index.js +++ b/packages/block-editor/src/components/button-block-appender/index.js @@ -6,7 +6,7 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { Button, Tooltip, VisuallyHidden } from '@wordpress/components'; +import { Button, VisuallyHidden } from '@wordpress/components'; import { forwardRef } from '@wordpress/element'; import { _x, sprintf } from '@wordpress/i18n'; import { Icon, plus } from '@wordpress/icons'; @@ -47,7 +47,7 @@ function ButtonBlockAppender( } const isToggleButton = ! hasSingleBlockType; - let inserterButton = ( + return ( ); - - if ( isToggleButton || hasSingleBlockType ) { - inserterButton = ( - { inserterButton } - ); - } - return inserterButton; } } isAppender /> From b5e33c48076fe61a84684e01f6c3c746a29e344b Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Sun, 11 Apr 2021 22:05:37 -0700 Subject: [PATCH 3/9] Remove extraneous fragment in PostSchedule --- .../components/sidebar/post-schedule/index.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/edit-post/src/components/sidebar/post-schedule/index.js b/packages/edit-post/src/components/sidebar/post-schedule/index.js index 6d99ccf506141..59a958c9339f8 100644 --- a/packages/edit-post/src/components/sidebar/post-schedule/index.js +++ b/packages/edit-post/src/components/sidebar/post-schedule/index.js @@ -22,16 +22,14 @@ export function PostSchedule() { position="bottom left" contentClassName="edit-post-post-schedule__dialog" renderToggle={ ( { onToggle, isOpen } ) => ( - <> - - + ) } renderContent={ () => } /> From 51dd613902786a77665c5f3d49668ab705cdfc82 Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Tue, 13 Apr 2021 10:36:04 -0700 Subject: [PATCH 4/9] =?UTF-8?q?Return=20a=20single-child=20toggle=20for=20?= =?UTF-8?q?Dropdown=20in=20ColorEdit=E2=80=99s=20ColorOption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/components/src/color-edit/index.js | 35 ++++++++++----------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/components/src/color-edit/index.js b/packages/components/src/color-edit/index.js index 31b49ad1454b3..54673a20136c2 100644 --- a/packages/components/src/color-edit/index.js +++ b/packages/components/src/color-edit/index.js @@ -8,7 +8,7 @@ import { isEmpty, kebabCase } from 'lodash'; * WordPress dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { useEffect, useState } from '@wordpress/element'; +import { useEffect, useRef, useState } from '@wordpress/element'; import { edit, close, chevronDown, chevronUp, plus } from '@wordpress/icons'; /** @@ -21,15 +21,6 @@ import Button from '../button'; import TextControl from '../text-control'; import BaseControl from '../base-control'; -function DropdownOpenOnMount( { shouldOpen, isOpen, onToggle } ) { - useEffect( () => { - if ( shouldOpen && ! isOpen ) { - onToggle(); - } - }, [] ); - return null; -} - function ColorOption( { color, name, @@ -56,6 +47,14 @@ function ColorOption( { ( isHover || isFocused || isEditingName || isShowingAdvancedPanel ) && ! immutableColorSlugs.includes( slug ); + const dropdownHandle = useRef(); + useEffect( () => { + const { isOpen, onToggle } = dropdownHandle.current; + if ( isEditingColorOnMount && isOpen === false ) { + onToggle(); + } + }, [ isEditingColorOnMount ] ); + return (
( - <> - + renderToggle={ ( { isOpen, onToggle } ) => { + if ( isOpen !== dropdownHandle.current?.isOpen ) { + dropdownHandle.current = { isOpen, onToggle }; + } + return ( - - ) } + ); + } } renderContent={ () => ( Date: Tue, 13 Apr 2021 10:52:01 -0700 Subject: [PATCH 5/9] =?UTF-8?q?Update=20dropdown=20usage=20in=20Template?= =?UTF-8?q?=20Part=20block=E2=80=99s=20placeholder?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To toggle properly it should return a single-child from renderToggle render prop. The prior use seems to be a hack around style concerns. --- .../template-part/edit/placeholder/index.js | 23 +++++++++---------- .../src/template-part/editor.scss | 5 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/block-library/src/template-part/edit/placeholder/index.js b/packages/block-library/src/template-part/edit/placeholder/index.js index 4a8462b278b9c..48f204862b67c 100644 --- a/packages/block-library/src/template-part/edit/placeholder/index.js +++ b/packages/block-library/src/template-part/edit/placeholder/index.js @@ -55,21 +55,17 @@ export default function TemplatePartPlaceholder( { ) } > ( - <> - - - + ) } renderContent={ ( { onClose } ) => ( ) } /> + ); } diff --git a/packages/block-library/src/template-part/editor.scss b/packages/block-library/src/template-part/editor.scss index c61e0651ae725..e96c7746c5272 100644 --- a/packages/block-library/src/template-part/editor.scss +++ b/packages/block-library/src/template-part/editor.scss @@ -1,3 +1,8 @@ +// Make dropdown margins the same as Placeholder component does for buttons +.wp-block-template-part__placeholder-preview-dropdown { + margin-right: $grid-unit-15; + margin-bottom: $grid-unit-15; +} .wp-block-template-part__placeholder-preview-dropdown-content, .wp-block-template-part__preview-dropdown-content { From b82696a9edefd7b2e945cf73734a4419c9befbae Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Tue, 13 Apr 2021 11:00:00 -0700 Subject: [PATCH 6/9] Keep toggle enabled when dropdown is open in Template Part block It was a hack to avoid toggle behavior bug in some UAs. Also, an unused classname is removed. --- packages/block-library/src/template-part/edit/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/block-library/src/template-part/edit/index.js b/packages/block-library/src/template-part/edit/index.js index b3673a17775b8..640f152f3560b 100644 --- a/packages/block-library/src/template-part/edit/index.js +++ b/packages/block-library/src/template-part/edit/index.js @@ -131,16 +131,12 @@ export default function TemplatePartEdit( { ( { __( 'Replace' ) } From 4991346548362b882a26a50dac4254a35061b422 Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Tue, 13 Apr 2021 11:12:57 -0700 Subject: [PATCH 7/9] Update dropdown test due to single-child toggle concern --- packages/components/src/dropdown/test/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/components/src/dropdown/test/index.js b/packages/components/src/dropdown/test/index.js index a18e2479d37e8..82048902f2be5 100644 --- a/packages/components/src/dropdown/test/index.js +++ b/packages/components/src/dropdown/test/index.js @@ -65,20 +65,20 @@ describe( 'Dropdown', () => { [ + renderToggle={ ( { isOpen, onToggle } ) => ( , - , - ] } - renderContent={ () => null } + + ) } + renderContent={ ( { onClose } ) => ( + + ) } /> ); From 4ab48b910ff0246175613237f363ae0bb208e531 Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Sat, 17 Apr 2021 07:38:58 -0700 Subject: [PATCH 8/9] Update snapshots --- .../color-palette/test/__snapshots__/control.js.snap | 2 ++ .../src/color-palette/test/__snapshots__/index.js.snap | 8 ++++++++ .../header/more-menu/test/__snapshots__/index.js.snap | 3 +++ 3 files changed, 13 insertions(+) diff --git a/packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap b/packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap index 1328665a03eb5..d707f817ee273 100644 --- a/packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap +++ b/packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap @@ -98,6 +98,8 @@ exports[`ColorPaletteControl matches the snapshot 1`] = ` aria-label="Custom color picker" className="components-button is-link" onClick={[Function]} + onMouseDown={[Function]} + onMouseUp={[Function]} type="button" > Custom color diff --git a/packages/components/src/color-palette/test/__snapshots__/index.js.snap b/packages/components/src/color-palette/test/__snapshots__/index.js.snap index 1db3cdb27938f..c4bff0c9438d9 100644 --- a/packages/components/src/color-palette/test/__snapshots__/index.js.snap +++ b/packages/components/src/color-palette/test/__snapshots__/index.js.snap @@ -118,6 +118,8 @@ exports[`ColorPalette Dropdown should render it correctly 1`] = ` aria-label="Custom color picker" isLink={true} onClick={[Function]} + onMouseDown={[Function]} + onMouseUp={[Function]} >