BoxControl: Deprecate 36px default size

[hbhalodia]

Part of #65751

What?

Deprecate the 36px default size on BoxControl.

Testing Instructions

• Unit tests pass
• Storybook stories should not log console warnings
• All code snippets in documentation (JSDoc, Storybook, README) should have the __next40pxDefaultSize prop enabled

Screenshot

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: hbhalodia <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: ciampo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[mirka]

The instances in the unit tests aren't fully covered. It looks like we should take this opportunity to abstract/refactor a bit. How about something like this?

diff --git a/packages/components/src/box-control/test/index.tsx b/packages/components/src/box-control/test/index.tsx
index cc492cda18..185a18b258 100644
--- a/packages/components/src/box-control/test/index.tsx
+++ b/packages/components/src/box-control/test/index.tsx
@@ -15,11 +15,14 @@ import { useState } from '@wordpress/element';
 import BoxControl from '..';
 import type { BoxControlProps, BoxControlValue } from '../types';
 
-const Example = ( extraProps: Omit< BoxControlProps, 'onChange' > ) => {
+const ControlledBoxControl = (
+	extraProps: Omit< BoxControlProps, 'onChange' >
+) => {
 	const [ state, setState ] = useState< BoxControlValue >();
 
 	return (
 		<BoxControl
+			__next40pxDefaultSize
 			values={ state }
 			onChange={ ( next ) => setState( next ) }
 			{ ...extraProps }
@@ -27,12 +30,17 @@ const Example = ( extraProps: Omit< BoxControlProps, 'onChange' > ) => {
 	);
 };
 
+const UncontrolledBoxControl = ( {
+	onChange = () => {},
+	...props
+}: Omit< BoxControlProps, 'onChange' > & {
+	onChange?: BoxControlProps[ 'onChange' ];
+} ) => <BoxControl __next40pxDefaultSize onChange={ onChange } { ...props } />;
+
 describe( 'BoxControl', () => {
 	describe( 'Basic rendering', () => {
 		it( 'should render a box control input', () => {
-			render(
-				<BoxControl __next40pxDefaultSize onChange={ () => {} } />
-			);
+			render( <UncontrolledBoxControl /> );
 
 			expect(
 				screen.getByRole( 'group', { name: 'Box Control' } )
@@ -45,7 +53,7 @@ describe( 'BoxControl', () => {
 		it( 'should update values when interacting with input', async () => {
 			const user = userEvent.setup();
 
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', { name: 'All sides' } );
 
@@ -56,7 +64,7 @@ describe( 'BoxControl', () => {
 		} );
 
 		it( 'should update input values when interacting with slider', () => {
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			const slider = screen.getByRole( 'slider' );
 
@@ -70,7 +78,7 @@ describe( 'BoxControl', () => {
 
 		it( 'should update slider values when interacting with input', async () => {
 			const user = userEvent.setup();
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -84,7 +92,7 @@ describe( 'BoxControl', () => {
 		} );
 
 		it( 'should render the number input with a default min value of 0', () => {
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', { name: 'All sides' } );
 
@@ -93,10 +101,7 @@ describe( 'BoxControl', () => {
 
 		it( 'should pass down `inputProps` to the underlying number input', () => {
 			render(
-				<BoxControl
-					onChange={ () => {} }
-					inputProps={ { min: 10, max: 50 } }
-				/>
+				<UncontrolledBoxControl inputProps={ { min: 10, max: 50 } } />
 			);
 
 			const input = screen.getByRole( 'textbox', { name: 'All sides' } );
@@ -110,7 +115,7 @@ describe( 'BoxControl', () => {
 		it( 'should reset values when clicking Reset', async () => {
 			const user = userEvent.setup();
 
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -129,7 +134,7 @@ describe( 'BoxControl', () => {
 		it( 'should reset values when clicking Reset, if controlled', async () => {
 			const user = userEvent.setup();
 
-			render( <Example /> );
+			render( <ControlledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -148,7 +153,7 @@ describe( 'BoxControl', () => {
 		it( 'should reset values when clicking Reset, if controlled <-> uncontrolled state changes', async () => {
 			const user = userEvent.setup();
 
-			render( <Example /> );
+			render( <ControlledBoxControl /> );
 
 			const input = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -168,7 +173,9 @@ describe( 'BoxControl', () => {
 			const user = userEvent.setup();
 			const spyChange = jest.fn();
 
-			render( <BoxControl onChange={ ( v ) => spyChange( v ) } /> );
+			render(
+				<UncontrolledBoxControl onChange={ ( v ) => spyChange( v ) } />
+			);
 
 			const input = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -198,7 +205,7 @@ describe( 'BoxControl', () => {
 		it( 'should update a single side value when unlinked', async () => {
 			const user = userEvent.setup();
 
-			render( <Example /> );
+			render( <ControlledBoxControl /> );
 
 			await user.click(
 				screen.getByRole( 'button', { name: 'Unlink sides' } )
@@ -226,7 +233,7 @@ describe( 'BoxControl', () => {
 		it( 'should update a single side value when using slider unlinked', async () => {
 			const user = userEvent.setup();
 
-			render( <Example /> );
+			render( <ControlledBoxControl /> );
 
 			await user.click(
 				screen.getByRole( 'button', { name: 'Unlink sides' } )
@@ -254,7 +261,7 @@ describe( 'BoxControl', () => {
 		it( 'should update a whole axis when value is changed when unlinked', async () => {
 			const user = userEvent.setup();
 
-			render( <Example splitOnAxis /> );
+			render( <ControlledBoxControl splitOnAxis /> );
 
 			await user.click(
 				screen.getByRole( 'button', { name: 'Unlink sides' } )
@@ -278,7 +285,7 @@ describe( 'BoxControl', () => {
 		it( 'should update a whole axis using a slider when value is changed when unlinked', async () => {
 			const user = userEvent.setup();
 
-			render( <Example splitOnAxis /> );
+			render( <ControlledBoxControl splitOnAxis /> );
 
 			await user.click(
 				screen.getByRole( 'button', { name: 'Unlink sides' } )
@@ -302,7 +309,7 @@ describe( 'BoxControl', () => {
 		it( 'should show "Mixed" label when sides have different values but are linked', async () => {
 			const user = userEvent.setup();
 
-			render( <Example /> );
+			render( <ControlledBoxControl /> );
 
 			const unlinkButton = screen.getByRole( 'button', {
 				name: 'Unlink sides',
@@ -332,7 +339,7 @@ describe( 'BoxControl', () => {
 			const user = userEvent.setup();
 
 			// Render control.
-			render( <BoxControl onChange={ () => {} } /> );
+			render( <UncontrolledBoxControl /> );
 
 			// Make unit selection on all input control.
 			await user.selectOptions(
@@ -364,7 +371,7 @@ describe( 'BoxControl', () => {
 			const user = userEvent.setup();
 
 			// Render control.
-			const { rerender } = render( <BoxControl onChange={ () => {} } /> );
+			const { rerender } = render( <UncontrolledBoxControl /> );
 
 			// Make unit selection on all input control.
 			await user.selectOptions(
@@ -392,9 +399,7 @@ describe( 'BoxControl', () => {
 			} );
 
 			// Rerender with individual side value & confirm unit is selected.
-			rerender(
-				<BoxControl values={ { top: '2.5em' } } onChange={ () => {} } />
-			);
+			rerender( <UncontrolledBoxControl values={ { top: '2.5em' } } /> );
 
 			const rerenderedControls = screen.getAllByRole( 'combobox', {
 				name: 'Select unit',
@@ -416,7 +421,7 @@ describe( 'BoxControl', () => {
 			const user = userEvent.setup();
 			const onChangeSpy = jest.fn();
 
-			render( <BoxControl onChange={ onChangeSpy } /> );
+			render( <UncontrolledBoxControl onChange={ onChangeSpy } /> );
 
 			const valueInput = screen.getByRole( 'textbox', {
 				name: 'All sides',
@@ -445,7 +450,7 @@ describe( 'BoxControl', () => {
 			const user = userEvent.setup();
 			const setState = jest.fn();
 
-			render( <BoxControl onChange={ setState } /> );
+			render( <UncontrolledBoxControl onChange={ setState } /> );
 
 			await user.selectOptions(
 				screen.getByRole( 'combobox', {

[mirka]

I guess we should also add the prop to this ToolsPanel readme for good measure.

[ciampo]

Hey @hbhalodia , just checking if you're still able to work on this PR?

[hbhalodia]

Hi @mirka @ciampo, I am extremely sorry for not responding back 🙇. I do not have any bandwidth from last few days to work on the current in progress issues. I can look at those as soon as I get some time.

Thank you for understanding,

[hbhalodia]

Hi @mirka @ciampo, This is now ready to be reviewed again. Thanks for the feedbacks and suggestions 🙇.

[mirka]

All good, thank you for the follow up!