From f3582695e494587f91274e82f05a35f7b1eeb267 Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Wed, 28 Feb 2024 12:45:12 -0800 Subject: [PATCH] Partially revert "ColorPicker: improve UX of dragging the handle when in popover on top of the editor (#55149)" This reverts commit 9da6f48b84b418e73d5f6f500a8553e010a42fe9. --- .../components/src/color-picker/component.tsx | 28 +--- .../components/src/color-picker/picker.tsx | 98 +---------- packages/components/src/color-picker/types.ts | 3 - packages/components/src/popover/index.tsx | 156 +++++++----------- packages/components/src/popover/style.scss | 9 - .../dot-tip/test/__snapshots__/index.js.snap | 4 +- 6 files changed, 63 insertions(+), 235 deletions(-) diff --git a/packages/components/src/color-picker/component.tsx b/packages/components/src/color-picker/component.tsx index 98e37df9783c5..8154a0a41331a 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, useMergeRefs } from '@wordpress/compose'; +import { useDebounce } from '@wordpress/compose'; import { __ } from '@wordpress/i18n'; /** @@ -56,24 +56,8 @@ const UnconnectedColorPicker = ( onChange, defaultValue = '#fff', copyFormat, - - // Context - onPickerDragStart, - onPickerDragEnd, ...divProps - } = 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 ); - }; + } = useContextSystem( props, 'ColorPicker' ); // Use a safe default value for the color and remove the possibility of `undefined`. const [ color, setColor ] = useControlledValue( { @@ -100,17 +84,11 @@ const UnconnectedColorPicker = ( ); return ( - + diff --git a/packages/components/src/color-picker/picker.tsx b/packages/components/src/color-picker/picker.tsx index af3727f1f8201..f2d7a84a48556 100644 --- a/packages/components/src/color-picker/picker.tsx +++ b/packages/components/src/color-picker/picker.tsx @@ -7,112 +7,18 @@ import { colord } from 'colord'; /** * WordPress dependencies */ -import { useMemo, useEffect, useRef } from '@wordpress/element'; +import { useMemo } from '@wordpress/element'; /** * Internal dependencies */ import type { PickerProps } from './types'; -/** - * 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 ) => { +export const Picker = ( { color, enableAlpha, onChange }: 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/popover/index.tsx b/packages/components/src/popover/index.tsx index 1634079c8cae7..07b6fb75df379 100644 --- a/packages/components/src/popover/index.tsx +++ b/packages/components/src/popover/index.tsx @@ -53,11 +53,6 @@ import { placementToMotionAnimationProps, getReferenceElement, } from './utils'; -import { - contextConnect, - useContextSystem, - ContextSystemProvider, -} from '../context'; import type { WordPressComponentProps } from '../context'; import type { PopoverProps, @@ -113,7 +108,7 @@ const getPopoverFallbackContainer = () => { return container; }; -const UnconnectedPopover = ( +const UnforwardedPopover = ( props: Omit< WordPressComponentProps< PopoverProps, 'div', false >, // To avoid overlaps between the standard HTML attributes and the props @@ -154,7 +149,7 @@ const UnconnectedPopover = ( // Rest ...contentProps - } = useContextSystem( props, 'Popover' ); + } = props; let computedFlipProp = flip; let computedResizeProp = resize; @@ -390,100 +385,63 @@ const UnconnectedPopover = ( 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 = ( - <> - { showBackdrop && ( -