From 6a9665a157fea63554c3a63fc7c10968703ccc0d Mon Sep 17 00:00:00 2001
From: Jorge Costa <jorge.costa@developer.pt>
Date: Sat, 16 Mar 2019 10:52:33 +0000
Subject: [PATCH] Fix: use checkValidity() to perform the validation in
RangeControl (#14322)
---
packages/block-library/src/columns/index.js | 1 +
packages/block-library/src/cover/edit.js | 1 +
packages/block-library/src/gallery/edit.js | 1 +
.../block-library/src/latest-comments/edit.js | 1 +
.../block-library/src/latest-posts/edit.js | 1 +
packages/block-library/src/rss/edit.js | 3 +
.../block-library/src/text-columns/index.js | 1 +
packages/components/CHANGELOG.md | 10 ++
.../components/src/query-controls/index.js | 1 +
.../components/src/range-control/index.js | 14 +--
.../src/range-control/test/index.js | 116 ++++++++++++++++--
11 files changed, 130 insertions(+), 20 deletions(-)
diff --git a/packages/block-library/src/columns/index.js b/packages/block-library/src/columns/index.js
index 7d28794e8a776..eabd06e0e5ae2 100644
--- a/packages/block-library/src/columns/index.js
+++ b/packages/block-library/src/columns/index.js
@@ -172,6 +172,7 @@ export const settings = {
} }
min={ 2 }
max={ 6 }
+ required
/>
</PanelBody>
</InspectorControls>
diff --git a/packages/block-library/src/cover/edit.js b/packages/block-library/src/cover/edit.js
index 398e842d9eafb..bdf85c0874a75 100644
--- a/packages/block-library/src/cover/edit.js
+++ b/packages/block-library/src/cover/edit.js
@@ -214,6 +214,7 @@ class CoverEdit extends Component {
min={ 0 }
max={ 100 }
step={ 10 }
+ required
/>
</PanelColorSettings>
</PanelBody>
diff --git a/packages/block-library/src/gallery/edit.js b/packages/block-library/src/gallery/edit.js
index 868a6a083723d..ade7ba978264b 100644
--- a/packages/block-library/src/gallery/edit.js
+++ b/packages/block-library/src/gallery/edit.js
@@ -250,6 +250,7 @@ class GalleryEdit extends Component {
onChange={ this.setColumnsNumber }
min={ 1 }
max={ Math.min( MAX_COLUMNS, images.length ) }
+ required
/> }
<ToggleControl
label={ __( 'Crop Images' ) }
diff --git a/packages/block-library/src/latest-comments/edit.js b/packages/block-library/src/latest-comments/edit.js
index 1bc21e552939e..b87aecdabd7d1 100644
--- a/packages/block-library/src/latest-comments/edit.js
+++ b/packages/block-library/src/latest-comments/edit.js
@@ -85,6 +85,7 @@ class LatestComments extends Component {
onChange={ this.setCommentsToShow }
min={ MIN_COMMENTS }
max={ MAX_COMMENTS }
+ required
/>
</PanelBody>
</InspectorControls>
diff --git a/packages/block-library/src/latest-posts/edit.js b/packages/block-library/src/latest-posts/edit.js
index 2e0f00e32eea3..d9790466b492e 100644
--- a/packages/block-library/src/latest-posts/edit.js
+++ b/packages/block-library/src/latest-posts/edit.js
@@ -109,6 +109,7 @@ class LatestPostsEdit extends Component {
onChange={ ( value ) => setAttributes( { columns: value } ) }
min={ 2 }
max={ ! hasPosts ? MAX_POSTS_COLUMNS : Math.min( MAX_POSTS_COLUMNS, latestPosts.length ) }
+ required
/>
}
</PanelBody>
diff --git a/packages/block-library/src/rss/edit.js b/packages/block-library/src/rss/edit.js
index e7262e11b2bc1..9b98eaea53781 100644
--- a/packages/block-library/src/rss/edit.js
+++ b/packages/block-library/src/rss/edit.js
@@ -119,6 +119,7 @@ class RSSEdit extends Component {
onChange={ ( value ) => setAttributes( { itemsToShow: value } ) }
min={ DEFAULT_MIN_ITEMS }
max={ DEFAULT_MAX_ITEMS }
+ required
/>
<ToggleControl
label={ __( 'Display author' ) }
@@ -142,6 +143,7 @@ class RSSEdit extends Component {
onChange={ ( value ) => setAttributes( { excerptLength: value } ) }
min={ 10 }
max={ 100 }
+ required
/>
}
{ blockLayout === 'grid' &&
@@ -151,6 +153,7 @@ class RSSEdit extends Component {
onChange={ ( value ) => setAttributes( { columns: value } ) }
min={ 2 }
max={ 6 }
+ required
/>
}
</PanelBody>
diff --git a/packages/block-library/src/text-columns/index.js b/packages/block-library/src/text-columns/index.js
index 5bcf5cc3b6a79..f14b3b8153baa 100644
--- a/packages/block-library/src/text-columns/index.js
+++ b/packages/block-library/src/text-columns/index.js
@@ -114,6 +114,7 @@ export const settings = {
onChange={ ( value ) => setAttributes( { columns: value } ) }
min={ 2 }
max={ 4 }
+ required
/>
</PanelBody>
</InspectorControls>
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
index b596c09199849..8bba28618e50d 100644
--- a/packages/components/CHANGELOG.md
+++ b/packages/components/CHANGELOG.md
@@ -1,3 +1,13 @@
+## 7.2.0 (Unreleased)
+
+### Improvements
+
+- Make `RangeControl` validation rely on the `checkValidity` provided by the browsers instead of using our own validation.
+
+### Bug Fixes
+
+- Fix a problem that made `RangeControl` not work as expected with float values.
+
## 7.1.0 (2019-03-06)
### New Features
diff --git a/packages/components/src/query-controls/index.js b/packages/components/src/query-controls/index.js
index 4ed57e468eb69..f3cbf47998325 100644
--- a/packages/components/src/query-controls/index.js
+++ b/packages/components/src/query-controls/index.js
@@ -79,6 +79,7 @@ export default function QueryControls( {
onChange={ onNumberOfItemsChange }
min={ minItems }
max={ maxItems }
+ required
/>
),
];
diff --git a/packages/components/src/range-control/index.js b/packages/components/src/range-control/index.js
index 1503813def5cb..2fd28d31bcae1 100644
--- a/packages/components/src/range-control/index.js
+++ b/packages/components/src/range-control/index.js
@@ -48,14 +48,9 @@ function RangeControl( {
const onChangeValue = ( event ) => {
const newValue = event.target.value;
- const newNumericValue = parseInt( newValue, 10 );
// If the input value is invalid temporarily save it to the state,
// without calling on change.
- if (
- isNaN( newNumericValue ) ||
- ( min !== undefined && newNumericValue < min ) ||
- ( max !== undefined && newNumericValue > max )
- ) {
+ if ( ! event.target.checkValidity() ) {
setState( {
currentInput: newValue,
} );
@@ -64,9 +59,12 @@ function RangeControl( {
// The input is valid, reset the local state property used to temporaly save the value,
// and call onChange with the new value as a number.
resetCurrentInput();
- onChange( newNumericValue );
+ onChange( ( newValue === '' ) ?
+ undefined :
+ parseFloat( newValue )
+ );
};
- const initialSliderValue = isFinite( value ) ?
+ const initialSliderValue = isFinite( currentInputValue ) ?
currentInputValue :
initialPosition || '';
diff --git a/packages/components/src/range-control/test/index.js b/packages/components/src/range-control/test/index.js
index 5f8e0bccb8aa8..e1308c06363e4 100644
--- a/packages/components/src/range-control/test/index.js
+++ b/packages/components/src/range-control/test/index.js
@@ -42,13 +42,23 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
rangeInputElement(),
{
- target: { value: '5' },
+ target: {
+ value: '5',
+ checkValidity() {
+ return true;
+ },
+ },
}
);
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '10' },
+ target: {
+ value: '10',
+ checkValidity() {
+ return true;
+ },
+ },
}
);
@@ -96,7 +106,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '10' },
+ target: {
+ value: '10',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -116,7 +131,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '21' },
+ target: {
+ value: '21',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -136,14 +156,24 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '10' },
+ target: {
+ value: '10',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '21' },
+ target: {
+ value: '21',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -152,7 +182,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '14' },
+ target: {
+ value: '14',
+ checkValidity() {
+ return true;
+ },
+ },
}
);
@@ -171,7 +206,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '1' },
+ target: {
+ value: '1',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -190,7 +230,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '-101' },
+ target: {
+ value: '-101',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -199,7 +244,12 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '-49' },
+ target: {
+ value: '-49',
+ checkValidity() {
+ return false;
+ },
+ },
}
);
@@ -208,11 +258,53 @@ describe( 'RangeControl', () => {
TestUtils.Simulate.change(
numberInputElement(),
{
- target: { value: '-50' },
+ target: {
+ value: '-50',
+ checkValidity() {
+ return true;
+ },
+ },
}
);
- expect( onChange ).toHaveBeenCalled();
+ expect( onChange ).toHaveBeenCalledWith( -50 );
+ } );
+ it( 'takes into account the step starting from min', () => {
+ const onChange = jest.fn();
+ const wrapper = getWrapper( { onChange, min: 0.1, step: 0.125, value: 0.1 } );
+
+ const numberInputElement = () => TestUtils.findRenderedDOMComponentWithClass(
+ wrapper,
+ 'components-range-control__number'
+ );
+
+ TestUtils.Simulate.change(
+ numberInputElement(),
+ {
+ target: {
+ value: '0.125',
+ checkValidity() {
+ return false;
+ },
+ },
+ }
+ );
+
+ expect( onChange ).not.toHaveBeenCalled();
+
+ TestUtils.Simulate.change(
+ numberInputElement(),
+ {
+ target: {
+ value: '0.225',
+ checkValidity() {
+ return true;
+ },
+ },
+ }
+ );
+
+ expect( onChange ).toHaveBeenCalledWith( 0.225 );
} );
} );
} );