From fcb341d6315dd759e88b179afdbc702c8c0f4710 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 5 Dec 2024 13:10:10 +0100 Subject: [PATCH 01/11] BoxControl: Refactor and unify the different sides implementation --- .../src/box-control/all-input-control.tsx | 110 --------- .../src/box-control/axial-input-controls.tsx | 165 -------------- packages/components/src/box-control/index.tsx | 33 ++- .../src/box-control/input-control.tsx | 211 ++++++++++++++++++ .../src/box-control/input-controls.tsx | 167 -------------- packages/components/src/box-control/types.ts | 1 + packages/components/src/box-control/utils.ts | 7 +- 7 files changed, 239 insertions(+), 455 deletions(-) delete mode 100644 packages/components/src/box-control/all-input-control.tsx delete mode 100644 packages/components/src/box-control/axial-input-controls.tsx create mode 100644 packages/components/src/box-control/input-control.tsx delete mode 100644 packages/components/src/box-control/input-controls.tsx diff --git a/packages/components/src/box-control/all-input-control.tsx b/packages/components/src/box-control/all-input-control.tsx deleted file mode 100644 index 0cfaea21915f6d..00000000000000 --- a/packages/components/src/box-control/all-input-control.tsx +++ /dev/null @@ -1,110 +0,0 @@ -/** - * WordPress dependencies - */ -import { useInstanceId } from '@wordpress/compose'; -/** - * Internal dependencies - */ -import type { UnitControlProps } from '../unit-control/types'; -import { - FlexedRangeControl, - StyledUnitControl, -} from './styles/box-control-styles'; -import type { BoxControlInputControlProps } from './types'; -import { parseQuantityAndUnitFromRawValue } from '../unit-control'; -import { - LABELS, - applyValueToSides, - getAllValue, - isValuesMixed, - isValuesDefined, - CUSTOM_VALUE_SETTINGS, -} from './utils'; - -const noop = () => {}; - -export default function AllInputControl( { - __next40pxDefaultSize, - onChange = noop, - onFocus = noop, - values, - sides, - selectedUnits, - setSelectedUnits, - ...props -}: BoxControlInputControlProps ) { - const inputId = useInstanceId( AllInputControl, 'box-control-input-all' ); - - const allValue = getAllValue( values, selectedUnits, sides ); - const hasValues = isValuesDefined( values ); - const isMixed = hasValues && isValuesMixed( values, selectedUnits, sides ); - const allPlaceholder = isMixed ? LABELS.mixed : undefined; - - const [ parsedQuantity, parsedUnit ] = - parseQuantityAndUnitFromRawValue( allValue ); - - const handleOnFocus: React.FocusEventHandler< HTMLInputElement > = ( - event - ) => { - onFocus( event, { side: 'all' } ); - }; - - const onValueChange = ( next?: string ) => { - const isNumeric = next !== undefined && ! isNaN( parseFloat( next ) ); - const nextValue = isNumeric ? next : undefined; - const nextValues = applyValueToSides( values, nextValue, sides ); - - onChange( nextValues ); - }; - - const sliderOnChange = ( next?: number ) => { - onValueChange( - next !== undefined ? [ next, parsedUnit ].join( '' ) : undefined - ); - }; - - // Set selected unit so it can be used as fallback by unlinked controls - // when individual sides do not have a value containing a unit. - const handleOnUnitChange: UnitControlProps[ 'onUnitChange' ] = ( unit ) => { - const newUnits = applyValueToSides( selectedUnits, unit, sides ); - setSelectedUnits( newUnits ); - }; - - return ( - <> - - - - - ); -} diff --git a/packages/components/src/box-control/axial-input-controls.tsx b/packages/components/src/box-control/axial-input-controls.tsx deleted file mode 100644 index f8cbc5635c9b55..00000000000000 --- a/packages/components/src/box-control/axial-input-controls.tsx +++ /dev/null @@ -1,165 +0,0 @@ -/** - * WordPress dependencies - */ -import { useInstanceId } from '@wordpress/compose'; -/** - * Internal dependencies - */ -import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; -import Tooltip from '../tooltip'; -import { CUSTOM_VALUE_SETTINGS, LABELS } from './utils'; -import { - FlexedBoxControlIcon, - FlexedRangeControl, - InputWrapper, - StyledUnitControl, -} from './styles/box-control-styles'; -import type { BoxControlInputControlProps } from './types'; - -const groupedSides = [ 'vertical', 'horizontal' ] as const; -type GroupedSide = ( typeof groupedSides )[ number ]; - -export default function AxialInputControls( { - __next40pxDefaultSize, - onChange, - onFocus, - values, - selectedUnits, - setSelectedUnits, - sides, - ...props -}: BoxControlInputControlProps ) { - const generatedId = useInstanceId( - AxialInputControls, - `box-control-input` - ); - - const createHandleOnFocus = - ( side: GroupedSide ) => - ( event: React.FocusEvent< HTMLInputElement > ) => { - if ( ! onFocus ) { - return; - } - onFocus( event, { side } ); - }; - - const handleOnValueChange = ( side: GroupedSide, next?: string ) => { - if ( ! onChange ) { - return; - } - const nextValues = { ...values }; - const isNumeric = next !== undefined && ! isNaN( parseFloat( next ) ); - const nextValue = isNumeric ? next : undefined; - - if ( side === 'vertical' ) { - nextValues.top = nextValue; - nextValues.bottom = nextValue; - } - - if ( side === 'horizontal' ) { - nextValues.left = nextValue; - nextValues.right = nextValue; - } - - onChange( nextValues ); - }; - - const createHandleOnUnitChange = - ( side: GroupedSide ) => ( next?: string ) => { - const newUnits = { ...selectedUnits }; - - if ( side === 'vertical' ) { - newUnits.top = next; - newUnits.bottom = next; - } - - if ( side === 'horizontal' ) { - newUnits.left = next; - newUnits.right = next; - } - - setSelectedUnits( newUnits ); - }; - - // Filter sides if custom configuration provided, maintaining default order. - const filteredSides = sides?.length - ? groupedSides.filter( ( side ) => sides.includes( side ) ) - : groupedSides; - - return ( - <> - { filteredSides.map( ( side ) => { - const [ parsedQuantity, parsedUnit ] = - parseQuantityAndUnitFromRawValue( - side === 'vertical' ? values.top : values.left - ); - const selectedUnit = - side === 'vertical' - ? selectedUnits.top - : selectedUnits.left; - - const inputId = [ generatedId, side ].join( '-' ); - - return ( - - - - - handleOnValueChange( side, newValue ) - } - onUnitChange={ createHandleOnUnitChange( - side - ) } - onFocus={ createHandleOnFocus( side ) } - label={ LABELS[ side ] } - hideLabelFromVision - key={ side } - /> - - - handleOnValueChange( - side, - newValue !== undefined - ? [ - newValue, - selectedUnit ?? parsedUnit, - ].join( '' ) - : undefined - ) - } - min={ 0 } - max={ - CUSTOM_VALUE_SETTINGS[ selectedUnit ?? 'px' ] - ?.max ?? 10 - } - step={ - CUSTOM_VALUE_SETTINGS[ selectedUnit ?? 'px' ] - ?.step ?? 0.1 - } - value={ parsedQuantity ?? 0 } - withInputField={ false } - /> - - ); - } ) } - - ); -} diff --git a/packages/components/src/box-control/index.tsx b/packages/components/src/box-control/index.tsx index 46cf2cd93a9444..7b219a33332d90 100644 --- a/packages/components/src/box-control/index.tsx +++ b/packages/components/src/box-control/index.tsx @@ -9,13 +9,10 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { BaseControl } from '../base-control'; -import AllInputControl from './all-input-control'; -import InputControls from './input-controls'; -import AxialInputControls from './axial-input-controls'; +import InputControl from './input-control'; import LinkedButton from './linked-button'; import { Grid } from '../grid'; import { - FlexedBoxControlIcon, InputWrapper, ResetButton, LinkedButtonWrapper, @@ -26,6 +23,7 @@ import { getInitialSide, isValuesMixed, isValuesDefined, + ALL_SIDES, } from './utils'; import { useControlledState } from '../utils/hooks'; import type { @@ -176,8 +174,7 @@ function BoxControl( { { isLinked && ( - - + ) } { ! hasOneSide && ( @@ -189,12 +186,24 @@ function BoxControl( { ) } - { ! isLinked && splitOnAxis && ( - - ) } - { ! isLinked && ! splitOnAxis && ( - - ) } + { ! isLinked && + splitOnAxis && + [ 'horizontal', 'vertical' ].map( ( axis ) => ( + + ) ) } + { ! isLinked && + ! splitOnAxis && + ALL_SIDES.map( ( axis ) => ( + + ) ) } { allowReset && ( {}; + +function getAllowedSides( sides: BoxControlInputControlProps[ 'sides' ] ) { + const allowedSides: Set< keyof BoxControlValue > = new Set( + ! sides ? ALL_SIDES : [] + ); + sides?.forEach( ( allowedSide ) => { + if ( allowedSide === 'vertical' ) { + allowedSides.add( 'top' ); + allowedSides.add( 'bottom' ); + } else if ( allowedSide === 'horizontal' ) { + allowedSides.add( 'right' ); + allowedSides.add( 'left' ); + } else { + allowedSides.add( allowedSide ); + } + } ); + return allowedSides; +} + +function getSidesToModify( + side: BoxControlInputControlProps[ 'side' ], + sides: BoxControlInputControlProps[ 'sides' ], + isAlt?: boolean +) { + const allowedSides = getAllowedSides( sides ); + + let modifiedSides: ( keyof BoxControlValue )[] = []; + switch ( side ) { + case 'all': + modifiedSides = [ 'top', 'bottom', 'left', 'right' ]; + break; + case 'horizontal': + modifiedSides = [ 'left', 'right' ]; + break; + case 'vertical': + modifiedSides = [ 'top', 'bottom' ]; + break; + default: + modifiedSides = [ side ]; + } + + if ( isAlt ) { + switch ( side ) { + case 'top': + modifiedSides.push( 'bottom' ); + break; + case 'bottom': + modifiedSides.push( 'top' ); + break; + case 'left': + modifiedSides.push( 'left' ); + break; + case 'right': + modifiedSides.push( 'right' ); + break; + } + } + + return modifiedSides.filter( ( s ) => allowedSides.has( s ) ); +} + +export default function BoxInputControl( { + __next40pxDefaultSize, + onChange = noop, + onFocus = noop, + values, + selectedUnits, + setSelectedUnits, + sides, + side, + ...props +}: BoxControlInputControlProps ) { + const defaultValuesToModify = getSidesToModify( side, sides ); + + const handleOnFocus = ( event: React.FocusEvent< HTMLInputElement > ) => { + onFocus( event, { side } ); + }; + + const handleOnChange = ( nextValues: BoxControlValue ) => { + onChange( nextValues ); + }; + + const handleOnValueChange = ( + next?: string, + extra?: { event: React.SyntheticEvent< Element, Event > } + ) => { + const nextValues = { ...values }; + const isNumeric = next !== undefined && ! isNaN( parseFloat( next ) ); + const nextValue = isNumeric ? next : undefined; + const modifiedSides = getSidesToModify( + side, + sides, + /** + * Supports changing pair sides. For example, holding the ALT key + * when changing the TOP will also update BOTTOM. + */ + // @ts-expect-error - TODO: event.altKey is only present when the change event was + // triggered by a keyboard event. Should this feature be implemented differently so + // it also works with drag events? + !! extra?.event.altKey + ); + + modifiedSides.forEach( ( modifiedSide ) => { + nextValues[ modifiedSide ] = nextValue; + } ); + + handleOnChange( nextValues ); + }; + + const handleOnUnitChange = ( next?: string ) => { + const newUnits = { ...selectedUnits }; + defaultValuesToModify.forEach( ( modifiedSide ) => { + newUnits[ modifiedSide ] = next; + } ); + setSelectedUnits( newUnits ); + }; + + const mergedValue = getAllValue( + values, + selectedUnits, + defaultValuesToModify + ); + const hasValues = isValuesDefined( values ); + const isMixed = + hasValues && + isValuesMixed( values, selectedUnits, defaultValuesToModify ); + const mixedPlaceholder = isMixed ? __( 'Mixed' ) : undefined; + const [ parsedQuantity, parsedUnit ] = + parseQuantityAndUnitFromRawValue( mergedValue ); + const computedUnit = hasValues + ? parsedUnit + : selectedUnits[ defaultValuesToModify[ 0 ] ]; + const generatedId = useInstanceId( BoxInputControl, 'box-control-input' ); + const inputId = [ generatedId, side ].join( '-' ); + + return ( + + + + + handleOnValueChange( nextValue, extra ) + } + onUnitChange={ handleOnUnitChange } + onFocus={ handleOnFocus } + label={ LABELS[ side ] } + placeholder={ mixedPlaceholder } + hideLabelFromVision + /> + + + { + handleOnValueChange( + newValue !== undefined + ? [ newValue, computedUnit ].join( '' ) + : undefined + ); + } } + min={ 0 } + max={ CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ]?.max ?? 10 } + step={ + CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ]?.step ?? 0.1 + } + value={ parsedQuantity ?? 0 } + withInputField={ false } + /> + + ); +} diff --git a/packages/components/src/box-control/input-controls.tsx b/packages/components/src/box-control/input-controls.tsx deleted file mode 100644 index 8f4518d717dbed..00000000000000 --- a/packages/components/src/box-control/input-controls.tsx +++ /dev/null @@ -1,167 +0,0 @@ -/** - * WordPress dependencies - */ -import { useInstanceId } from '@wordpress/compose'; -/** - * Internal dependencies - */ -import Tooltip from '../tooltip'; -import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; -import { ALL_SIDES, CUSTOM_VALUE_SETTINGS, LABELS } from './utils'; -import { - FlexedBoxControlIcon, - FlexedRangeControl, - InputWrapper, - StyledUnitControl, -} from './styles/box-control-styles'; -import type { BoxControlInputControlProps, BoxControlValue } from './types'; - -const noop = () => {}; - -export default function BoxInputControls( { - __next40pxDefaultSize, - onChange = noop, - onFocus = noop, - values, - selectedUnits, - setSelectedUnits, - sides, - ...props -}: BoxControlInputControlProps ) { - const generatedId = useInstanceId( BoxInputControls, 'box-control-input' ); - - const createHandleOnFocus = - ( side: keyof BoxControlValue ) => - ( event: React.FocusEvent< HTMLInputElement > ) => { - onFocus( event, { side } ); - }; - - const handleOnChange = ( nextValues: BoxControlValue ) => { - onChange( nextValues ); - }; - - const handleOnValueChange = ( - side: keyof BoxControlValue, - next?: string, - extra?: { event: React.SyntheticEvent< Element, Event > } - ) => { - const nextValues = { ...values }; - const isNumeric = next !== undefined && ! isNaN( parseFloat( next ) ); - const nextValue = isNumeric ? next : undefined; - - nextValues[ side ] = nextValue; - - /** - * Supports changing pair sides. For example, holding the ALT key - * when changing the TOP will also update BOTTOM. - */ - // @ts-expect-error - TODO: event.altKey is only present when the change event was - // triggered by a keyboard event. Should this feature be implemented differently so - // it also works with drag events? - if ( extra?.event.altKey ) { - switch ( side ) { - case 'top': - nextValues.bottom = nextValue; - break; - case 'bottom': - nextValues.top = nextValue; - break; - case 'left': - nextValues.right = nextValue; - break; - case 'right': - nextValues.left = nextValue; - break; - } - } - - handleOnChange( nextValues ); - }; - - const createHandleOnUnitChange = - ( side: keyof BoxControlValue ) => ( next?: string ) => { - const newUnits = { ...selectedUnits }; - newUnits[ side ] = next; - setSelectedUnits( newUnits ); - }; - - // Filter sides if custom configuration provided, maintaining default order. - const filteredSides = sides?.length - ? ALL_SIDES.filter( ( side ) => sides.includes( side ) ) - : ALL_SIDES; - - return ( - <> - { filteredSides.map( ( side ) => { - const [ parsedQuantity, parsedUnit ] = - parseQuantityAndUnitFromRawValue( values[ side ] ); - - const computedUnit = values[ side ] - ? parsedUnit - : selectedUnits[ side ]; - - const inputId = [ generatedId, side ].join( '-' ); - - return ( - - - - - handleOnValueChange( - side, - nextValue, - extra - ) - } - onUnitChange={ createHandleOnUnitChange( - side - ) } - onFocus={ createHandleOnFocus( side ) } - label={ LABELS[ side ] } - hideLabelFromVision - /> - - - { - handleOnValueChange( - side, - newValue !== undefined - ? [ newValue, computedUnit ].join( '' ) - : undefined - ); - } } - min={ 0 } - max={ - CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ] - ?.max ?? 10 - } - step={ - CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ] - ?.step ?? 0.1 - } - value={ parsedQuantity ?? 0 } - withInputField={ false } - /> - - ); - } ) } - - ); -} diff --git a/packages/components/src/box-control/types.ts b/packages/components/src/box-control/types.ts index 16b69dfe70a64a..46e209d2a894d6 100644 --- a/packages/components/src/box-control/types.ts +++ b/packages/components/src/box-control/types.ts @@ -112,6 +112,7 @@ export type BoxControlInputControlProps = UnitControlPassthroughProps & { setSelectedUnits: React.Dispatch< React.SetStateAction< BoxControlValue > >; sides: BoxControlProps[ 'sides' ]; values: BoxControlValue; + side: keyof typeof LABELS; }; export type BoxControlIconProps = { diff --git a/packages/components/src/box-control/utils.ts b/packages/components/src/box-control/utils.ts index 73c7f4a6a46cfb..6fcbaf932de3de 100644 --- a/packages/components/src/box-control/utils.ts +++ b/packages/components/src/box-control/utils.ts @@ -12,6 +12,7 @@ import type { BoxControlValue, CustomValueUnits, } from './types'; +import deprecated from '@wordpress/deprecated'; export const CUSTOM_VALUE_SETTINGS: CustomValueUnits = { px: { max: 300, step: 1 }, @@ -50,7 +51,6 @@ export const LABELS = { bottom: __( 'Bottom side' ), left: __( 'Left side' ), right: __( 'Right side' ), - mixed: __( 'Mixed' ), vertical: __( 'Top and bottom sides' ), horizontal: __( 'Left and right sides' ), }; @@ -239,6 +239,8 @@ export function normalizeSides( sides: BoxControlProps[ 'sides' ] ) { * Applies a value to an object representing top, right, bottom and left sides * while taking into account any custom side configuration. * + * @deprecated + * * @param currentValues The current values for each side. * @param newValue The value to apply to the sides object. * @param sides Array defining valid sides. @@ -250,6 +252,9 @@ export function applyValueToSides( newValue?: string, sides?: BoxControlProps[ 'sides' ] ): BoxControlValue { + deprecated( 'applyValueToSides', { + since: '6.8', + } ); const newValues = { ...currentValues }; if ( sides?.length ) { From fb91c42a7353a4f23b8d8efe97456ce0f2f597f7 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 5 Dec 2024 13:33:23 +0100 Subject: [PATCH 02/11] Add Changelog entry --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 249d4f3f65b936..dffc595e39995b 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -19,6 +19,7 @@ - `MenuItem`: Increase height to 40px ([#67435](https://github.com/WordPress/gutenberg/pull/67435)). - `MenuItemsChoice`: Increase option height to 40px ([#67435](https://github.com/WordPress/gutenberg/pull/67435)). - `Navigation`: Fix active item hover color ([#67732](https://github.com/WordPress/gutenberg/pull/67732)). +- `BoxControl`: Refactor internals to unify the different combinations of sides. ([#67626](https://github.com/WordPress/gutenberg/pull/67626)) ### Deprecations From 57a3fdc8ecf31bafb5aee0385013c795676c6a0d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 6 Dec 2024 12:19:05 +0100 Subject: [PATCH 03/11] Prevent the mixed placeholder form appearing when we're only modifying a single side --- packages/components/src/box-control/input-control.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/box-control/input-control.tsx b/packages/components/src/box-control/input-control.tsx index e0d0685e8e089f..b39040470dcf74 100644 --- a/packages/components/src/box-control/input-control.tsx +++ b/packages/components/src/box-control/input-control.tsx @@ -151,6 +151,7 @@ export default function BoxInputControl( { const hasValues = isValuesDefined( values ); const isMixed = hasValues && + defaultValuesToModify.length > 1 && isValuesMixed( values, selectedUnits, defaultValuesToModify ); const mixedPlaceholder = isMixed ? __( 'Mixed' ) : undefined; const [ parsedQuantity, parsedUnit ] = From d50eb95976b11146f7faecbf6f8a3da5de0e1bc9 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 6 Dec 2024 12:27:56 +0100 Subject: [PATCH 04/11] Fix the arbitrary sides use-case and swap sides --- packages/components/src/box-control/index.tsx | 7 ++--- .../src/box-control/input-control.tsx | 20 +------------- packages/components/src/box-control/utils.ts | 27 +++++++++++++++++++ 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/packages/components/src/box-control/index.tsx b/packages/components/src/box-control/index.tsx index 7b219a33332d90..1c453913da3583 100644 --- a/packages/components/src/box-control/index.tsx +++ b/packages/components/src/box-control/index.tsx @@ -23,7 +23,7 @@ import { getInitialSide, isValuesMixed, isValuesDefined, - ALL_SIDES, + getAllowedSides, } from './utils'; import { useControlledState } from '../utils/hooks'; import type { @@ -160,6 +160,7 @@ function BoxControl( { __next40pxDefaultSize, size: undefined, } ); + const sidesToRender = getAllowedSides( sides ); return ( ( + [ 'vertical', 'horizontal' ].map( ( axis ) => ( ( + Array.from( sidesToRender ).map( ( axis ) => ( {}; -function getAllowedSides( sides: BoxControlInputControlProps[ 'sides' ] ) { - const allowedSides: Set< keyof BoxControlValue > = new Set( - ! sides ? ALL_SIDES : [] - ); - sides?.forEach( ( allowedSide ) => { - if ( allowedSide === 'vertical' ) { - allowedSides.add( 'top' ); - allowedSides.add( 'bottom' ); - } else if ( allowedSide === 'horizontal' ) { - allowedSides.add( 'right' ); - allowedSides.add( 'left' ); - } else { - allowedSides.add( allowedSide ); - } - } ); - return allowedSides; -} - function getSidesToModify( side: BoxControlInputControlProps[ 'side' ], sides: BoxControlInputControlProps[ 'sides' ], diff --git a/packages/components/src/box-control/utils.ts b/packages/components/src/box-control/utils.ts index 6fcbaf932de3de..2d064d38f53b41 100644 --- a/packages/components/src/box-control/utils.ts +++ b/packages/components/src/box-control/utils.ts @@ -8,6 +8,7 @@ import { __ } from '@wordpress/i18n'; */ import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; import type { + BoxControlInputControlProps, BoxControlProps, BoxControlValue, CustomValueUnits, @@ -275,3 +276,29 @@ export function applyValueToSides( return newValues; } + +/** + * Return the allowed sides based on the sides configuration. + * + * @param sides Sides configuration. + * @return Allowed sides. + */ +export function getAllowedSides( + sides: BoxControlInputControlProps[ 'sides' ] +) { + const allowedSides: Set< keyof BoxControlValue > = new Set( + ! sides ? ALL_SIDES : [] + ); + sides?.forEach( ( allowedSide ) => { + if ( allowedSide === 'vertical' ) { + allowedSides.add( 'top' ); + allowedSides.add( 'bottom' ); + } else if ( allowedSide === 'horizontal' ) { + allowedSides.add( 'right' ); + allowedSides.add( 'left' ); + } else { + allowedSides.add( allowedSide ); + } + } ); + return allowedSides; +} From 9d69dcac62f9b9c8f76bd90e9279cf5baf373f2f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 6 Dec 2024 12:31:34 +0100 Subject: [PATCH 05/11] Apply small simplification Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --- packages/components/src/box-control/input-control.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/components/src/box-control/input-control.tsx b/packages/components/src/box-control/input-control.tsx index b2fcd2cb105978..58d441209c1b7c 100644 --- a/packages/components/src/box-control/input-control.tsx +++ b/packages/components/src/box-control/input-control.tsx @@ -156,9 +156,7 @@ export default function BoxInputControl( { id={ inputId } isPressEnterToChange value={ mergedValue } - onChange={ ( nextValue, extra ) => - handleOnValueChange( nextValue, extra ) - } + onChange={ handleOnValueChange } onUnitChange={ handleOnUnitChange } onFocus={ handleOnFocus } label={ LABELS[ side ] } From e06583a66606816d9e2e7d49a76ceeeacffd6a9a Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 6 Dec 2024 13:56:20 +0100 Subject: [PATCH 06/11] Simplify merging logic --- packages/components/src/box-control/index.tsx | 4 +- .../src/box-control/input-control.tsx | 12 +-- packages/components/src/box-control/utils.ts | 87 ++++++------------- 3 files changed, 34 insertions(+), 69 deletions(-) diff --git a/packages/components/src/box-control/index.tsx b/packages/components/src/box-control/index.tsx index 1c453913da3583..279dfa55eafe38 100644 --- a/packages/components/src/box-control/index.tsx +++ b/packages/components/src/box-control/index.tsx @@ -21,7 +21,7 @@ import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; import { DEFAULT_VALUES, getInitialSide, - isValuesMixed, + isValueMixed, isValuesDefined, getAllowedSides, } from './utils'; @@ -95,7 +95,7 @@ function BoxControl( { const [ isDirty, setIsDirty ] = useState( hasInitialValue ); const [ isLinked, setIsLinked ] = useState( - ! hasInitialValue || ! isValuesMixed( inputValues ) || hasOneSide + ! hasInitialValue || ! isValueMixed( inputValues ) || hasOneSide ); const [ side, setSide ] = useState< BoxControlIconProps[ 'side' ] >( diff --git a/packages/components/src/box-control/input-control.tsx b/packages/components/src/box-control/input-control.tsx index 58d441209c1b7c..992459a5587155 100644 --- a/packages/components/src/box-control/input-control.tsx +++ b/packages/components/src/box-control/input-control.tsx @@ -12,9 +12,9 @@ import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; import { CUSTOM_VALUE_SETTINGS, getAllowedSides, - getAllValue, + getMergedValue, + isValueMixed, isValuesDefined, - isValuesMixed, LABELS, } from './utils'; import { @@ -125,16 +125,12 @@ export default function BoxInputControl( { setSelectedUnits( newUnits ); }; - const mergedValue = getAllValue( - values, - selectedUnits, - defaultValuesToModify - ); + const mergedValue = getMergedValue( values, defaultValuesToModify ); const hasValues = isValuesDefined( values ); const isMixed = hasValues && defaultValuesToModify.length > 1 && - isValuesMixed( values, selectedUnits, defaultValuesToModify ); + isValueMixed( values, defaultValuesToModify ); const mixedPlaceholder = isMixed ? __( 'Mixed' ) : undefined; const [ parsedQuantity, parsedUnit ] = parseQuantityAndUnitFromRawValue( mergedValue ); diff --git a/packages/components/src/box-control/utils.ts b/packages/components/src/box-control/utils.ts index 2d064d38f53b41..c37ea4cc126300 100644 --- a/packages/components/src/box-control/utils.ts +++ b/packages/components/src/box-control/utils.ts @@ -6,7 +6,6 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { parseQuantityAndUnitFromRawValue } from '../unit-control/utils'; import type { BoxControlInputControlProps, BoxControlProps, @@ -83,56 +82,46 @@ function mode< T >( arr: T[] ) { } /** - * Gets the 'all' input value and unit from values data. + * Gets the merged input value and unit from values data. * * @param values Box values. - * @param selectedUnits Box units. * @param availableSides Available box sides to evaluate. * * @return A value + unit for the 'all' input. */ -export function getAllValue( +export function getMergedValue( values: BoxControlValue = {}, - selectedUnits?: BoxControlValue, availableSides: BoxControlProps[ 'sides' ] = ALL_SIDES ) { const sides = normalizeSides( availableSides ); - const parsedQuantitiesAndUnits = sides.map( ( side ) => - parseQuantityAndUnitFromRawValue( values[ side ] ) - ); - const allParsedQuantities = parsedQuantitiesAndUnits.map( - ( value ) => value[ 0 ] ?? '' - ); - const allParsedUnits = parsedQuantitiesAndUnits.map( - ( value ) => value[ 1 ] - ); - - const commonQuantity = allParsedQuantities.every( - ( v ) => v === allParsedQuantities[ 0 ] - ) - ? allParsedQuantities[ 0 ] - : ''; - - /** - * The typeof === 'number' check is important. On reset actions, the incoming value - * may be null or an empty string. - * - * Also, the value may also be zero (0), which is considered a valid unit value. - * - * typeof === 'number' is more specific for these cases, rather than relying on a - * simple truthy check. - */ - let commonUnit; - if ( typeof commonQuantity === 'number' ) { - 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 ); + if ( + sides.every( + ( side: keyof BoxControlValue ) => + values[ side ] === values[ sides[ 0 ] ] + ) + ) { + return values[ sides[ 0 ] ]; } - return [ commonQuantity, commonUnit ].join( '' ); + return undefined; +} + +/** + * Checks if the values are mixed. + * + * @param values Box values. + * @param availableSides Available box sides to evaluate. + * @return Whether the values are mixed. + */ +export function isValueMixed( + values: BoxControlValue = {}, + availableSides: BoxControlProps[ 'sides' ] = ALL_SIDES +) { + const sides = normalizeSides( availableSides ); + return sides.some( + ( side: keyof BoxControlValue ) => + values[ side ] !== values[ sides[ 0 ] ] + ); } /** @@ -151,26 +140,6 @@ export function getAllUnitFallback( selectedUnits?: BoxControlValue ) { return mode( filteredUnits ); } -/** - * Checks to determine if values are mixed. - * - * @param values Box values. - * @param selectedUnits Box units. - * @param sides Available box sides to evaluate. - * - * @return Whether values are mixed. - */ -export function isValuesMixed( - values: BoxControlValue = {}, - selectedUnits?: BoxControlValue, - sides: BoxControlProps[ 'sides' ] = ALL_SIDES -) { - const allValue = getAllValue( values, selectedUnits, sides ); - const isMixed = isNaN( parseFloat( allValue ) ); - - return isMixed; -} - /** * Checks to determine if values are defined. * From e3db93f5dad3526d5d3f076572c725cae83c7285 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 9 Dec 2024 11:58:47 +0100 Subject: [PATCH 07/11] Fix edge case --- packages/components/src/box-control/input-control.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/components/src/box-control/input-control.tsx b/packages/components/src/box-control/input-control.tsx index 992459a5587155..6678cf1fdbd31d 100644 --- a/packages/components/src/box-control/input-control.tsx +++ b/packages/components/src/box-control/input-control.tsx @@ -139,6 +139,8 @@ export default function BoxInputControl( { : selectedUnits[ defaultValuesToModify[ 0 ] ]; const generatedId = useInstanceId( BoxInputControl, 'box-control-input' ); const inputId = [ generatedId, side ].join( '-' ); + const usedValue = + mergedValue === undefined && computedUnit ? computedUnit : mergedValue; return ( @@ -151,7 +153,7 @@ export default function BoxInputControl( { className="component-box-control__unit-control" id={ inputId } isPressEnterToChange - value={ mergedValue } + value={ usedValue } onChange={ handleOnValueChange } onUnitChange={ handleOnUnitChange } onFocus={ handleOnFocus } From 5c8d2014edc87e7a440d87e8a1ef384e825f2317 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 9 Dec 2024 17:17:43 +0100 Subject: [PATCH 08/11] Disable units when mixed units are used --- packages/components/src/box-control/input-control.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/components/src/box-control/input-control.tsx b/packages/components/src/box-control/input-control.tsx index 6678cf1fdbd31d..9086cebedc2749 100644 --- a/packages/components/src/box-control/input-control.tsx +++ b/packages/components/src/box-control/input-control.tsx @@ -131,7 +131,6 @@ export default function BoxInputControl( { hasValues && defaultValuesToModify.length > 1 && isValueMixed( values, defaultValuesToModify ); - const mixedPlaceholder = isMixed ? __( 'Mixed' ) : undefined; const [ parsedQuantity, parsedUnit ] = parseQuantityAndUnitFromRawValue( mergedValue ); const computedUnit = hasValues @@ -139,8 +138,15 @@ export default function BoxInputControl( { : selectedUnits[ defaultValuesToModify[ 0 ] ]; const generatedId = useInstanceId( BoxInputControl, 'box-control-input' ); const inputId = [ generatedId, side ].join( '-' ); + const isMixedUnit = + defaultValuesToModify.length > 1 && + mergedValue === undefined && + defaultValuesToModify.some( + ( s ) => selectedUnits[ s ] !== computedUnit + ); const usedValue = mergedValue === undefined && computedUnit ? computedUnit : mergedValue; + const mixedPlaceholder = isMixed || isMixedUnit ? __( 'Mixed' ) : undefined; return ( @@ -153,6 +159,7 @@ export default function BoxInputControl( { className="component-box-control__unit-control" id={ inputId } isPressEnterToChange + disableUnits={ isMixed || isMixedUnit } value={ usedValue } onChange={ handleOnValueChange } onUnitChange={ handleOnUnitChange } From e04e4db4f61039e928c2f89fb621145c74e8b9af Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 10 Dec 2024 09:45:22 +0100 Subject: [PATCH 09/11] Move changelog entry to internal --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index dffc595e39995b..6a7b2545c2e68c 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -19,7 +19,6 @@ - `MenuItem`: Increase height to 40px ([#67435](https://github.com/WordPress/gutenberg/pull/67435)). - `MenuItemsChoice`: Increase option height to 40px ([#67435](https://github.com/WordPress/gutenberg/pull/67435)). - `Navigation`: Fix active item hover color ([#67732](https://github.com/WordPress/gutenberg/pull/67732)). -- `BoxControl`: Refactor internals to unify the different combinations of sides. ([#67626](https://github.com/WordPress/gutenberg/pull/67626)) ### Deprecations @@ -41,6 +40,7 @@ - Upgraded `@ariakit/react` (v0.4.15) and `@ariakit/test` (v0.4.7) ([#67404](https://github.com/WordPress/gutenberg/pull/67404)). - `ToolbarButton`: Set size to "compact" ([#67440](https://github.com/WordPress/gutenberg/pull/67440)). - `SlotFill`: remove manual rerenders from the portal `Fill` component ([#67471](https://github.com/WordPress/gutenberg/pull/67471)). +- `BoxControl`: Refactor internals to unify the different combinations of sides ([#67626](https://github.com/WordPress/gutenberg/pull/67626)). ### Bug Fixes From 0c29c0624ee2a9991598de0b497bee318a869b47 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 10 Dec 2024 09:47:59 +0100 Subject: [PATCH 10/11] Document the props properly --- packages/components/src/box-control/types.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/components/src/box-control/types.ts b/packages/components/src/box-control/types.ts index 46e209d2a894d6..73de68d1bd513a 100644 --- a/packages/components/src/box-control/types.ts +++ b/packages/components/src/box-control/types.ts @@ -110,8 +110,15 @@ export type BoxControlInputControlProps = UnitControlPassthroughProps & { ) => void; selectedUnits: BoxControlValue; setSelectedUnits: React.Dispatch< React.SetStateAction< BoxControlValue > >; - sides: BoxControlProps[ 'sides' ]; values: BoxControlValue; + /** + * Collection of sides to allow control of. If omitted or empty, all sides will be available. + */ + sides: BoxControlProps[ 'sides' ]; + /** + * Side represents the current side being rendered by the input. + * It can be a concrete side like: left, right, top, bottom or a combined one like: horizontal, vertical. + */ side: keyof typeof LABELS; }; From e3ff02f4fe0f6e3266dfb2c56e0086e990e6dece Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 10 Dec 2024 09:49:22 +0100 Subject: [PATCH 11/11] Add due date for deprecation --- packages/components/src/box-control/utils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/box-control/utils.ts b/packages/components/src/box-control/utils.ts index c37ea4cc126300..111451790e35d5 100644 --- a/packages/components/src/box-control/utils.ts +++ b/packages/components/src/box-control/utils.ts @@ -224,6 +224,7 @@ export function applyValueToSides( ): BoxControlValue { deprecated( 'applyValueToSides', { since: '6.8', + version: '7.0', } ); const newValues = { ...currentValues };