Box Control range slider doesn't set a unit initially

[albanyacademy]

Description

When using the __experimentalBoxControl with all values as null, the unit for each side is only set when the UnitControl itself is changed. When one uses the RangeControl first and drags to a number, the value received is just the number itself without a unit attached.

Step-by-step reproduction instructions

1. Use a box control with a default value empty/null;
2. set the onChange to console log the value;
3. observe that when only dragging the range slider the value returned is only a number
4. observe that unit only gets added when unitcontrol is used

Screenshots, screen recording, code snippet

https://www.screencast.com/t/I3RDlCL9ht2

Environment info

wp 6.5.2

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[t-hamano]

Thanks for the report.
I don't know if this specification was intended or a bug, but I was able to reproduce the problem on Storybook.

53a4f7fe277820c0a84a8a04d38698d7.mp4

[mirka]

Interesting. It might be debatable, but I think we can say this is a bug.

[Tropicalista]

Any news on this?

[albanyacademy]

Interesting. It might be debatable, but I think we can say this is a bug.

I mean, a number without a unit isn't usable, so I'd say without a doubt it's a bug 😛

[giannis-koulouris-bb]

I've been having the same issue for the past couple of months, still not fix.
My workaround was to create a function that checks if the unit exists, if not then i append it on the value

[im3dabasia]

Hi @t-hamano , I tried debugging and investigating the root cause of this issue and can say this as per my observations

1. The input box on the left works well, As soon as we input a value it has both the value and the unit.

I backtracked it and could say that the UnitControl passes the correct value to the onChangeProp() which has both the unit and value and this in return comes to this and gets executed perfectly as expected in this onChange handler in the BoxInputControl

gutenberg/packages/components/src/box-control/input-control.tsx

Lines 111 to 114 in 523c2ba

	const handleOnValueChange = (
	next?: string,
	extra?: { event: React.SyntheticEvent< Element, Event > }
	) => {

2. The case is different in the range control slider, here we have something like this defined as the onChange handler

gutenberg/packages/components/src/box-control/input-control.tsx

Lines 224 to 230 in 523c2ba

	handleOnValueChange(
	newValue !== undefined
	? [ newValue, computedUnit ].join( '' )
	: undefined
	);
	} }
	min={ isFinite( min ) ? min : 0 }

In this i observed that the root cause is computedUnit not assigned any value and is undefined thus we just get numbers only passed onto the handleOnValueChange of the BoxInputControl.

After a bit of more backtracking ...

gutenberg/packages/components/src/box-control/input-control.tsx

Lines 146 to 156 in 523c2ba

	const mergedValue = getMergedValue( values, defaultValuesToModify );
	const hasValues = isValuesDefined( values );
	const isMixed =
	hasValues &&
	defaultValuesToModify.length > 1 &&
	isValueMixed( values, defaultValuesToModify );
	const [ parsedQuantity, parsedUnit ] =
	parseQuantityAndUnitFromRawValue( mergedValue );
	const computedUnit = hasValues
	? parsedUnit
	: selectedUnits[ defaultValuesToModify[ 0 ] ];

I linked computedUnit depends on parsedUnit depends on mergedValue which depends on values this is being passed to BoxInputControl component as a prop, after this I lost track 😅 .

I am not quite sure of how to handle this bug in the most correct way possible. I have a temporary fix. This is not scaleable IMO.

I would appreciate your thoughts on this Issue.
Thank you in advance 🙇

Code sample with possible fix

  <FlexedRangeControl
        __nextHasNoMarginBottom
        __next40pxDefaultSize={ __next40pxDefaultSize }
        __shouldNotWarnDeprecated36pxSize
        aria-controls={ inputId }
        label={ LABELS[ side ] }
        hideLabelFromVision
        onChange={ ( newValue ) => {
        	handleOnValueChange(
        		newValue !== undefined
        			? [ newValue, computedUnit ].join( '' ) // Here we can do something like this computedUnit ?? 'px' same patterns as below 
        			: undefined
        	);
        } }
        min={ isFinite( min ) ? min : 0 }
        max={
        	CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ]
        		?.max ?? 10
        }
        step={
        	CUSTOM_VALUE_SETTINGS[ computedUnit ?? 'px' ]
        		?.step ?? 0.1
        }
        value={ parsedQuantity ?? 0 }
        withInputField={ false }
  />

[t-hamano]

I think the reason the input box works correctly is because it uses the first available unit as a fallback if the unit is not yet defined:

gutenberg/packages/components/src/unit-control/utils.ts

Lines 360 to 365 in 44dba38

	// If no unit is parsed from the raw value, or if the fallback unit is not
	// defined, use the first value from the list of allowed units as fallback.
	let unitToReturn = parsedUnit || fallbackUnit;
	if ( ! unitToReturn && hasUnits( allowedUnits ) ) {
	unitToReturn = allowedUnits[ 0 ].value;

In this i observed that the root cause is computedUnit not assigned any value and is undefined thus we just get numbers only passed onto the handleOnValueChange of the BoxInputControl.

Maybe the fallback logic is needed for the slider as well. In other words, if we provide a fallback unit here, selectedUnits will not be undefined, and as a result, computedUnit will not be undefined either.