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

BoxControl: stop using UnitControl's deprecated unit prop #39511

Merged
merged 4 commits into from
Mar 17, 2022
+94 −62
Merged

BoxControl: stop using UnitControl's deprecated unit prop #39511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2413a07
Stop using `unit` prop in `AllInputControl`
ciampo Mar 16, 2022
2ccaea3
Stop using `unit` prop in `AxialInputControls`
ciampo Mar 16, 2022
488a2bd
Stop using `unit` prop in `InputControls`
ciampo Mar 16, 2022
a2ea148
CHANGELOG
ciampo Mar 16, 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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
### Internal

- Delete the `composeStateReducers` utility function ([#39262](https://github.com/WordPress/gutenberg/pull/39262)).
- `BoxControl`: stop using `UnitControl`'s deprecated `unit` prop ([#39511](https://github.com/WordPress/gutenberg/pull/39511)).

## 19.6.0 (2022-03-11)

Expand Down
12 changes: 2 additions & 10 deletions packages/components/src/box-control/all-input-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
ALL_SIDES,
LABELS,
getAllValue,
getAllUnitFallback,
isValuesMixed,
isValuesDefined,
} from './utils';
Expand All @@ -26,17 +25,11 @@ export default function AllInputControl( {
setSelectedUnits,
...props
} ) {
const allValue = getAllValue( values, sides );
const allValue = getAllValue( values, selectedUnits, sides );
const hasValues = isValuesDefined( values );
const isMixed = hasValues && isValuesMixed( values, sides );
const isMixed = hasValues && isValuesMixed( values, selectedUnits, sides );
const allPlaceholder = isMixed ? LABELS.mixed : null;

// Set meaningful unit selection if no allValue and user has previously
// selected units without assigning values while controls were unlinked.
const allUnitFallback = ! allValue
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For ease of review: the unit prop below was removed. As a consequence, the allUnitFallback variable was also removed. The logic used for computing allUnitFallback has been moved and "embedded" in the getAllValue utility function

? getAllUnitFallback( selectedUnits )
: undefined;

const handleOnFocus = ( event ) => {
onFocus( event, { side: 'all' } );
};
Expand Down Expand Up @@ -104,7 +97,6 @@ export default function AllInputControl( {
disableUnits={ isMixed }
isOnly
value={ allValue }
unit={ allUnitFallback }
onChange={ handleOnChange }
onUnitChange={ handleOnUnitChange }
onFocus={ handleOnFocus }
Expand Down
53 changes: 32 additions & 21 deletions packages/components/src/box-control/axial-input-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* Internal dependencies
*/
import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils';
import UnitControl from './unit-control';
import { LABELS } from './utils';
import { Layout } from './styles/box-control-styles';
Expand Down Expand Up @@ -113,27 +114,37 @@ export default function AxialInputControls( {
align="top"
className="component-box-control__vertical-horizontal-input-controls"
>
{ filteredSides.map( ( side ) => (
<UnitControl
{ ...props }
isFirst={ first === side }
isLast={ last === side }
isOnly={ only === side }
value={ side === 'vertical' ? values.top : values.left }
unit={
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For ease of review: the unit prop here was removed, and instead the parsing and potentially overriding of the unit is applied to the value prop

side === 'vertical'
? selectedUnits.top
: selectedUnits.left
}
onChange={ createHandleOnChange( side ) }
onUnitChange={ createHandleOnUnitChange( side ) }
onFocus={ createHandleOnFocus( side ) }
onHoverOn={ createHandleOnHoverOn( side ) }
onHoverOff={ createHandleOnHoverOff( side ) }
label={ LABELS[ side ] }
key={ side }
/>
) ) }
{ filteredSides.map( ( side ) => {
const [
parsedQuantity,
parsedUnit,
] = parseQuantityAndUnitFromRawValue(
side === 'vertical' ? values.top : values.left
);
const selectedUnit =
side === 'vertical'
? selectedUnits.top
: selectedUnits.left;
return (
<UnitControl
{ ...props }
isFirst={ first === side }
isLast={ last === side }
isOnly={ only === side }
value={ [
parsedQuantity,
selectedUnit ?? parsedUnit,
].join( '' ) }
onChange={ createHandleOnChange( side ) }
onUnitChange={ createHandleOnUnitChange( side ) }
onFocus={ createHandleOnFocus( side ) }
onHoverOn={ createHandleOnHoverOn( side ) }
onHoverOff={ createHandleOnHoverOff( side ) }
label={ LABELS[ side ] }
key={ side }
/>
);
} ) }
</Layout>
);
}
49 changes: 30 additions & 19 deletions packages/components/src/box-control/input-controls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { noop } from 'lodash';
* Internal dependencies
*/
import UnitControl from './unit-control';
import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils';
import { ALL_SIDES, LABELS } from './utils';
import { LayoutContainer, Layout } from './styles/box-control-styles';

Expand Down Expand Up @@ -91,25 +92,35 @@ export default function BoxInputControls( {
align="top"
className="component-box-control__input-controls"
>
{ filteredSides.map( ( side ) => (
<UnitControl
{ ...props }
isFirst={ first === side }
isLast={ last === side }
isOnly={ only === side }
value={ values[ side ] }
unit={
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For ease of review: the unit prop here was removed, and instead the parsing and potentially overriding of the unit is applied to the value prop

values[ side ] ? undefined : selectedUnits[ side ]
}
onChange={ createHandleOnChange( side ) }
onUnitChange={ createHandleOnUnitChange( side ) }
onFocus={ createHandleOnFocus( side ) }
onHoverOn={ createHandleOnHoverOn( side ) }
onHoverOff={ createHandleOnHoverOff( side ) }
label={ LABELS[ side ] }
key={ `box-control-${ side }` }
/>
) ) }
{ filteredSides.map( ( side ) => {
const [
parsedQuantity,
parsedUnit,
] = parseQuantityAndUnitFromRawValue( values[ side ] );

const computedUnit = values[ side ]
? parsedUnit
: selectedUnits[ side ];

return (
<UnitControl
{ ...props }
isFirst={ first === side }
isLast={ last === side }
isOnly={ only === side }
value={ [ parsedQuantity, computedUnit ].join(
''
) }
onChange={ createHandleOnChange( side ) }
onUnitChange={ createHandleOnUnitChange( side ) }
onFocus={ createHandleOnFocus( side ) }
onHoverOn={ createHandleOnHoverOn( side ) }
onHoverOff={ createHandleOnHoverOff( side ) }
label={ LABELS[ side ] }
key={ `box-control-${ side }` }
/>
);
} ) }
</Layout>
</LayoutContainer>
);
Expand Down
41 changes: 29 additions & 12 deletions packages/components/src/box-control/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,32 @@ function mode( arr ) {
* Gets the 'all' input value and unit from values data.
*
* @param {Object} values Box values.
* @param {Object} selectedUnits Box units.
* @param {Array} availableSides Available box sides to evaluate.
*
* @return {string} A value + unit for the 'all' input.
*/
export function getAllValue( values = {}, availableSides = ALL_SIDES ) {
export function getAllValue(
values = {},
selectedUnits,
availableSides = ALL_SIDES
) {
const sides = normalizeSides( availableSides );
const parsedQuantitiesAndUnits = sides.map( ( side ) =>
parseQuantityAndUnitFromRawValue( values[ side ] )
);
const allValues = parsedQuantitiesAndUnits.map(
const allParsedQuantities = parsedQuantitiesAndUnits.map(
( value ) => value[ 0 ] ?? ''
);
const allUnits = parsedQuantitiesAndUnits.map( ( value ) => value[ 1 ] );
const allParsedUnits = parsedQuantitiesAndUnits.map(
Comment on lines +78 to +81
Copy link
Member

Choose a reason for hiding this comment

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

Verbosity FTW

( value ) => value[ 1 ]
);

const value = allValues.every( ( v ) => v === allValues[ 0 ] )
? allValues[ 0 ]
const commonQuantity = allParsedQuantities.every(
( v ) => v === allParsedQuantities[ 0 ]
)
? allParsedQuantities[ 0 ]
: '';
const unit = mode( allUnits );

/**
* The isNumber check is important. On reset actions, the incoming value
Expand All @@ -89,9 +97,17 @@ export function getAllValue( values = {}, availableSides = ALL_SIDES ) {
* isNumber() is more specific for these cases, rather than relying on a
* simple truthy check.
*/
const allValue = isNumber( value ) ? `${ value }${ unit }` : undefined;
let commonUnit;
if ( isNumber( commonQuantity ) ) {
commonUnit = mode( allParsedUnits );
} else {
// Set meaningful unit selection if no commonQuantity and user has previously
// selected units without assigning values while controls were unlinked.
commonUnit =
getAllUnitFallback( selectedUnits ) ?? mode( allParsedUnits );
}

return allValue;
return [ commonQuantity, commonUnit ].join( '' );
}

/**
Expand All @@ -113,13 +129,14 @@ export function getAllUnitFallback( selectedUnits ) {
/**
* Checks to determine if values are mixed.
*
* @param {Object} values Box values.
* @param {Array} sides Available box sides to evaluate.
* @param {Object} values Box values.
* @param {Object} selectedUnits Box units.
* @param {Array} sides Available box sides to evaluate.
*
* @return {boolean} Whether values are mixed.
*/
export function isValuesMixed( values = {}, sides = ALL_SIDES ) {
const allValue = getAllValue( values, sides );
export function isValuesMixed( values = {}, selectedUnits, sides = ALL_SIDES ) {
const allValue = getAllValue( values, selectedUnits, sides );
const isMixed = isNaN( parseFloat( allValue ) );

return isMixed;
Expand Down