UnitControl: Value on the display is truncated on blur

[t-hamano]

Description

When the value is placed in UnitControl and moved away from the field, the decimal point value specified in the step of each unit is truncated.
However, the decimal point values are retained as attributes, and the input values are displayed correctly when the browser is reloaded.

This behavior may lead to the misunderstanding that the specified decimal point cannot be entered.
I think that the decimal point should not be truncated on blur to display the same value as the attributes.

Step-by-step reproduction instructions

• Insert a block that has UnitControl (e.g. column block).
• In UnitControl, set numbers and units that include a decimal point as follows:
  • 10.1111 px
  • 10.1111 %
  • 10.1111 em
• Confirm that the numbers are rounded off when they leave the input field, as shown below:
  • 10.1111 px > 10 px
  • 10.1111 % > 10.1 %
  • 10.1111 em > 10.11 em
• Save the post, reload the browser, and confirm that the value of the control is not rounded.

Screenshots, screen recording, code snippet

a2d48445f35f6b5839bdeb7a39e5ebfa.mp4

Environment info

• WordPress version: 6.0 RC4
• Gutenberg Version: 13.2.2
• Theme: Twenty Twenty Two

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

[mirka]

After some testing, my impression is that this is a bug somewhere in process where the block commits the value to a block attribute. In the screen recording, it looks like the block attribute isn't updated at all on blur.

On the component side, it seems to be working as expected. Decimal precision is dictated by these step values:

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

Lines 14 to 26 in 77d5d91

	const allUnits: Record< string, WPUnitControlUnit > = {
	px: {
	value: 'px',
	label: isWeb ? 'px' : __( 'Pixels (px)' ),
	a11yLabel: __( 'Pixels (px)' ),
	step: 1,
	},
	'%': {
	value: '%',
	label: isWeb ? '%' : __( 'Percentage (%)' ),
	a11yLabel: __( 'Percent (%)' ),
	step: 0.1,
	},

And the onChange is firing with the expected values:

CleanShot.2022-05-31.at.23.49.32.mp4

If anything, I would think the component's behavior (i.e. respect the step precision) is the intended behavior. I don't think the blocks are intending to save those high-precision numbers.

[t-hamano]

Thank you for the test!
I agree with you about the component's behavior is the intended behavior.

However, I'm concerned that users might be confused if the block attributes are adjusted to the component specifications, because the values that could previously be entered on the block side will now be truncated.

For example, if the unit is a percentage, some users might enter two decimal places as shown below, but the component will remove the two decimal places.

• 56.25vw: Video Aspect Ratio
• 33.33%, 66.66%: When we want to divide a block into exactly three parts

If the attributes of the block are to be adapted to the component's specifications, how about a little finer precision in the step?

[mirka]

First of all, there is a bug (likely in the blocks) that doesn't respect the canonical value given by the component. So this should be investigated and fixed. Should this issue here be about fixing this bug specifically?

For any custom usage of UnitControl, consumers are free to pass their own array of units, including a step precision of their choice.

If you would like to propose changes in the step precision of the default units bundled with the component, you could make an issue or PR for that. Personally, I agree that two decimal places for %, vw, etc. is reasonable. For reference, the reasoning for the current precision values can be seen in #32692.

[t-hamano]

As you explained, I would like to solve the following two problems in this issue:

• The block attributes are not updating correctly when blurred
• Is the component's step value appropriate?

Since it is complicated to address both issues at the same place, should I close this issue and propose each as a separate issue or PR?

[mirka]

Since it is complicated to address both issues at the same place, should I close this issue and propose each as a separate issue or PR?

Sounds good!

[vena]

at some point, UnitControl appears to have broken the units value spec which uses an array of objects described in the documentation, and all units defined in such a way are treated as "px" with this step truncating.

https://developer.wordpress.org/block-editor/reference-guides/components/unit-control/

const Example = () => {
    const [ value, setValue ] = useState( '10px' );
 
    const units = [
        { value: 'px', label: 'px', default: 0 },
        { value: '%', label: '%', default: 10 },
        { value: 'em', label: 'em', default: 0 },
    ];
 
    return <UnitControl onChange={ setValue } value={ value } units={units} />;
};

Both % and em values are rounded to an integer on blur when units are specified in this way.

Is this a bug or does the documentation need to be updated? Possibly deserving of a separate issue report...

[mirka]

Is this a bug or does the documentation need to be updated? Possibly deserving of a separate issue report...

I'm not sure I follow about the spec having changed, but please do open a PR or issue if you think the docs can be improved. Is this specifically about the step property not being documented? I do think the example in the readme would be easier to understand if the units were more clearly "custom", like the ones in the Storybook example. The examples should include step and a11yLabel properties as well, so they are more discoverable.

[vena]

Well, I'm not sure. If you implement the example I copied above from the developer.wordpress.org docs, both the % and em units are rounded on blur to an integer (so, i suppose, step: 1). It seems odd that % and em are treated as "custom" rather than inheriting properties they're missing, but this seems like a recent change. I have blocks using UnitControl with units defined similarly which did not round floats on blur for those units until recently (not sure if it was a 6.0 change, but that seems logical).

I guess the question is, is this intended behavior? If so, then sure, I'll see about opening an issue to update the docs at developer.wordpress.org. I've actually never seen the documentation link you posted, and don't know where I'd have found that otherwise...

[mirka]

It seems odd that % and em are treated as "custom" rather than inheriting properties they're missing, but this seems like a recent change. I have blocks using UnitControl with units defined similarly which did not round floats on blur for those units until recently (not sure if it was a 6.0 change, but that seems logical).

@ciampo Do you remember if the units prop ever "inherited" unit properties from allUnits? Nothing in the current code suggests that kind of intent or behavior 🤔 The closest I can find is getUnitsWithCurrentUnit(), which ensures that the currently selected unit is loaded from allUnits if it isn't included in units.

I wonder if that perceived change in behavior is due to the bug fix in #39186, where values are now (properly) validated/rounded on blur.

[ciampo]

@ciampo Do you remember if the units prop ever "inherited" unit properties from allUnits? Nothing in the current code suggests that kind of intent or behavior 🤔

I don't think so — from what I know, I've never seen units "inheriting" from allUnits.

The closest I can find is getUnitsWithCurrentUnit(), which ensures that the currently selected unit is loaded from allUnits if it isn't included in units.

That behavior / utility function was introduced in #34768 — but it looks like even before that PR, the general behavior of the units prop still didn't include inhering from allUnits(cc'ing @andrewserong who worked on that original PR in case I'm missing anything).

I wonder if that perceived change in behavior is due to the bug fix in #39186, where values are now (properly) validated/rounded on blur.

That could well be the case, good thinking!

We also need to understand the potential mis-alignment of documentation between the READMEs and Storybook examples in this package, and the docs on developer.wordpress.com

[vena]

I wonder if that perceived change in behavior is due to the bug fix in #39186, where values are now (properly) validated/rounded on blur.

i'd say it's less that it inherited anything, more that it's only recently we've seen UnitControl with a units prop defined like the developer.wordpress.com doc examples rounding the units we often use floats with, like em/rem/%

[andrewserong]

cc'ing @andrewserong who worked on that original PR in case I'm missing anything).

That's my understanding, too @ciampo — I don't think there was ever any merging of missing properties in the units array.