From 9da6f48b84b418e73d5f6f500a8553e010a42fe9 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 12 Oct 2023 10:03:56 +0200 Subject: [PATCH] ColorPicker: improve UX of dragging the handle when in popover on top of the editor (#55149) * ColorPicker: expose drag start / end handlers via context * ColorPicker: remove overflow hidden rule * Dropdown: add some sort of backdrop to capture pointer events * Add test story * Prevent resizing on color palette's popover, which also triggers overflow: hidden * Improve backdrop position by using react portal * Cleanup * Aria-hidden * Cleanup * Scope querySelector within color picker s container element * Listen for drag events also on hue and alpha sliders * Remove temporary styles * Remove temporary storybook example * CHANGELOG * Move pointer events trap from Dropdown to Popover component * Use state to grab ColorPicker's container element to make sure the component re-renders * Remove overflow:hidden from PaletteEdit component's popover * Update CHANGELOG * Remove dead code * Update snapshot * Move newly added ColorPicker logic to a separate hook * Add inline comment around the resize: false change * Add more comments --- packages/components/CHANGELOG.md | 1 + .../components/src/color-palette/index.tsx | 4 + .../components/src/color-picker/component.tsx | 28 +++- .../components/src/color-picker/picker.tsx | 98 ++++++++++- .../components/src/color-picker/styles.ts | 1 - packages/components/src/color-picker/types.ts | 3 + .../components/src/palette-edit/index.tsx | 4 + packages/components/src/popover/index.tsx | 156 +++++++++++------- packages/components/src/popover/style.scss | 9 + .../dot-tip/test/__snapshots__/index.js.snap | 4 +- 10 files changed, 244 insertions(+), 64 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 924b8aab825a8c..e2455048a61d82 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -12,6 +12,7 @@ ### Bug Fix +- Render a "mouse event trap" when using a `ColorPicker` inside a `Popover` to prevent issues when rendering on top of `iframes` ([#55149](https://github.com/WordPress/gutenberg/pull/55149)). - `Modal`: fix closing when contained iframe is focused ([#51602](https://github.com/WordPress/gutenberg/pull/51602)). ### Internal diff --git a/packages/components/src/color-palette/index.tsx b/packages/components/src/color-palette/index.tsx index 297e03f389e3e2..96f8fba24cd644 100644 --- a/packages/components/src/color-palette/index.tsx +++ b/packages/components/src/color-palette/index.tsx @@ -147,6 +147,10 @@ export function CustomColorPickerDropdown( { const popoverProps = useMemo< DropdownProps[ 'popoverProps' ] >( () => ( { shift: true, + // Disabling resize as it would otherwise cause the popover to show + // scrollbars while dragging the color picker's handle close to the + // popover edge. + resize: false, ...( isRenderedInSidebar ? { // When in the sidebar: open to the left (stacking), diff --git a/packages/components/src/color-picker/component.tsx b/packages/components/src/color-picker/component.tsx index cff3b5d9c8ae4f..5f6d0754c93ccc 100644 --- a/packages/components/src/color-picker/component.tsx +++ b/packages/components/src/color-picker/component.tsx @@ -10,7 +10,7 @@ import namesPlugin from 'colord/plugins/names'; * WordPress dependencies */ import { useCallback, useState, useMemo } from '@wordpress/element'; -import { useDebounce } from '@wordpress/compose'; +import { useDebounce, useMergeRefs } from '@wordpress/compose'; import { __ } from '@wordpress/i18n'; /** @@ -49,8 +49,24 @@ const UnconnectedColorPicker = ( onChange, defaultValue = '#fff', copyFormat, + + // Context + onPickerDragStart, + onPickerDragEnd, ...divProps - } = useContextSystem( props, 'ColorPicker' ); + } = useContextSystem< + ColorPickerProps & { + onPickerDragStart?: ( event: MouseEvent ) => void; + onPickerDragEnd?: ( event: MouseEvent ) => void; + } + >( props, 'ColorPicker' ); + + const [ containerEl, setContainerEl ] = useState< HTMLElement | null >( + null + ); + const containerRef = ( node: HTMLElement | null ) => { + setContainerEl( node ); + }; // Use a safe default value for the color and remove the possibility of `undefined`. const [ color, setColor ] = useControlledValue( { @@ -77,11 +93,17 @@ const UnconnectedColorPicker = ( ); return ( - + diff --git a/packages/components/src/color-picker/picker.tsx b/packages/components/src/color-picker/picker.tsx index f2d7a84a485565..af3727f1f82014 100644 --- a/packages/components/src/color-picker/picker.tsx +++ b/packages/components/src/color-picker/picker.tsx @@ -7,18 +7,112 @@ import { colord } from 'colord'; /** * WordPress dependencies */ -import { useMemo } from '@wordpress/element'; +import { useMemo, useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies */ import type { PickerProps } from './types'; -export const Picker = ( { color, enableAlpha, onChange }: PickerProps ) => { +/** + * Track the start and the end of drag pointer events related to controlling + * the picker's saturation / hue / alpha, and fire the corresponding callbacks. + * This is particularly useful to implement synergies like the one with the + * `Popover` component, where a pointer events "trap" is rendered while + * the user is dragging the pointer to avoid potential interference with iframe + * elements. + * + * @param props + * @param props.containerEl + * @param props.onDragStart + * @param props.onDragEnd + */ +const useOnPickerDrag = ( { + containerEl, + onDragStart, + onDragEnd, +}: Pick< PickerProps, 'containerEl' | 'onDragStart' | 'onDragEnd' > ) => { + const isDragging = useRef( false ); + const leftWhileDragging = useRef( false ); + useEffect( () => { + if ( ! containerEl || ( ! onDragStart && ! onDragEnd ) ) { + return; + } + const interactiveElements = [ + containerEl.querySelector( '.react-colorful__saturation' ), + containerEl.querySelector( '.react-colorful__hue' ), + containerEl.querySelector( '.react-colorful__alpha' ), + ].filter( ( el ) => !! el ) as Element[]; + + if ( interactiveElements.length === 0 ) { + return; + } + + const doc = containerEl.ownerDocument; + + const onPointerUp: EventListener = ( event ) => { + isDragging.current = false; + leftWhileDragging.current = false; + onDragEnd?.( event as MouseEvent ); + }; + + const onPointerDown: EventListener = ( event ) => { + isDragging.current = true; + onDragStart?.( event as MouseEvent ); + }; + + const onPointerLeave: EventListener = () => { + leftWhileDragging.current = isDragging.current; + }; + + // Try to detect if the user released the pointer while away from the + // current window. If the check is successfull, the dragEnd callback will + // called as soon as the pointer re-enters the window (better late than never) + const onPointerEnter: EventListener = ( event ) => { + const noPointerButtonsArePressed = + ( event as PointerEvent ).buttons === 0; + + if ( leftWhileDragging.current && noPointerButtonsArePressed ) { + onPointerUp( event ); + } + }; + + // The pointerdown event is added on the interactive elements, + // while the remaining events are added on the document object since + // the pointer wouldn't necessarily be hovering the initial interactive + // element at that point. + interactiveElements.forEach( ( el ) => + el.addEventListener( 'pointerdown', onPointerDown ) + ); + doc.addEventListener( 'pointerup', onPointerUp ); + doc.addEventListener( 'pointerenter', onPointerEnter ); + doc.addEventListener( 'pointerleave', onPointerLeave ); + + return () => { + interactiveElements.forEach( ( el ) => + el.removeEventListener( 'pointerdown', onPointerDown ) + ); + doc.removeEventListener( 'pointerup', onPointerUp ); + doc.removeEventListener( 'pointerenter', onPointerEnter ); + doc.removeEventListener( 'pointerleave', onPointerUp ); + }; + }, [ onDragStart, onDragEnd, containerEl ] ); +}; + +export const Picker = ( { + color, + enableAlpha, + onChange, + onDragStart, + onDragEnd, + containerEl, +}: PickerProps ) => { const Component = enableAlpha ? RgbaStringColorPicker : RgbStringColorPicker; const rgbColor = useMemo( () => color.toRgbString(), [ color ] ); + useOnPickerDrag( { containerEl, onDragStart, onDragEnd } ); + return ( void; + containerEl: HTMLElement | null; + onDragStart?: ( event: MouseEvent ) => void; + onDragEnd?: ( event: MouseEvent ) => void; } export interface ColorInputProps { diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 9061222567103a..b3b8b626ce3b4a 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -121,6 +121,10 @@ function ColorPickerPopover< T extends Color | Gradient >( { () => ( { shift: true, offset: 20, + // Disabling resize as it would otherwise cause the popover to show + // scrollbars while dragging the color picker's handle close to the + // popover edge. + resize: false, placement: 'left-start', ...receivedPopoverProps, className: classnames( diff --git a/packages/components/src/popover/index.tsx b/packages/components/src/popover/index.tsx index 19df26e0777ee1..709d4b9884b5ec 100644 --- a/packages/components/src/popover/index.tsx +++ b/packages/components/src/popover/index.tsx @@ -53,6 +53,11 @@ import { placementToMotionAnimationProps, getReferenceElement, } from './utils'; +import { + contextConnect, + useContextSystem, + ContextSystemProvider, +} from '../context'; import type { WordPressComponentProps } from '../context'; import type { PopoverProps, @@ -108,7 +113,7 @@ const getPopoverFallbackContainer = () => { return container; }; -const UnforwardedPopover = ( +const UnconnectedPopover = ( props: Omit< WordPressComponentProps< PopoverProps, 'div', false >, // To avoid overlaps between the standard HTML attributes and the props @@ -148,7 +153,7 @@ const UnforwardedPopover = ( // Rest ...contentProps - } = props; + } = useContextSystem( props, 'Popover' ); let computedFlipProp = flip; let computedResizeProp = resize; @@ -383,63 +388,100 @@ const UnforwardedPopover = ( const isPositioned = ( ! shouldAnimate || animationFinished ) && x !== null && y !== null; + // In case a `ColorPicker` component is rendered as a child of `Popover`, + // the `Popover` component can be notified of when the user is dragging + // parts of the `ColorPicker` UI (this is possible because the `ColorPicker` + // component exposes the `onPickerDragStart` and `onPickerDragEnd` props + // via internal context). + // While the user is performing a pointer drag, the `Popover` will render + // a transparent backdrop element that will serve as a "pointer events trap", + // making sure that no pointer events reach any potential `iframe` element + // underneath (like, for example, the editor canvas in the WordPress editor). + const [ showBackdrop, setShowBackdrop ] = useState( false ); + const contextValue = useMemo( + () => ( { + ColorPicker: { + onPickerDragStart() { + setShowBackdrop( true ); + }, + onPickerDragEnd() { + setShowBackdrop( false ); + }, + }, + } ), + [] + ); + let content = ( - - { /* Prevents scroll on the document */ } - { isExpanded && } - { isExpanded && ( -
- - { headerTitle } - -
- ) } -
{ children }
- { hasArrow && ( + <> + { showBackdrop && (
- -
+ className="components-popover-pointer-events-trap" + aria-hidden="true" + onClick={ () => setShowBackdrop( false ) } + /> ) } - + + { /* Prevents scroll on the document */ } + { isExpanded && } + { isExpanded && ( +
+ + { headerTitle } + +
+ ) } +
+ + { children } + +
+ { hasArrow && ( +
+ +
+ ) } + + ); const shouldRenderWithinSlot = slot.ref && ! inline; @@ -489,7 +531,7 @@ const UnforwardedPopover = ( * ``` * */ -export const Popover = forwardRef( UnforwardedPopover ); +export const Popover = contextConnect( UnconnectedPopover, 'Popover' ); function PopoverSlot( { name = SLOT_NAME }: { name?: string }, diff --git a/packages/components/src/popover/style.scss b/packages/components/src/popover/style.scss index 790b840e6a648b..2e8a766829f3a6 100644 --- a/packages/components/src/popover/style.scss +++ b/packages/components/src/popover/style.scss @@ -133,3 +133,12 @@ $shadow-popover-border-top-only-alternate: 0 #{-$border-width} 0 $gray-900; stroke: $gray-900; } } + +.components-popover-pointer-events-trap { + // Same z-index as popover, but rendered before the popover element + // in DOM order = it will display just under the popover + z-index: z-index(".components-popover"); + position: fixed; + inset: 0; + background-color: transparent; +} 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 29a743a5316de3..287c550bb09c1b 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 @@ -3,7 +3,9 @@ exports[`DotTip should render correctly 1`] = `