NumberControl: Infinity isn't a valid value for min and max

[afercia]

Description

Discovered while investigating #68599 and similar issues.

By default, the NumberControl component and other components that extend it e.g. UnitControl > ValueInpu > NumberControl use Infinity and -Infinity for the min and max HTMl attributes values.

That is: the rendered HTML attribute values is:
max="Infinity"
min="-Infinity"

Those aren't valid values, it's invalid HTML.

I'm not sure what was the reasoning about using Infinity as default value. Shouldn't the max and min attributes be simply omitted to allow infinite values?

Reference:

https://html.spec.whatwg.org/multipage/input.html#the-min-and-max-attributes
https://html.spec.whatwg.org/multipage/input.html#number-state-(type=number)

The min attribute, if specified, must have a value that is a valid floating-point number. The max attribute, if specified, must have a value that is a valid floating-point number.

Cc @WordPress/gutenberg-components

Step-by-step reproduction instructions

• Go to the Storybook at https://wordpress.github.io/gutenberg/?path=/story/components-experimental-numbercontrol--default
• Inspect the source and observe the values are max="Infinity" and min="-Infinity".

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

Please confirm which theme type you used for testing.

• Block
• Classic
• Hybrid (e.g. classic with theme.json)
• Not sure

[hbhalodia]

Hi @afercia, Looks like this was added while the component was initially being created - 91e624a, and from there only this values are being used.

But yes, we need an eye from @WordPress/gutenberg-components team on why we have added an Infinity and -Infinity options.

[mirka]

Looks like the Infinity values are used to calculate stuff in JS, which is fine, but yes I think they should be normalized to proper HTML attribute values when they're passed to the input element:

gutenberg/packages/components/src/number-control/index.tsx

Lines 232 to 233 in af2915f

	max={ max }
	min={ min }

[Infinite-Null]

You’re right, @mirka! Using "Infinity" and "-Infinity" as HTML attribute values for min and max is invalid.

For an unrestricted numeric input, the best practices might be:

1. To omit the min and max attributes entirely.
2. We can use very large numbers to effectively remove practical restrictions, such as:
   • min="-999999999999" max="999999999999"
3. Alternatively, using ±9007199254740991 aligns with JavaScript’s maximum safe integer values.

I think Both the second and third approaches seem to work well, but I’m a bit unsure about which one would be the best choice.

If these seem to work. I would like to work on this!

[mirka]

1. To omit the min and max attributes entirely.

Why not the first approach, as @afercia suggested in the description? I think that would be the standard way to do it.

max={ max === Infinity ? undefined : max } or something like that (untested).

[Infinite-Null]

Missed that 😅. Thanks for the suggestion! I'll create a PR and test it.

[Infinite-Null]

Hi @mirka, @afercia, and @hbhalodia,

I've created the PR implementing the solution and tested it. Would appreciate it if you could test and review the PR.

Thank you!

[afercia]

I'd agree they should be entirely omitted.

Looks like the Infinity values are used to calculate stuff in JS

@mirka not sure where the Infinity is actually used? I may be missing something here.

[mirka]

@afercia I was thinking about this part:

gutenberg/packages/components/src/number-control/index.tsx

Lines 82 to 91 in e992acf

	const constrainValue = (
	value: number | string,
	stepOverride?: number
	) => {
	// When step is "any" clamp the value, otherwise round and clamp it.
	// Use '' + to convert to string for use in input value attribute.
	return isStepAny
	? '' + Math.min( max, Math.max( min, ensureNumber( value ) ) )
	: '' + roundClamp( value, min, max, stepOverride ?? baseStep );
	};

To be fair, it looks like roundClamp() can already handle undefined min/max values, but Math.min/Math.max cannot.