Skip to content

Commit 9a55df7

Browse files
Merge 52a8ca1 into a11bd54
2 parents a11bd54 + 52a8ca1 commit 9a55df7

5 files changed

Lines changed: 90 additions & 2 deletions

File tree

‎packages/components/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
- `Text`: Make single-line truncation and `isBlock` overrides consistent across the main document and iframes ([#82573](https://github.com/WordPress/gutenberg/pull/82573)).
1515
- `CardDivider`: Apply the Card's width, display, and border color consistently inside iframes ([#82577](https://github.com/WordPress/gutenberg/pull/82577)).
16+
- `BorderControl`, `BorderBoxControl`: Stop offering `%` as a width unit, since `border-width` does not accept percentages. A width already set in `%` keeps showing its unit ([#83492](https://github.com/WordPress/gutenberg/pull/83492)).
17+
- `UnitControl`: After clearing a value whose unit is not in `units`, give the next typed value the unit shown in the select instead of the cleared one ([#83492](https://github.com/WordPress/gutenberg/pull/83492)).
1618

1719
### Internal
1820

‎packages/components/src/border-control/border-control/component.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { __ } from '@wordpress/i18n';
22
import BorderControlDropdown from '../border-control-dropdown';
33
import UnitControl from '../../unit-control';
4+
import { CSS_UNITS } from '../../unit-control/utils';
45
import RangeControl from '../../range-control';
56
import { HStack } from '../../h-stack';
67
import { StyledLabel } from '../../base-control/styles/base-control-styles';
@@ -12,6 +13,9 @@ import { useBorderControl } from './hook';
1213
import type { BorderControlProps, LabelProps } from '../types';
1314
import { Spacer } from '../../spacer';
1415

16+
// Percentages are not valid `border-width` values.
17+
const BORDER_WIDTH_UNITS = CSS_UNITS.filter( ( unit ) => unit.value !== '%' );
18+
1519
const BorderLabel = ( props: LabelProps ) => {
1620
const { label, hideLabelFromVision } = props;
1721

@@ -93,6 +97,7 @@ const UnconnectedBorderControl = (
9397
onChange={ onWidthChange }
9498
value={ border?.width || '' }
9599
placeholder={ placeholder }
100+
units={ BORDER_WIDTH_UNITS }
96101
disableUnits={ disableUnits }
97102
__unstableInputWidth={ inputWidth }
98103
/>
@@ -105,7 +110,7 @@ const UnconnectedBorderControl = (
105110
max={ 100 }
106111
min={ 0 }
107112
onChange={ onSliderChange }
108-
step={ [ 'px', '%' ].includes( widthUnit ) ? 1 : 0.1 }
113+
step={ widthUnit === 'px' ? 1 : 0.1 }
109114
value={ widthValue || undefined }
110115
withInputField={ false }
111116
/>

‎packages/components/src/border-control/test/index.jsdom.test.tsx‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,44 @@ describe( 'BorderControl', () => {
139139
expect( widthInput ).toHaveAttribute( 'placeholder', 'Mixed' );
140140
} );
141141

142+
it( 'should not offer a percentage unit for the width', () => {
143+
const props = createProps();
144+
render( <TestBorderControl { ...props } /> );
145+
146+
const unitSelect = screen.getByRole( 'combobox', {
147+
name: 'Select unit',
148+
} );
149+
expect(
150+
within( unitSelect ).queryByRole( 'option', { name: '%' } )
151+
).not.toBeInTheDocument();
152+
} );
153+
154+
it( 'should keep the unit of a percentage width until it is changed', async () => {
155+
const user = userEvent.setup();
156+
const props = createProps( {
157+
value: { ...defaultBorder, width: '10%' },
158+
} );
159+
const { rerender } = render( <TestBorderControl { ...props } /> );
160+
161+
const unitSelect = screen.getByRole( 'combobox', {
162+
name: 'Select unit',
163+
} );
164+
expect( unitSelect ).toHaveValue( '%' );
165+
166+
await user.selectOptions( unitSelect, 'px' );
167+
168+
expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
169+
...defaultBorder,
170+
width: '10px',
171+
} );
172+
173+
rerender( <TestBorderControl { ...props } /> );
174+
175+
expect(
176+
within( unitSelect ).queryByRole( 'option', { name: '%' } )
177+
).not.toBeInTheDocument();
178+
} );
179+
142180
describe( 'color indicator inline styles', () => {
143181
const getIndicatorWrapper = ( border: Border ) => {
144182
render(

‎packages/components/src/unit-control/index.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ function UnforwardedUnitControl(
119119
return;
120120
}
121121

122+
// After clearing a value whose unit isn't offered, the select shows the
123+
// first offered unit, so only fall back to the current unit if offered.
124+
const fallbackUnit = units.some( ( option ) => option.value === unit )
125+
? unit
126+
: undefined;
127+
122128
/*
123129
* Customizing the onChange callback.
124130
* This allows as to broadcast a combined value+unit to onChange.
@@ -127,7 +133,7 @@ function UnforwardedUnitControl(
127133
nextQuantityValue,
128134
units,
129135
parsedQuantity,
130-
unit
136+
fallbackUnit
131137
).join( '' );
132138

133139
onChangeProp?.( onChangeValue, changeProps );

‎packages/components/src/unit-control/test/index.jsdom.test.tsx‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,43 @@ describe( 'UnitControl', () => {
529529
expect( select ).toHaveValue( 'vmax' );
530530
} );
531531

532+
it( 'should use the unit shown in the select after clearing a value whose unit is not offered', async () => {
533+
const user = userEvent.setup();
534+
const onChangeSpy = vi.fn();
535+
536+
const ControlledUnitControl = () => {
537+
const [ value, setValue ] = useState< string | undefined >(
538+
'10%'
539+
);
540+
return (
541+
<UnitControl
542+
value={ value }
543+
units={ [
544+
{ value: 'px', label: 'px' },
545+
{ value: 'em', label: 'em' },
546+
] }
547+
onChange={ ( nextValue, extra ) => {
548+
setValue( nextValue );
549+
onChangeSpy( nextValue, extra );
550+
} }
551+
/>
552+
);
553+
};
554+
render( <ControlledUnitControl /> );
555+
556+
const input = getInput();
557+
await user.clear( input );
558+
559+
expect( getSelect() ).toHaveValue( 'px' );
560+
561+
await user.type( input, '5' );
562+
563+
expect( onChangeSpy ).toHaveBeenLastCalledWith(
564+
'5px',
565+
expect.anything()
566+
);
567+
} );
568+
532569
it( 'should run onBlur callback when the unit select is blurred', async () => {
533570
const user = userEvent.setup();
534571

0 commit comments

Comments
 (0)