From 30dbcb1b3a8106bdd70466cd54f54134d1a0f268 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Tue, 17 Sep 2024 10:55:45 +0530 Subject: [PATCH 1/7] Fix replace Button and ButtonGroup component to use ToggleGroupControl and ToggleGroupControlOption --- .../components/image-size-control/index.js | 108 +++++++++++------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/index.js b/packages/block-editor/src/components/image-size-control/index.js index 7a333e98f795a..ecccaa417efd8 100644 --- a/packages/block-editor/src/components/image-size-control/index.js +++ b/packages/block-editor/src/components/image-size-control/index.js @@ -2,11 +2,11 @@ * WordPress dependencies */ import { - Button, - ButtonGroup, SelectControl, __experimentalNumberControl as NumberControl, __experimentalHStack as HStack, + __experimentalToggleGroupControl as ToggleGroupControl, + __experimentalToggleGroupControlOption as ToggleGroupControlOption, } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; @@ -33,6 +33,47 @@ export default function ImageSizeControl( { const { currentHeight, currentWidth, updateDimension, updateDimensions } = useDimensionHandler( height, width, imageHeight, imageWidth, onChange ); + /** + * Updates the dimensions for the given scale. + * Handler for toggle group control change. + * + * @param {number} scale The scale to update the dimensions for. + */ + const handleUpdateDimensions = ( scale ) => { + /** + * Check if scale is deselected from toggle group control option. + */ + if ( undefined === scale ) { + /** + * Update dimensions to default. + */ + updateDimensions(); + return; + } + + /** + * Calculate scaled width and height. + */ + const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); + const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + + /** + * Update dimensions. + */ + updateDimensions( scaledHeight, scaledWidth ); + }; + + /** + * Handler for adding the value to toggle group control. + */ + const addSelectedScaleValue = () => { + /** + * Calculate scale size based on current width and image width. + */ + const scaleSize = Math.round( currentWidth * ( 100 / imageWidth ) ); + return scaleSize; + }; + return ( <> { imageSizeOptions && imageSizeOptions.length > 0 && ( @@ -70,47 +111,28 @@ export default function ImageSizeControl( { size="__unstable-large" /> - - - { IMAGE_SIZE_PRESETS.map( ( scale ) => { - const scaledWidth = Math.round( - imageWidth * ( scale / 100 ) - ); - const scaledHeight = Math.round( - imageHeight * ( scale / 100 ) - ); - - const isCurrent = - currentWidth === scaledWidth && - currentHeight === scaledHeight; - - return ( - - ); - } ) } - - - + + handleUpdateDimensions( scale ) + } + value={ addSelectedScaleValue() } + isBlock + isDeselectable + __next40pxDefaultSize + __nextHasNoMarginBottom + > + { IMAGE_SIZE_PRESETS.map( ( scale ) => { + return ( + + ); + } ) } + ) } From e740c517f0a25ab2eba7dc137c058aba831aac50 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Mon, 14 Oct 2024 10:45:57 +0530 Subject: [PATCH 2/7] Add the logic to set undefined when custom values selected --- .../components/image-size-control/index.js | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/index.js b/packages/block-editor/src/components/image-size-control/index.js index ecccaa417efd8..8ec6aac4dba39 100644 --- a/packages/block-editor/src/components/image-size-control/index.js +++ b/packages/block-editor/src/components/image-size-control/index.js @@ -40,26 +40,14 @@ export default function ImageSizeControl( { * @param {number} scale The scale to update the dimensions for. */ const handleUpdateDimensions = ( scale ) => { - /** - * Check if scale is deselected from toggle group control option. - */ if ( undefined === scale ) { - /** - * Update dimensions to default. - */ updateDimensions(); return; } - /** - * Calculate scaled width and height. - */ const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); - /** - * Update dimensions. - */ updateDimensions( scaledHeight, scaledWidth ); }; @@ -67,11 +55,13 @@ export default function ImageSizeControl( { * Handler for adding the value to toggle group control. */ const addSelectedScaleValue = () => { - /** - * Calculate scale size based on current width and image width. - */ const scaleSize = Math.round( currentWidth * ( 100 / imageWidth ) ); - return scaleSize; + + if ( IMAGE_SIZE_PRESETS.includes( scaleSize ) ) { + return scaleSize; + } + + return undefined; }; return ( @@ -114,12 +104,9 @@ export default function ImageSizeControl( { - handleUpdateDimensions( scale ) - } + onChange={ handleUpdateDimensions } value={ addSelectedScaleValue() } isBlock - isDeselectable __next40pxDefaultSize __nextHasNoMarginBottom > From 12532be5803ddc616fe63bd9ab106d0f877b9268 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Wed, 16 Oct 2024 10:01:47 +0530 Subject: [PATCH 3/7] Updated the logic for adding the selected value to toggle group control --- .../src/components/image-size-control/index.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/index.js b/packages/block-editor/src/components/image-size-control/index.js index 8ec6aac4dba39..3cffd066efb22 100644 --- a/packages/block-editor/src/components/image-size-control/index.js +++ b/packages/block-editor/src/components/image-size-control/index.js @@ -52,17 +52,13 @@ export default function ImageSizeControl( { }; /** - * Handler for adding the value to toggle group control. + * Add the stored image preset value to toggle group control. */ - const addSelectedScaleValue = () => { - const scaleSize = Math.round( currentWidth * ( 100 / imageWidth ) ); - - if ( IMAGE_SIZE_PRESETS.includes( scaleSize ) ) { - return scaleSize; - } - - return undefined; - }; + const selectedValue = IMAGE_SIZE_PRESETS.find( ( scale ) => { + const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); + const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + return currentWidth === scaledWidth && currentHeight === scaledHeight; + } ); return ( <> @@ -105,7 +101,7 @@ export default function ImageSizeControl( { label={ __( 'Image size presets' ) } hideLabelFromVision onChange={ handleUpdateDimensions } - value={ addSelectedScaleValue() } + value={ selectedValue } isBlock __next40pxDefaultSize __nextHasNoMarginBottom From c84ff9fdec6401cffe180e1424f50df80202dba6 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Mon, 21 Oct 2024 14:35:21 +0530 Subject: [PATCH 4/7] Feat: Update the imageControl component to use utility function to get scaledWidth and scaledHeight --- .../components/image-size-control/index.js | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/index.js b/packages/block-editor/src/components/image-size-control/index.js index 3cffd066efb22..62f343d510b00 100644 --- a/packages/block-editor/src/components/image-size-control/index.js +++ b/packages/block-editor/src/components/image-size-control/index.js @@ -45,8 +45,11 @@ export default function ImageSizeControl( { return; } - const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); - const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + const { scaledWidth, scaledHeight } = getScaledWidthAndHeight( + scale, + imageWidth, + imageHeight + ); updateDimensions( scaledHeight, scaledWidth ); }; @@ -55,8 +58,12 @@ export default function ImageSizeControl( { * Add the stored image preset value to toggle group control. */ const selectedValue = IMAGE_SIZE_PRESETS.find( ( scale ) => { - const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); - const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + const { scaledWidth, scaledHeight } = getScaledWidthAndHeight( + scale, + imageWidth, + imageHeight + ); + return currentWidth === scaledWidth && currentHeight === scaledHeight; } ); @@ -121,3 +128,22 @@ export default function ImageSizeControl( { ); } + +/** + * Get scaled width and height for the given scale. + * + * @param {number} scale The scale to get the scaled width and height for. + * @param {number} imageWidth The image width. + * @param {number} imageHeight The image height. + * + * @return {Object} The scaled width and height. + */ +function getScaledWidthAndHeight( scale, imageWidth, imageHeight ) { + const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); + const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + + return { + scaledWidth, + scaledHeight, + }; +} From 08f19cdb6f7d8365e8d4085346493de14cbe955a Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Tue, 22 Oct 2024 16:56:54 +0530 Subject: [PATCH 5/7] Update the failing test for the imageSizeControl after updating the component --- .../image-size-control/test/index.js | 47 +------------------ 1 file changed, 2 insertions(+), 45 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/test/index.js b/packages/block-editor/src/components/image-size-control/test/index.js index c36be27971e69..6e142c554a531 100644 --- a/packages/block-editor/src/components/image-size-control/test/index.js +++ b/packages/block-editor/src/components/image-size-control/test/index.js @@ -221,45 +221,6 @@ describe( 'ImageSizeControl', () => { } ); } ); - describe( 'reset button', () => { - it( 'resets both height and width to default values', async () => { - const user = userEvent.setup(); - - render( - - ); - - const heightInput = screen.getByRole( 'spinbutton', { - name: 'Height', - } ); - const widthInput = screen.getByRole( 'spinbutton', { - name: 'Width', - } ); - - // The initial dimension values display first. - expect( heightInput ).toHaveValue( 300 ); - expect( widthInput ).toHaveValue( 400 ); - - await user.click( screen.getByRole( 'button', { name: 'Reset' } ) ); - - // Both attributes are set to undefined to clear custom values. - expect( mockOnChange ).toHaveBeenLastCalledWith( { - height: undefined, - width: undefined, - } ); - - // The inputs display the default values once more. - expect( heightInput ).toHaveValue( 100 ); - expect( widthInput ).toHaveValue( 200 ); - } ); - } ); - describe( 'image size percentage presets', () => { it( 'updates height and width attributes on selection', async () => { const user = userEvent.setup(); @@ -272,15 +233,12 @@ describe( 'ImageSizeControl', () => { /> ); - const button = screen.getByRole( 'button', { + const button = screen.getByRole( 'radio', { name: '50%', - pressed: false, } ); await user.click( button ); - expect( button ).toHaveClass( 'is-pressed' ); - // Both attributes are set to the rounded scaled value. expect( mockOnChange ).toHaveBeenLastCalledWith( { height: 50, @@ -299,9 +257,8 @@ describe( 'ImageSizeControl', () => { /> ); - const button = screen.getByRole( 'button', { + const button = screen.getByRole( 'radio', { name: '50%', - pressed: false, } ); await user.click( button ); From 4bc666b3196b16b13dfe511839d213a60a8c7bc0 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Mon, 28 Oct 2024 11:16:38 +0530 Subject: [PATCH 6/7] update the test suite and combine tests for ImageSizeControls dimensions --- .../image-size-control/test/index.js | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/test/index.js b/packages/block-editor/src/components/image-size-control/test/index.js index 6e142c554a531..70d94e78e70c2 100644 --- a/packages/block-editor/src/components/image-size-control/test/index.js +++ b/packages/block-editor/src/components/image-size-control/test/index.js @@ -222,7 +222,7 @@ describe( 'ImageSizeControl', () => { } ); describe( 'image size percentage presets', () => { - it( 'updates height and width attributes on selection', async () => { + it( 'updates height and width on selection', async () => { const user = userEvent.setup(); render( @@ -235,38 +235,23 @@ describe( 'ImageSizeControl', () => { const button = screen.getByRole( 'radio', { name: '50%', + checked: false, } ); await user.click( button ); + expect( button ).toBeChecked(); + // Both attributes are set to the rounded scaled value. expect( mockOnChange ).toHaveBeenLastCalledWith( { height: 50, width: 101, } ); - } ); - - it( 'updates height and width inputs on selection', async () => { - const user = userEvent.setup(); - - render( - - ); - - const button = screen.getByRole( 'radio', { - name: '50%', - } ); - - await user.click( button ); - // Both attributes are set to the rounded scaled value. expect( screen.getByRole( 'spinbutton', { name: 'Height' } ) ).toHaveValue( 50 ); + expect( screen.getByRole( 'spinbutton', { name: 'Width' } ) ).toHaveValue( 101 ); From a3d6c29cc98a935353ebc5098023c6eb105db6d9 Mon Sep 17 00:00:00 2001 From: hbhalodia Date: Tue, 29 Oct 2024 09:41:11 +0530 Subject: [PATCH 7/7] Refactor: move utility function before default export --- .../components/image-size-control/index.js | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/block-editor/src/components/image-size-control/index.js b/packages/block-editor/src/components/image-size-control/index.js index 62f343d510b00..b5bb705ab101c 100644 --- a/packages/block-editor/src/components/image-size-control/index.js +++ b/packages/block-editor/src/components/image-size-control/index.js @@ -18,6 +18,25 @@ import useDimensionHandler from './use-dimension-handler'; const IMAGE_SIZE_PRESETS = [ 25, 50, 75, 100 ]; const noop = () => {}; +/** + * Get scaled width and height for the given scale. + * + * @param {number} scale The scale to get the scaled width and height for. + * @param {number} imageWidth The image width. + * @param {number} imageHeight The image height. + * + * @return {Object} The scaled width and height. + */ +function getScaledWidthAndHeight( scale, imageWidth, imageHeight ) { + const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); + const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); + + return { + scaledWidth, + scaledHeight, + }; +} + export default function ImageSizeControl( { imageSizeHelp, imageWidth, @@ -128,22 +147,3 @@ export default function ImageSizeControl( { ); } - -/** - * Get scaled width and height for the given scale. - * - * @param {number} scale The scale to get the scaled width and height for. - * @param {number} imageWidth The image width. - * @param {number} imageHeight The image height. - * - * @return {Object} The scaled width and height. - */ -function getScaledWidthAndHeight( scale, imageWidth, imageHeight ) { - const scaledWidth = Math.round( imageWidth * ( scale / 100 ) ); - const scaledHeight = Math.round( imageHeight * ( scale / 100 ) ); - - return { - scaledWidth, - scaledHeight, - }; -}