Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnitControl: show unit label when units prop has only one unit #40784

Merged
merged 6 commits into from
May 4, 2022
Merged

UnitControl: show unit label when units prop has only one unit #40784

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5da089b
Fix controlled value of `unit` when the `units` prop only has one unit
ciampo May 3, 2022
0a8df80
Restore simpler Storybook "single unit" example
ciampo May 3, 2022
1cee4e3
Add one more test to check behaviour of component when single units
ciampo May 3, 2022
c86a634
CHANGELOG
ciampo May 3, 2022
35f2e35
Update packages/components/src/unit-control/index.tsx
ciampo May 4, 2022
8252bcf
Merge branch 'trunk' into fix/unit-control-single-unit-label
ciampo May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
- The `Button` component now displays the label as the tooltip for icon only buttons. ([#40716](https://github.com/WordPress/gutenberg/pull/40716))
- Use fake timers and fix usage of async methods from `@testing-library/user-event`. ([#40790](https://github.com/WordPress/gutenberg/pull/40790))
- UnitControl: avoid calling onChange callback twice when unit changes. ([#40796](https://github.com/WordPress/gutenberg/pull/40796))
- `UnitControl`: show unit label when units prop has only one unit. ([#40784](https://github.com/WordPress/gutenberg/pull/40784))


### Internal

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/unit-control/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function UnforwardedUnitControl(
);

const [ unit, setUnit ] = useControlledState< string | undefined >(
unitProp,
units.length === 1 ? units[ 0 ].label : unitProp,
ciampo marked this conversation as resolved.
Show resolved Hide resolved
{
initial: parsedUnit,
fallback: '',
Expand Down
36 changes: 3 additions & 33 deletions packages/components/src/unit-control/stories/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,40 +109,10 @@ TweakingTheNumberInput.args = {
/**
* When only one unit is available, the unit selection dropdown becomes static text.
*/
export const WithSingleUnitControlled: ComponentStory<
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just undoing some Storybook changes that were left from previous work on #40697

export const WithSingleUnit: ComponentStory<
typeof UnitControl
> = ( { onChange, ...args } ) => {
// Starting value is `undefined`
const [ value, setValue ] = useState< string | undefined >( undefined );

return (
<div style={ { maxWidth: '100px' } }>
<UnitControl
{ ...args }
value={ value }
onChange={ ( v, extra ) => {
setValue( v );
onChange?.( v, extra );
} }
/>
</div>
);
};
WithSingleUnitControlled.args = {
...Default.args,
units: CSS_UNITS.slice( 0, 1 ),
};

export const WithSingleUnitUncontrolled: ComponentStory<
typeof UnitControl
> = ( args ) => {
return (
<div style={ { maxWidth: '100px' } }>
<UnitControl { ...args } />
</div>
);
};
WithSingleUnitUncontrolled.args = {
> = DefaultTemplate.bind( {} );
WithSingleUnit.args = {
...Default.args,
units: CSS_UNITS.slice( 0, 1 ),
};
Expand Down
37 changes: 31 additions & 6 deletions packages/components/src/unit-control/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,7 @@ describe( 'UnitControl', () => {
} );

it( 'should render label if single units', () => {
render(
<UnitControl
units={ [ { value: '%', label: '%' } ] }
value="30%"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing value prop from test, which was added as a temporary fix in #40697 (see #40697 (comment) for context)

/>
);
render( <UnitControl units={ [ { value: '%', label: '%' } ] } /> );

const select = screen.queryByRole( 'combobox' );
const label = screen.getByText( '%' );
Expand Down Expand Up @@ -319,6 +314,36 @@ describe( 'UnitControl', () => {
expect.anything()
);
} );

it( 'should update value correctly when typed and blurred when a single unit is passed', async () => {
const onChangeSpy = jest.fn();
render(
<>
<button>Click me</button>
<UnitControl
units={ [ { value: '%', label: '%' } ] }
onChange={ onChangeSpy }
/>
</>
);

const input = getInput();
await user.type( input, '62' );

expect( onChangeSpy ).toHaveBeenLastCalledWith(
'62%',
expect.anything()
);

// Start counting again calls to `onChangeSpy`.
onChangeSpy.mockClear();

// Clicking on the button should cause the `onBlur` callback to fire.
const button = screen.getByRole( 'button' );
await user.click( button );

expect( onChangeSpy ).not.toHaveBeenCalled();
} );
} );

describe( 'Unit', () => {
Expand Down