Skip to content

Commit

Permalink
BoxControl: Deprecate 36px default size (#66704)
Browse files Browse the repository at this point in the history
* Add next40pxDefaultSize for the boxControl component

* Add the deperecation notice for the BoxControl component

* Add the changelog for the deprecation

* Add __next40pxDefaultSize to pass the failed unit test

* Update the unit test for the box control component

* Add __next40pxDefaultSize for BoxControl to toolspanel readme

This is added because with this PR, we are passing as default 40px size to the BoxControl component, so its usage should also get updated

* Fix changelog

---------

Co-authored-by: hbhalodia <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: ciampo <[email protected]>
  • Loading branch information
4 people authored Nov 28, 2024
1 parent 5b44d8c commit 35a68f6
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 26 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Deprecations

- `BoxControl`: Passive deprecate `onMouseOver`/`onMouseOut`. Pass to the `inputProps` prop instead ([#67332](https://github.com/WordPress/gutenberg/pull/67332)).
- `BoxControl`: Deprecate 36px default size ([#66704](https://github.com/WordPress/gutenberg/pull/66704)).

### Internal

Expand Down
1 change: 1 addition & 0 deletions packages/components/src/box-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function Example() {

return (
<BoxControl
__next40pxDefaultSize
values={ values }
onChange={ setValues }
/>
Expand Down
8 changes: 8 additions & 0 deletions packages/components/src/box-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type {
BoxControlProps,
BoxControlValue,
} from './types';
import { maybeWarnDeprecated36pxSize } from '../utils/deprecated-36px-size';

const defaultInputProps = {
min: 0,
Expand Down Expand Up @@ -64,6 +65,7 @@ function useUniqueId( idProp?: string ) {
*
* return (
* <BoxControl
* __next40pxDefaultSize
* values={ values }
* onChange={ setValues }
* />
Expand Down Expand Up @@ -155,6 +157,12 @@ function BoxControl( {
__next40pxDefaultSize,
};

maybeWarnDeprecated36pxSize( {
componentName: 'BoxControl',
__next40pxDefaultSize,
size: undefined,
} );

return (
<Grid
id={ id }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const TemplateControlled: StoryFn< typeof BoxControl > = ( props ) => {
export const Default = TemplateUncontrolled.bind( {} );
Default.args = {
label: 'Label',
__next40pxDefaultSize: true,
};

export const Controlled = TemplateControlled.bind( {} );
Expand Down
59 changes: 33 additions & 26 deletions packages/components/src/box-control/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,32 @@ 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 }
/>
);
};

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 onChange={ () => {} } /> );
render( <UncontrolledBoxControl /> );

expect(
screen.getByRole( 'group', { name: 'Box Control' } )
Expand All @@ -43,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' } );

Expand All @@ -54,7 +64,7 @@ describe( 'BoxControl', () => {
} );

it( 'should update input values when interacting with slider', () => {
render( <BoxControl onChange={ () => {} } /> );
render( <UncontrolledBoxControl /> );

const slider = screen.getByRole( 'slider' );

Expand All @@ -68,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',
Expand All @@ -82,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' } );

Expand All @@ -91,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' } );
Expand All @@ -108,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',
Expand All @@ -127,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',
Expand All @@ -146,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',
Expand All @@ -166,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',
Expand Down Expand Up @@ -196,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' } )
Expand Down Expand Up @@ -224,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' } )
Expand Down Expand Up @@ -252,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' } )
Expand All @@ -276,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' } )
Expand All @@ -300,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',
Expand Down Expand Up @@ -330,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(
Expand Down Expand Up @@ -362,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(
Expand Down Expand Up @@ -390,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',
Expand All @@ -414,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',
Expand Down Expand Up @@ -443,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', {
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/tools-panel/tools-panel/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export function DimensionPanel() {
onDeselect={ () => setPadding( undefined ) }
>
<BoxControl
__next40pxDefaultSize
label={ __( 'Padding' ) }
onChange={ setPadding }
values={ padding }
Expand All @@ -136,6 +137,7 @@ export function DimensionPanel() {
onDeselect={ () => setMargin( undefined ) }
>
<BoxControl
__next40pxDefaultSize
label={ __( 'Margin' ) }
onChange={ setMargin }
values={ margin }
Expand Down

1 comment on commit 35a68f6

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 35a68f6.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12076242086
📝 Reported issues:

Please sign in to comment.