From 8f6ff1dffb65b074180257814d7f985865c9cc24 Mon Sep 17 00:00:00 2001 From: Ramon Date: Thu, 28 May 2026 09:58:37 +1000 Subject: [PATCH 1/4] - Added `roundDownZoomForDisplay` function to format zoom values for better user experience. - Updated zoom control to use formatted display values instead of raw state values. - Introduced `CurrentZoomValue` component to show the current zoom state in the UI. - Updated tests --- .../media-editor-crop-panel/index.tsx | 19 +++++++- .../media-editor-crop-panel/test/index.tsx | 44 +++++++++++++++---- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx index 9eff839319c054..4613018c38327e 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx @@ -21,6 +21,19 @@ import { MAX_ZOOM } from '../../image-editor/core/constants'; import { getMinZoom } from '../../image-editor/core/containment'; import type { AspectRatioPreset } from '../../image-editor/core/constants'; +const ZOOM_DISPLAY_PRECISION = 10; +const ZOOM_DISPLAY_TOLERANCE = 1e-8; + +function roundDownZoomForDisplay( value: number ): number { + // Keep exact tenths represented slightly below their decimal value + // from dropping by another tenth in the visible control. + return ( + Math.floor( + ( value + ZOOM_DISPLAY_TOLERANCE ) * ZOOM_DISPLAY_PRECISION + ) / ZOOM_DISPLAY_PRECISION + ); +} + export interface MediaEditorCropPanelProps { /** * Selected aspect-ratio preset value as a string (so it round-trips @@ -63,6 +76,8 @@ export default function MediaEditorCropPanel( { const { state, setZoom } = useMediaEditor(); const zoomGestureHandlers = useCropGestureHandlers(); const minZoom = getMinZoom( state ); + const displayZoom = roundDownZoomForDisplay( state.zoom ); + const displayMinZoom = roundDownZoomForDisplay( minZoom ); return ( // Tag the whole panel as a crop-control region so the modal's @@ -95,10 +110,10 @@ export default function MediaEditorCropPanel( { { onPlacementControlInteraction?.(); setZoom( typeof value === 'number' ? value : minZoom ); diff --git a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx index c16050ecdb913c..114df66c970aad 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx @@ -8,10 +8,12 @@ import { fireEvent, render, screen } from '@testing-library/react'; */ import MediaEditorCropPanel from '..'; import type { MediaEditorCropPanelProps } from '..'; -import { MediaEditorStateProvider } from '../../../state'; +import { MediaEditorStateProvider, useMediaEditor } from '../../../state'; +import type { CropperState } from '../../../image-editor'; function setupCropPanel( - overrides: Partial< MediaEditorCropPanelProps > = {} + overrides: Partial< MediaEditorCropPanelProps > = {}, + initialCropperState?: Partial< CropperState > ) { const props: MediaEditorCropPanelProps = { aspectRatioValue: '1', @@ -27,18 +29,19 @@ function setupCropPanel( }; render( - + + ); return props; } -function expectElementBefore( first: HTMLElement, second: HTMLElement ) { - expect( first.compareDocumentPosition( second ) ).toBe( - Node.DOCUMENT_POSITION_FOLLOWING - ); +function CurrentZoomValue() { + const { state } = useMediaEditor(); + + return { state.zoom }; } describe( 'MediaEditorCropPanel', () => { @@ -49,8 +52,12 @@ describe( 'MediaEditorCropPanel', () => { const resizeCropArea = screen.getByLabelText( 'Show resize handles' ); const zoom = screen.getByRole( 'slider', { name: 'Zoom' } ); - expectElementBefore( aspectRatio, resizeCropArea ); - expectElementBefore( resizeCropArea, zoom ); + expect( aspectRatio.compareDocumentPosition( resizeCropArea ) ).toBe( + Node.DOCUMENT_POSITION_FOLLOWING + ); + expect( resizeCropArea.compareDocumentPosition( zoom ) ).toBe( + Node.DOCUMENT_POSITION_FOLLOWING + ); } ); it( 'passes selected aspect ratio changes to the caller', () => { @@ -80,4 +87,23 @@ describe( 'MediaEditorCropPanel', () => { expect( controls.onFreeformChange ).toHaveBeenCalledWith( false ); } ); + + it.each( [ + [ 3.749999999999999, 3.7 ], + [ 3.799999999999999, 3.8 ], + ] )( + 'formats displayed zoom value %s as %s without changing cropper state', + ( zoom, displayZoom ) => { + setupCropPanel( {}, { zoom } ); + + const zoomInput = screen.getByRole( 'spinbutton', { + name: 'Zoom', + } ); + + expect( zoomInput ).toHaveValue( displayZoom ); + expect( screen.getByTestId( 'current-zoom' ) ).toHaveTextContent( + String( zoom ) + ); + } + ); } ); From 165272e8dd5355f155f72471202a2cc89f7cb9fa Mon Sep 17 00:00:00 2001 From: Ramon Date: Thu, 28 May 2026 10:41:30 +1000 Subject: [PATCH 2/4] - Introduced a new `NumberControl` for displaying and adjusting zoom as a percentage. - Updated the `RangeControl` to utilize the new zoom percentage display. - Added utility functions to convert zoom values for display and input. --- .../media-editor-crop-panel/index.tsx | 103 ++++++++++++------ .../media-editor-crop-panel/test/index.tsx | 48 +++++--- 2 files changed, 100 insertions(+), 51 deletions(-) diff --git a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx index 4613018c38327e..b494d1dbc0430b 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx @@ -2,6 +2,11 @@ * WordPress dependencies */ import { + Flex, + FlexBlock, + FlexItem, + __experimentalInputControlSuffixWrapper as InputControlSuffixWrapper, + __experimentalNumberControl as NumberControl, RangeControl, SelectControl, ToggleControl, @@ -21,17 +26,11 @@ import { MAX_ZOOM } from '../../image-editor/core/constants'; import { getMinZoom } from '../../image-editor/core/containment'; import type { AspectRatioPreset } from '../../image-editor/core/constants'; -const ZOOM_DISPLAY_PRECISION = 10; -const ZOOM_DISPLAY_TOLERANCE = 1e-8; +const ZOOM_PERCENTAGE_SCALE = 100; +const MAX_ZOOM_PERCENTAGE = MAX_ZOOM * ZOOM_PERCENTAGE_SCALE; -function roundDownZoomForDisplay( value: number ): number { - // Keep exact tenths represented slightly below their decimal value - // from dropping by another tenth in the visible control. - return ( - Math.floor( - ( value + ZOOM_DISPLAY_TOLERANCE ) * ZOOM_DISPLAY_PRECISION - ) / ZOOM_DISPLAY_PRECISION - ); +function getZoomPercentageForDisplay( zoom: number ): number { + return Math.round( zoom * ZOOM_PERCENTAGE_SCALE ); } export interface MediaEditorCropPanelProps { @@ -76,8 +75,8 @@ export default function MediaEditorCropPanel( { const { state, setZoom } = useMediaEditor(); const zoomGestureHandlers = useCropGestureHandlers(); const minZoom = getMinZoom( state ); - const displayZoom = roundDownZoomForDisplay( state.zoom ); - const displayMinZoom = roundDownZoomForDisplay( minZoom ); + const zoomPercentage = getZoomPercentageForDisplay( state.zoom ); + const minZoomPercentage = getZoomPercentageForDisplay( minZoom ); return ( // Tag the whole panel as a crop-control region so the modal's @@ -107,27 +106,65 @@ export default function MediaEditorCropPanel( { onChange={ onFreeformChange } />
- { - onPlacementControlInteraction?.(); - setZoom( typeof value === 'number' ? value : minZoom ); - } } - renderTooltipContent={ ( value ) => { - const zoom = - typeof value === 'number' ? value : minZoom; - return sprintf( - /* translators: %d: zoom level as a percentage. */ - __( '%d%%' ), - Math.round( zoom * 100 ) - ); - } } - /> + + + { + onPlacementControlInteraction?.(); + setZoom( + typeof value === 'number' ? value : minZoom + ); + } } + renderTooltipContent={ ( value ) => { + const zoom = + typeof value === 'number' ? value : minZoom; + return sprintf( + /* translators: %d: zoom level as a percentage. */ + __( '%d%%' ), + getZoomPercentageForDisplay( zoom ) + ); + } } + withInputField={ false } + /> + + + { + const nextZoomPercentage = parseFloat( + value ?? '' + ); + if ( Number.isNaN( nextZoomPercentage ) ) { + return; + } + + onPlacementControlInteraction?.(); + setZoom( + nextZoomPercentage / ZOOM_PERCENTAGE_SCALE + ); + } } + suffix={ + + % + + } + /> + +
); diff --git a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx index 114df66c970aad..c0499fa7f009af 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx @@ -88,22 +88,34 @@ describe( 'MediaEditorCropPanel', () => { expect( controls.onFreeformChange ).toHaveBeenCalledWith( false ); } ); - it.each( [ - [ 3.749999999999999, 3.7 ], - [ 3.799999999999999, 3.8 ], - ] )( - 'formats displayed zoom value %s as %s without changing cropper state', - ( zoom, displayZoom ) => { - setupCropPanel( {}, { zoom } ); - - const zoomInput = screen.getByRole( 'spinbutton', { - name: 'Zoom', - } ); - - expect( zoomInput ).toHaveValue( displayZoom ); - expect( screen.getByTestId( 'current-zoom' ) ).toHaveTextContent( - String( zoom ) - ); - } - ); + it( 'displays zoom as a percentage without changing cropper state', () => { + setupCropPanel( {}, { zoom: 3.749999999999999 } ); + + const zoomInput = screen.getByRole( 'spinbutton', { + name: 'Zoom percentage', + } ); + + expect( zoomInput ).toHaveValue( 375 ); + expect( screen.getByTestId( 'current-zoom' ) ).toHaveTextContent( + '3.749999999999999' + ); + } ); + + it( 'converts percentage input back to the cropper zoom multiplier', () => { + const controls = setupCropPanel( { + onPlacementControlInteraction: jest.fn(), + } ); + + fireEvent.change( + screen.getByRole( 'spinbutton', { name: 'Zoom percentage' } ), + { + target: { value: '250' }, + } + ); + + expect( screen.getByTestId( 'current-zoom' ) ).toHaveTextContent( + '2.5' + ); + expect( controls.onPlacementControlInteraction ).toHaveBeenCalled(); + } ); } ); From 4fe788c6132bf9baeea6ef4950153ac8bfee9d93 Mon Sep 17 00:00:00 2001 From: Ramon Date: Thu, 28 May 2026 12:55:51 +1000 Subject: [PATCH 3/4] Fix spacing --- .../src/components/media-editor-crop-panel/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx index b494d1dbc0430b..9c420d84e4775d 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx @@ -106,7 +106,7 @@ export default function MediaEditorCropPanel( { onChange={ onFreeformChange } />
- + Date: Thu, 28 May 2026 13:05:35 +1000 Subject: [PATCH 4/4] Refactor zoom controls in MediaEditorCropPanel - Updated the zoom control to display percentage in the label and adjusted the associated utility functions for better clarity. - Simplified the RangeControl and removed the Flex layout for a more streamlined UI. - Updated tests to reflect changes in the zoom control's label and functionality. --- .../media-editor-crop-panel/index.tsx | 98 +++++++------------ .../media-editor-crop-panel/test/index.tsx | 6 +- 2 files changed, 36 insertions(+), 68 deletions(-) diff --git a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx index 9c420d84e4775d..cbf11327573335 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/index.tsx @@ -2,11 +2,6 @@ * WordPress dependencies */ import { - Flex, - FlexBlock, - FlexItem, - __experimentalInputControlSuffixWrapper as InputControlSuffixWrapper, - __experimentalNumberControl as NumberControl, RangeControl, SelectControl, ToggleControl, @@ -33,6 +28,10 @@ function getZoomPercentageForDisplay( zoom: number ): number { return Math.round( zoom * ZOOM_PERCENTAGE_SCALE ); } +function getMinZoomPercentageForDisplay( zoom: number ): number { + return Math.ceil( zoom * ZOOM_PERCENTAGE_SCALE ); +} + export interface MediaEditorCropPanelProps { /** * Selected aspect-ratio preset value as a string (so it round-trips @@ -76,7 +75,7 @@ export default function MediaEditorCropPanel( { const zoomGestureHandlers = useCropGestureHandlers(); const minZoom = getMinZoom( state ); const zoomPercentage = getZoomPercentageForDisplay( state.zoom ); - const minZoomPercentage = getZoomPercentageForDisplay( minZoom ); + const minZoomPercentage = getMinZoomPercentageForDisplay( minZoom ); return ( // Tag the whole panel as a crop-control region so the modal's @@ -106,65 +105,34 @@ export default function MediaEditorCropPanel( { onChange={ onFreeformChange } />
- - - { - onPlacementControlInteraction?.(); - setZoom( - typeof value === 'number' ? value : minZoom - ); - } } - renderTooltipContent={ ( value ) => { - const zoom = - typeof value === 'number' ? value : minZoom; - return sprintf( - /* translators: %d: zoom level as a percentage. */ - __( '%d%%' ), - getZoomPercentageForDisplay( zoom ) - ); - } } - withInputField={ false } - /> - - - { - const nextZoomPercentage = parseFloat( - value ?? '' - ); - if ( Number.isNaN( nextZoomPercentage ) ) { - return; - } - - onPlacementControlInteraction?.(); - setZoom( - nextZoomPercentage / ZOOM_PERCENTAGE_SCALE - ); - } } - suffix={ - - % - - } - /> - - + { + onPlacementControlInteraction?.(); + setZoom( + typeof value === 'number' + ? value / ZOOM_PERCENTAGE_SCALE + : minZoom + ); + } } + renderTooltipContent={ ( value ) => { + const percentage = + typeof value === 'number' + ? value + : minZoomPercentage; + return sprintf( + /* translators: %d: zoom level as a percentage. */ + __( '%d%%' ), + Math.round( percentage ) + ); + } } + />
); diff --git a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx index c0499fa7f009af..b5c68dccb6078c 100644 --- a/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx +++ b/packages/media-editor/src/components/media-editor-crop-panel/test/index.tsx @@ -50,7 +50,7 @@ describe( 'MediaEditorCropPanel', () => { const aspectRatio = screen.getByLabelText( 'Aspect ratio' ); const resizeCropArea = screen.getByLabelText( 'Show resize handles' ); - const zoom = screen.getByRole( 'slider', { name: 'Zoom' } ); + const zoom = screen.getByRole( 'slider', { name: 'Zoom (%)' } ); expect( aspectRatio.compareDocumentPosition( resizeCropArea ) ).toBe( Node.DOCUMENT_POSITION_FOLLOWING @@ -92,7 +92,7 @@ describe( 'MediaEditorCropPanel', () => { setupCropPanel( {}, { zoom: 3.749999999999999 } ); const zoomInput = screen.getByRole( 'spinbutton', { - name: 'Zoom percentage', + name: 'Zoom (%)', } ); expect( zoomInput ).toHaveValue( 375 ); @@ -107,7 +107,7 @@ describe( 'MediaEditorCropPanel', () => { } ); fireEvent.change( - screen.getByRole( 'spinbutton', { name: 'Zoom percentage' } ), + screen.getByRole( 'spinbutton', { name: 'Zoom (%)' } ), { target: { value: '250' }, }