From 23171fbc05dd0a1d270a424cd3c294aac506ec31 Mon Sep 17 00:00:00 2001 From: Mitchell Austin Date: Mon, 28 Feb 2022 23:11:34 -0800 Subject: [PATCH] Rewrite as function component, decruftify, update tests --- .../src/focal-point-picker/controls.js | 8 +- .../src/focal-point-picker/focal-point.js | 7 +- .../components/src/focal-point-picker/grid.js | 6 +- .../src/focal-point-picker/index.js | 447 ++++++------------ .../src/focal-point-picker/media.js | 14 +- .../styles/focal-point-picker-style.js | 5 +- .../src/focal-point-picker/test/index.js | 29 +- .../src/focal-point-picker/utils.js | 8 +- 8 files changed, 181 insertions(+), 343 deletions(-) diff --git a/packages/components/src/focal-point-picker/controls.js b/packages/components/src/focal-point-picker/controls.js index c001aa866ca13..dc8ea6404029e 100644 --- a/packages/components/src/focal-point-picker/controls.js +++ b/packages/components/src/focal-point-picker/controls.js @@ -18,19 +18,19 @@ const noop = () => {}; export default function FocalPointPickerControls( { onChange = noop, - percentages = { + point = { x: 0.5, y: 0.5, }, } ) { - const valueX = fractionToPercentage( percentages.x ); - const valueY = fractionToPercentage( percentages.y ); + const valueX = fractionToPercentage( point.x ); + const valueY = fractionToPercentage( point.y ); const handleChange = ( value, axis ) => { const num = parseInt( value, 10 ); if ( ! isNaN( num ) ) { - onChange( { ...percentages, [ axis ]: num / 100 } ); + onChange( { ...point, [ axis ]: num / 100 } ); } }; diff --git a/packages/components/src/focal-point-picker/focal-point.js b/packages/components/src/focal-point-picker/focal-point.js index b27c978896264..5809bf81af9f1 100644 --- a/packages/components/src/focal-point-picker/focal-point.js +++ b/packages/components/src/focal-point-picker/focal-point.js @@ -13,15 +13,12 @@ import { */ import classnames from 'classnames'; -export default function FocalPoint( { - coordinates = { left: '50%', top: '50%' }, - ...props -} ) { +export default function FocalPoint( { left = '50%', top = '50%', ...props } ) { const classes = classnames( 'components-focal-point-picker__icon_container' ); - const style = { ...coordinates }; + const style = { left, top }; return ( diff --git a/packages/components/src/focal-point-picker/grid.js b/packages/components/src/focal-point-picker/grid.js index 776419eaef5b3..c8f01ea280267 100644 --- a/packages/components/src/focal-point-picker/grid.js +++ b/packages/components/src/focal-point-picker/grid.js @@ -16,11 +16,7 @@ import { useUpdateEffect } from '../utils/hooks'; const { clearTimeout, setTimeout } = typeof window !== 'undefined' ? window : {}; -export default function FocalPointPickerGrid( { - bounds = {}, - value, - ...props -} ) { +export default function FocalPointPickerGrid( { bounds, value, ...props } ) { const animationProps = useRevealAnimation( value ); const style = { width: bounds.width, diff --git a/packages/components/src/focal-point-picker/index.js b/packages/components/src/focal-point-picker/index.js index 37a8659571208..bdce052292573 100644 --- a/packages/components/src/focal-point-picker/index.js +++ b/packages/components/src/focal-point-picker/index.js @@ -7,8 +7,11 @@ import classnames from 'classnames'; * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { Component, createRef } from '@wordpress/element'; -import { withInstanceId } from '@wordpress/compose'; +import { useEffect, useRef, useState } from '@wordpress/element'; +import { + __experimentalUseDragging as useDragging, + useInstanceId, +} from '@wordpress/compose'; import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; /** @@ -25,309 +28,165 @@ import { } from './styles/focal-point-picker-style'; import { INITIAL_BOUNDS } from './utils'; -export class FocalPointPicker extends Component { - constructor( props ) { - super( ...arguments ); - - this.state = { - isDragging: false, - bounds: INITIAL_BOUNDS, - percentages: props.value, - }; - - this.containerRef = createRef(); - this.mediaRef = createRef(); - - this.onMouseDown = this.startDrag.bind( this ); - this.onMouseUp = this.stopDrag.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); - this.onMouseMove = this.doDrag.bind( this ); - this.ifDraggingStop = () => { - if ( this.state.isDragging ) { - this.stopDrag(); - } - }; - this.onChangeAtControls = ( value ) => { - this.updateValue( value, () => { - this.props.onChange( this.state.percentages ); - } ); - }; - - this.updateBounds = this.updateBounds.bind( this ); - this.updateValue = this.updateValue.bind( this ); - } - componentDidMount() { - const { defaultView } = this.containerRef.current.ownerDocument; - defaultView.addEventListener( 'resize', this.updateBounds ); - - /* - * Set initial bound values. - * - * This is necessary for Safari: - * https://github.com/WordPress/gutenberg/issues/25814 - */ - this.updateBounds(); - } - componentDidUpdate( prevProps ) { - if ( prevProps.url !== this.props.url ) { - this.ifDraggingStop(); - } - /* - * Handles cases where the incoming value changes. - * An example is the values resetting based on an UNDO action. - */ - const { - isDragging, - percentages: { x, y }, - } = this.state; - const { value } = this.props; - if ( ! isDragging && ( value.x !== x || value.y !== y ) ) { - this.setState( { percentages: value } ); - } - } - componentWillUnmount() { - const { defaultView } = this.containerRef.current.ownerDocument; - defaultView.removeEventListener( 'resize', this.updateBounds ); - this.ifDraggingStop(); - } - calculateBounds() { - const bounds = INITIAL_BOUNDS; - - if ( ! this.mediaRef.current ) { - return bounds; - } - - // Prevent division by zero when updateBounds runs in componentDidMount - if ( - this.mediaRef.current.clientWidth === 0 || - this.mediaRef.current.clientHeight === 0 - ) { - return bounds; - } +export default function FocalPointPicker( { + autoPlay = true, + className, + help, + label, + onChange, + onDrag, + onDragEnd, + onDragStart, + resolvePoint, + url, + value: valueProp = { + x: 0.5, + y: 0.5, + }, +} ) { + const [ point, setPoint ] = useState( valueProp ); + // Tracks whether the internal point value has not propagated. + const isPointHeld = useRef(); + const { current: keepPoint } = useRef( ( value ) => { + setPoint( value ); + isPointHeld.current = true; + } ); + const sendPoint = ( value = point ) => { + onChange?.( value ); + isPointHeld.current = false; + }; + // Uses the internal point if it is held or else the value from props. + const { x, y } = isPointHeld.current ? point : valueProp; + + const dragAreaRef = useRef(); + const [ bounds, setBounds ] = useState( INITIAL_BOUNDS ); + const { current: updateBounds } = useRef( () => { + const { clientWidth: width, clientHeight: height } = + dragAreaRef.current; + // Falls back to initial bounds if the ref has no size. Since styles + // give the drag area dimensions even when the media has not loaded + // this should only happen in unit tests (jsdom). + setBounds( + width > 0 && height > 0 ? { width, height } : { ...INITIAL_BOUNDS } + ); + } ); - const dimensions = { - width: this.mediaRef.current.clientWidth, - height: this.mediaRef.current.clientHeight, + useEffect( () => { + const { defaultView } = dragAreaRef.current.ownerDocument; + defaultView.addEventListener( 'resize', updateBounds ); + return () => { + defaultView.removeEventListener( 'resize', updateBounds ); }; - - const pickerDimensions = this.pickerDimensions(); - - const widthRatio = pickerDimensions.width / dimensions.width; - const heightRatio = pickerDimensions.height / dimensions.height; - - if ( heightRatio >= widthRatio ) { - bounds.width = bounds.right = pickerDimensions.width; - bounds.height = dimensions.height * widthRatio; - bounds.top = ( pickerDimensions.height - bounds.height ) / 2; - bounds.bottom = bounds.top + bounds.height; - } else { - bounds.height = bounds.bottom = pickerDimensions.height; - bounds.width = dimensions.width * heightRatio; - bounds.left = ( pickerDimensions.width - bounds.width ) / 2; - bounds.right = bounds.left + bounds.width; + }, [] ); + + const { startDrag, endDrag, isDragging } = useDragging( { + onDragStart: ( event ) => { + dragAreaRef.current.focus(); + const value = getValueWithinDragArea( event ); + onDragStart?.( value, event ); + keepPoint( value ); + }, + onDragMove: ( event ) => { + // Prevents text-selection when dragging. + event.preventDefault(); + const value = getValueWithinDragArea( event ); + onDrag?.( value, event ); + keepPoint( value ); + }, + onDragEnd: ( event ) => { + onDragEnd?.( event ); + }, + } ); + + // Propagates the value when a drag gesture has ended. + useEffect( () => { + if ( ! isDragging && isPointHeld.current ) sendPoint(); + }, [ isDragging ] ); + + const getValueWithinDragArea = ( { clientX, clientY, shiftKey } ) => { + const { top, left } = dragAreaRef.current.getBoundingClientRect(); + let nextX = ( clientX - left ) / bounds.width; + let nextY = ( clientY - top ) / bounds.height; + // Enables holding shift to jump values by 10%. + if ( shiftKey ) { + nextX = Math.round( nextX / 0.1 ) * 0.1; + nextY = Math.round( nextY / 0.1 ) * 0.1; } - return bounds; - } - updateValue( nextValue, callback ) { - const resolvedValue = - this.props.resolvePoint?.( nextValue ) ?? nextValue; + return getFinalValue( { x: nextX, y: nextY } ); + }; - const { x, y } = resolvedValue; - - const nextPercentage = { - x: parseFloat( x ).toFixed( 2 ), - y: parseFloat( y ).toFixed( 2 ), + const getFinalValue = ( value ) => { + const resolvedValue = resolvePoint?.( value ) ?? value; + resolvedValue.x = Math.max( 0, Math.min( resolvedValue.x, 1 ) ); + resolvedValue.y = Math.max( 0, Math.min( resolvedValue.y, 1 ) ); + return { + x: parseFloat( resolvedValue.x ).toFixed( 2 ), + y: parseFloat( resolvedValue.y ).toFixed( 2 ), }; + }; - this.setState( { percentages: nextPercentage }, callback ); - } - updateBounds() { - this.setState( { - bounds: this.calculateBounds(), - } ); - } - startDrag( event ) { - this.containerRef.current.focus(); - this.setState( { isDragging: true } ); - const { ownerDocument } = this.containerRef.current; - ownerDocument.addEventListener( 'mouseup', this.onMouseUp ); - ownerDocument.addEventListener( 'mousemove', this.onMouseMove ); - const value = this.getValueFromPoint( - { x: event.pageX, y: event.pageY }, - event.shiftKey - ); - this.updateValue( value ); - this.props.onDragStart?.( value, event ); - } - stopDrag( event ) { - const { ownerDocument } = this.containerRef.current; - ownerDocument.removeEventListener( 'mouseup', this.onMouseUp ); - ownerDocument.removeEventListener( 'mousemove', this.onMouseMove ); - this.setState( { isDragging: false }, () => { - this.props.onChange( this.state.percentages ); - } ); - this.props.onDragEnd?.( event ); - } - onKeyDown( event ) { + const arrowKeyStep = ( event ) => { const { keyCode, shiftKey } = event; if ( ! [ UP, DOWN, LEFT, RIGHT ].includes( keyCode ) ) return; event.preventDefault(); - - const next = { ...this.state.percentages }; + const value = { x, y }; const step = shiftKey ? 0.1 : 0.01; const delta = keyCode === UP || keyCode === LEFT ? -1 * step : step; const axis = keyCode === UP || keyCode === DOWN ? 'y' : 'x'; - const value = parseFloat( next[ axis ] ) + delta; - - next[ axis ] = value; - - this.updateValue( next, () => { - this.props.onChange( this.state.percentages ); - } ); - } - doDrag( event ) { - // Prevents text-selection when dragging. - event.preventDefault(); - const value = this.getValueFromPoint( - { x: event.pageX, y: event.pageY }, - event.shiftKey - ); - this.updateValue( value ); - this.props.onDrag?.( value, event ); - } - getValueFromPoint( point, byTenths ) { - const { bounds } = this.state; - - const pickerDimensions = this.pickerDimensions(); - const relativePoint = { - left: point.x - pickerDimensions.left, - top: point.y - pickerDimensions.top, - }; - - const left = Math.max( - bounds.left, - Math.min( relativePoint.left, bounds.right ) - ); - const top = Math.max( - bounds.top, - Math.min( relativePoint.top, bounds.bottom ) - ); - - let nextX = - ( left - bounds.left ) / - ( pickerDimensions.width - bounds.left * 2 ); - let nextY = - ( top - bounds.top ) / ( pickerDimensions.height - bounds.top * 2 ); - - // Enables holding shift to jump values by 10% - if ( byTenths ) { - nextX = Math.round( nextX / 0.1 ) * 0.1; - nextY = Math.round( nextY / 0.1 ) * 0.1; - } - - return { x: nextX, y: nextY }; - } - pickerDimensions() { - const containerNode = this.containerRef.current; - - if ( ! containerNode ) { - return { - width: 0, - height: 0, - left: 0, - top: 0, - }; - } - - const { clientHeight, clientWidth } = containerNode; - const { top, left } = containerNode.getBoundingClientRect(); - - return { - width: clientWidth, - height: clientHeight, - top: top + document.body.scrollTop, - left, - }; - } - iconCoordinates() { - const { - bounds, - percentages: { x, y }, - } = this.state; - - if ( bounds.left === undefined || bounds.top === undefined ) { - return; - } - - const { width, height } = this.pickerDimensions(); - return { - left: x * ( width - bounds.left * 2 ) + bounds.left, - top: y * ( height - bounds.top * 2 ) + bounds.top, - }; - } - render() { - const { autoPlay, className, help, instanceId, label, url } = - this.props; - const { bounds, isDragging, percentages } = this.state; - - const classes = classnames( - 'components-focal-point-picker-control', - className - ); - - const id = `inspector-focal-point-picker-control-${ instanceId }`; - - return ( - - - - - - - - - - - ); - } + value[ axis ] = parseFloat( value[ axis ] ) + delta; + sendPoint( getFinalValue( value ) ); + }; + + const focalPointPosition = { + left: x * bounds.width, + top: y * bounds.height, + }; + + const classes = classnames( + 'components-focal-point-picker-control', + className + ); + + const instanceId = useInstanceId( FocalPointPicker ); + const id = `inspector-focal-point-picker-control-${ instanceId }`; + + return ( + + + + + + + + + { + sendPoint( getFinalValue( value ) ); + } } + /> + + ); } - -FocalPointPicker.defaultProps = { - autoPlay: true, - value: { - x: 0.5, - y: 0.5, - }, - url: null, -}; - -export default withInstanceId( FocalPointPicker ); diff --git a/packages/components/src/focal-point-picker/media.js b/packages/components/src/focal-point-picker/media.js index fb4c3748e84b1..50a79d03416f1 100644 --- a/packages/components/src/focal-point-picker/media.js +++ b/packages/components/src/focal-point-picker/media.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { useRef, useLayoutEffect } from '@wordpress/element'; +import { useLayoutEffect } from '@wordpress/element'; /** * Internal dependencies @@ -9,13 +9,11 @@ import { useRef, useLayoutEffect } from '@wordpress/element'; import { MediaPlaceholder } from './styles/focal-point-picker-style'; import { isVideoType } from './utils'; -const noop = () => {}; - export default function Media( { alt, autoPlay, src, - onLoad = noop, + onLoad, mediaRef, // Exposing muted prop for test rendering purposes // https://github.com/testing-library/react-testing-library/issues/470 @@ -57,18 +55,14 @@ export default function Media( { ); } -function MediaPlaceholderElement( { mediaRef, onLoad = noop, ...props } ) { - const onLoadRef = useRef( onLoad ); - +function MediaPlaceholderElement( { mediaRef, onLoad, ...props } ) { /** * This async callback mimics the onLoad (img) / onLoadedData (video) callback * for media elements. It is used in the main component * to calculate the dimensions + boundaries for positioning. */ useLayoutEffect( () => { - window.requestAnimationFrame( () => { - onLoadRef.current(); - } ); + window.requestAnimationFrame( () => onLoad() ); }, [] ); return ; diff --git a/packages/components/src/focal-point-picker/styles/focal-point-picker-style.js b/packages/components/src/focal-point-picker/styles/focal-point-picker-style.js index 7e7dd9df46059..59e27fea5bc14 100644 --- a/packages/components/src/focal-point-picker/styles/focal-point-picker-style.js +++ b/packages/components/src/focal-point-picker/styles/focal-point-picker-style.js @@ -9,6 +9,7 @@ import styled from '@emotion/styled'; import { Flex } from '../../flex'; import BaseUnitControl from '../../unit-control'; import { COLORS } from '../../utils'; +import { INITIAL_BOUNDS } from '../utils'; export const MediaWrapper = styled.div` background-color: transparent; @@ -45,9 +46,9 @@ export const MediaContainer = styled.div` export const MediaPlaceholder = styled.div` background: ${ COLORS.lightGray[ 300 ] }; box-sizing: border-box; - height: 170px; + height: ${ INITIAL_BOUNDS.height }px; max-width: 280px; - min-width: 200px; + min-width: ${ INITIAL_BOUNDS.width }px; width: 100%; `; diff --git a/packages/components/src/focal-point-picker/test/index.js b/packages/components/src/focal-point-picker/test/index.js index 08b7d331b6845..59cb653091355 100644 --- a/packages/components/src/focal-point-picker/test/index.js +++ b/packages/components/src/focal-point-picker/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { act, fireEvent, render } from '@testing-library/react'; +import { fireEvent, render } from '@testing-library/react'; /** * Internal dependencies @@ -34,7 +34,8 @@ describe( 'FocalPointPicker', () => { } ); it( 'should stop a drag operation when focus is lost', () => { - expect( firedDragEnd && ! firedDrag ).toBe( true ); + expect( firedDrag ).toBe( undefined ); + expect( firedDragEnd ).toBe( true ); } ); } ); @@ -49,11 +50,9 @@ describe( 'FocalPointPicker', () => { } ); const { getByRole } = render( ); const dragArea = getByRole( 'button' ); - act( () => { - fireEvent.mouseDown( dragArea ); - fireEvent.mouseMove( dragArea ); - fireEvent.mouseUp( dragArea ); - } ); + fireEvent.mouseDown( dragArea ); + fireEvent.mouseMove( dragArea ); + fireEvent.mouseUp( dragArea ); expect( events.reduce( ( last, eventName, index ) => { return last && logs[ index ].name === eventName; @@ -76,12 +75,10 @@ describe( 'FocalPointPicker', () => { } } /> ); - // Click and press arrow up const dragArea = getByRole( 'button' ); - act( () => { - fireEvent.mouseDown( dragArea ); - fireEvent.keyDown( dragArea, { charCode: 0, keyCode: 38 } ); - } ); + fireEvent.mouseDown( dragArea, { clientX: 1, clientY: 1 } ); + // Change doesn't fire until mouseup + fireEvent.mouseUp( dragArea ); expect( spy ).toHaveBeenCalled(); expect( spyChange ).toHaveBeenCalledWith( { x: '0.91', @@ -104,12 +101,10 @@ describe( 'FocalPointPicker', () => { const { getByRole } = render( ); - // Click and press arrow up + // Focus and press arrow up const dragArea = getByRole( 'button' ); - act( () => { - fireEvent.mouseDown( dragArea ); - fireEvent.keyDown( dragArea, { charCode: 0, keyCode: 38 } ); - } ); + dragArea.focus(); + fireEvent.keyDown( dragArea, { charCode: 0, keyCode: 38 } ); expect( spyChange ).toHaveBeenCalledWith( { x: '0.14', y: '0.61', diff --git a/packages/components/src/focal-point-picker/utils.js b/packages/components/src/focal-point-picker/utils.js index 8366f8a7a9ebd..2205ae4ad66c9 100644 --- a/packages/components/src/focal-point-picker/utils.js +++ b/packages/components/src/focal-point-picker/utils.js @@ -1,10 +1,6 @@ export const INITIAL_BOUNDS = { - top: 0, - left: 0, - bottom: 0, - right: 0, - width: 0, - height: 0, + width: 200, + height: 170, }; const VIDEO_EXTENSIONS = [