From 6a9665a157fea63554c3a63fc7c10968703ccc0d Mon Sep 17 00:00:00 2001 From: Jorge Costa 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 /> 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 /> 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 /> } 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 /> } 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 /> 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 /> } 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 /> 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 ); } ); } ); } );