Fix: RangeControl does not validates the min and max properties

[jorgefilipecosta]

Description

This PR adds a simple min/max validation to RangeControl.
If the value is invalid it is temporarily saved in the local state and onChange is not called. During the blur event, the local state is cleared so if we had an invalid input we revert back to the last valid input. If the input is valid onChange is called as soon as the input happens.

Fixes: #12922
Related: #10791

How has this been tested?

Run the unit tests.
Verify the range controls in columns block, comments block, latest posts block and others present an expected behavior.

[aduth]

It took me a bit to figure out why we were doing it this way. I guess Number( '' ) coerces to 0, so we're differentiating?

Alternatively, we might consider a stricter parse, checking that the produced value is not NaN (isNaN).

For example, parseInt( '', 10 ) will produce NaN.

The main benefit I think is readability. Thoughts?

[jorgefilipecosta]

Hi @aduth,

It took me a bit to figure out why we were doing it this way. I guess Number( '' ) coerces to 0, so we're differentiating?

Exactly, the idea was to differentiate. I agree with your suggestion, using parseInt( '', 10 ) and isNaN improves readability and I updated the code.

[aduth]

Do we need to call onChange at all? If the attempted value wasn't valid, just reset the current state. The rendering parent should still think the value was the "original" since it never received any indication otherwise. Then, onChange is only ever called when the input value changes to something valid.

[jorgefilipecosta]

My idea was that deleting the input field would reset the field to the default value, in the same way pressing reset button does.
But that is not a requirement and just resetting to the previous value is also valid so I updated the code.

[aduth]

We should include a CHANGELOG.md entry.

[aduth]

Depending if we wanted to support negative numbers, the following hypothetical unit test fails:

it( 'validates when provided a max or min of zero', () => {
	const onChange = jest.fn();
	const wrapper = getWrapper( { onChange, min: -100, value: 0 } );

	const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass(
		wrapper,
		'components-range-control__number'
	);

	TestUtils.Simulate.change(
		numberInputElement(),
		{
			target: { value: '1' },
		}
	);

	expect( onChange ).not.toHaveBeenCalled();
} );

[jorgefilipecosta]

Hi @aduth we should support negative numbers, the logic bug was solved and this test case and one to explicitly test negative ranges were added.

[gziolo]

This PR needs to be updated with the latest changes from master and

We should include a CHANGELOG.md entry.

[jorgefilipecosta]

The changelog entry was added.

[aduth]

LGTM 👍

[kadencewp]

Hey, @jorgefilipecosta I'm not sure if this is the right place to post but this change breaks the ability to set the step of the range control to 0.1. Being able to set the step is really useful for opacity range, em sizes etc. This update breaks a number of controls in Kadence Blocks I'm sure others as well. Can I help fix this?

[kadencewp]

@jorgefilipecosta Could just be a matter of changing line 51 to this:

let newNumericValue = parseInt( newValue, 10 );
if ( props.step !== undefined && props.step < 1 ) {
     newNumericValue = parseFloat( newValue );
}

Would it be helpful if I submitted a pull request?

[jorgefilipecosta]

Thank you for reporting this issue @kadencethemes, I submitted a fix for it at #14322. I'm sorry for the trouble caused.