From 43085b327503fb1bd059de1ba5138643223fd25b Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 20 Jan 2019 08:26:44 -0700 Subject: [PATCH 1/4] EuiRange - onValidatedChange props --- src-docs/src/views/form_controls/range.js | 39 ++++ .../range/__snapshots__/range.test.js.snap | 13 +- src/components/form/range/range.js | 172 ++++++++++++++---- src/components/form/range/range.test.js | 12 +- 4 files changed, 191 insertions(+), 45 deletions(-) diff --git a/src-docs/src/views/form_controls/range.js b/src-docs/src/views/form_controls/range.js index d071610942a..283351dfd97 100644 --- a/src-docs/src/views/form_controls/range.js +++ b/src-docs/src/views/form_controls/range.js @@ -7,6 +7,7 @@ import { EuiRange, EuiSpacer, EuiFormHelpText, + EuiCode, } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -30,15 +31,24 @@ export default class extends Component { this.state = { value: '120', + validatedValue: '3000', }; } onChange = e => { + // onChange called with Event this.setState({ value: e.target.value, }); }; + onValidatedChange = newValue => { + // onValidatedChange called with Number (not Event like onChange) + this.setState({ + validatedValue: newValue + }); + }; + render() { return ( @@ -131,6 +141,35 @@ export default class extends Component { tickInterval={500} levels={this.levels} /> + + + +
+

+ onChange is called with the DOM Event and requires you to extract the value + and validate that the value is a Number within the specified range. +

+

+ Use onValidatedChange callback to only receive updates when the value + changes to a Number that is within the specified range and leave all that error checking + to EuiRange. +

+
+ + + ); } diff --git a/src/components/form/range/__snapshots__/range.test.js.snap b/src/components/form/range/__snapshots__/range.test.js.snap index de45d8e5ab5..f11c09b04b0 100644 --- a/src/components/form/range/__snapshots__/range.test.js.snap +++ b/src/components/form/range/__snapshots__/range.test.js.snap @@ -194,13 +194,14 @@ exports[`EuiRange props range should render 1`] = ` max="100" min="1" type="range" + value="50" />
@@ -219,6 +220,7 @@ exports[`EuiRange props ticks should render 1`] = ` max="100" min="1" type="range" + value="50" />
+ class="euiRange__value euiRange__value--right" + style="left:49.494949494949495%" + > + 50 +
diff --git a/src/components/form/range/range.js b/src/components/form/range/range.js index 46dd286c67f..13d4d022f63 100644 --- a/src/components/form/range/range.js +++ b/src/components/form/range/range.js @@ -1,16 +1,31 @@ -import React, { Component } from 'react'; +import React, { Component, Fragment } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { range, find } from 'lodash'; import { EuiFieldNumber } from '../field_number'; +import { EuiFormErrorText } from '../form_error_text'; export const LEVEL_COLORS = ['primary', 'success', 'warning', 'danger']; export class EuiRange extends Component { constructor(props) { super(props); + + this.state = {}; + } + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.value !== prevState.prevValue) { + return { + value: nextProps.value, + prevValue: nextProps.value, + isValid: isWithinRange(nextProps.min, nextProps.max, nextProps.value), + }; + } + + return null; } render() { @@ -33,8 +48,9 @@ export class EuiRange extends Component { showRange, showValue, valueAppend, // eslint-disable-line no-unused-vars - onChange, - value, + onChange, // eslint-disable-line no-unused-vars + onValidatedChange, // eslint-disable-line no-unused-vars + value, // eslint-disable-line no-unused-vars style, ...rest } = this.props; @@ -80,10 +96,10 @@ export class EuiRange extends Component { min={min} max={max} step={step} - value={Number(value)} + value={this.state.value === '' ? '' : Number(this.state.value)} disabled={disabled} compressed={compressed} - onChange={onChange} + onChange={this.onRangeChange} style={maxWidthStyle} {...rest} /> @@ -109,38 +125,68 @@ export class EuiRange extends Component { } return ( -
- {this.renderLabel('min')} - -
- - - {this.renderValue()} - {this.renderRange()} - {this.renderLevels()} - {this.renderTicks(tickObject)} + +
+ {this.renderLabel('min')} + +
+ + + {this.renderValue()} + {this.renderRange()} + {this.renderLevels()} + {this.renderTicks(tickObject)} +
+ + {this.renderLabel('max')} + {extraInputNode}
- - {this.renderLabel('max')} - {extraInputNode} -
+ {this.renderErrorMessage()} + ); } + onRangeChange = e => { + const { + max, + min, + onChange, + onValidatedChange + } = this.props; + if (!onValidatedChange) { + onChange(e); + return; + } + + const sanitizedValue = parseInt(e.target.value, 10); + const newValue = isNaN(sanitizedValue) ? '' : sanitizedValue; + + const isValid = isWithinRange(min, max, newValue); + + this.setState({ + value: newValue, + isValid, + }); + + if (isValid) { + onValidatedChange(newValue); + } + } + renderLabel = (side) => { const { showLabels, @@ -159,17 +205,21 @@ export class EuiRange extends Component { renderTicks = (tickObject) => { const { disabled, - onChange, showTicks, ticks, - value, max, } = this.props; + const { value } = this.state; + if (!showTicks) { return; } + if (isNaN(value) || value === '') { + return; + } + // Align with item labels across the range by adding // left and right negative margins that is half of the tick marks const ticksStyle = !!ticks ? undefined : { margin: `0 ${tickObject.percentageWidth / -2}%`, left: 0, right: 0 }; @@ -206,7 +256,7 @@ export class EuiRange extends Component { key={tickValue} value={tickValue} disabled={disabled} - onClick={onChange} + onClick={this.onRangeChange} style={tickStyle} // Don't allow tabbing and just let the range to do the work for non-sighted users tabIndex="-1" @@ -222,15 +272,20 @@ export class EuiRange extends Component { renderRange = () => { const { showRange, - value, max, min, } = this.props; + const { value } = this.state; + if (!showRange) { return; } + if (isNaN(value) || value === '' || value < min) { + return; + } + // Calculate the width the range based on value const rangeWidth = (value - min) / (max - min); const rangeWidthStyle = { width: `${rangeWidth * 100}%` }; @@ -245,17 +300,22 @@ export class EuiRange extends Component { renderValue = () => { const { showValue, - value, valueAppend, max, min, name, } = this.props; + const { value } = this.state; + if (!showValue) { return; } + if (isNaN(value) || value === '') { + return; + } + // Calculate the left position based on value const decimal = (value - min) / (max - min); // Must be between 0-100% @@ -310,6 +370,18 @@ export class EuiRange extends Component {
); } + + renderErrorMessage = () => { + if (!this.props.onValidatedChange || this.state.isValid) { + return null; + } + + return ( + + {`Must be between ${this.props.min} and ${this.props.max}`} + + ); + } } function calculateTicksObject(min, max, interval) { @@ -331,6 +403,18 @@ function calculateTicksObject(min, max, interval) { ); } +function isWithinRange(min, max, value) { + if (isNaN(value) || value === '') { + return false; + } + + if (value >= min && value <= max) { + return true; + } + + return false; +} + EuiRange.propTypes = { name: PropTypes.string, id: PropTypes.string, @@ -365,7 +449,19 @@ EuiRange.propTypes = { label: PropTypes.node.isRequired, }), ), + /* + * Callback function used when "onValidatedChange" props is not provided. + * Function called when value changes through all interactions with range input or number input. + * Called with (Event). + */ onChange: PropTypes.func, + /* + * Callback function used when value changes through interaction with range input or number input. + * Function is only called when value changes to a Number that + * is within the provided min and max (inclusive). + * Called with (Number). + */ + onValidatedChange: PropTypes.func, /** * Create colored indicators for certain intervals */ diff --git a/src/components/form/range/range.test.js b/src/components/form/range/range.test.js index 3ee3786d07a..0b238f24d6d 100644 --- a/src/components/form/range/range.test.js +++ b/src/components/form/range/range.test.js @@ -52,7 +52,7 @@ describe('EuiRange', () => { test('ticks should render', () => { const component = render( - + ); expect(component) @@ -61,7 +61,10 @@ describe('EuiRange', () => { test('range should render', () => { const component = render( - + ); expect(component) @@ -70,7 +73,10 @@ describe('EuiRange', () => { test('value should render', () => { const component = render( - + ); expect(component) From 5b688287b1160cc100f3d920c99ef75f02034acd Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 20 Jan 2019 08:33:10 -0700 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e9afeeb4fa..f5737568a96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## [`master`](https://github.com/elastic/eui/tree/master) +- Added `onValidatedChange` prop to `EuiRange` ([#1461](https://github.com/elastic/eui/pull/1461)) - Added `inputRef` prop to `EuiFieldNumber` and updated `EuiFieldText`'s to a Ref type ([#1434](https://github.com/elastic/eui/pull/1434)) - Added `snowflake` icon ([#1445](https://github.com/elastic/eui/pull/1445)) From 3462c8e6884746e52eb6c1b94a93e4ce96a873e9 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 21 Jan 2019 07:42:06 -0700 Subject: [PATCH 3/4] support decimals --- src/components/form/range/range.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/form/range/range.js b/src/components/form/range/range.js index 13d4d022f63..600f3d3ea97 100644 --- a/src/components/form/range/range.js +++ b/src/components/form/range/range.js @@ -172,7 +172,7 @@ export class EuiRange extends Component { return; } - const sanitizedValue = parseInt(e.target.value, 10); + const sanitizedValue = parseFloat(e.target.value, 10); const newValue = isNaN(sanitizedValue) ? '' : sanitizedValue; const isValid = isWithinRange(min, max, newValue); From ed66390e65c5834e1425ddddaccaab6780668fa8 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Wed, 23 Jan 2019 05:43:53 -0700 Subject: [PATCH 4/4] review feedback --- src-docs/src/views/form_controls/range.js | 49 +++++++--- .../range/__snapshots__/range.test.js.snap | 2 +- src/components/form/range/range.js | 92 ++++++++----------- 3 files changed, 75 insertions(+), 68 deletions(-) diff --git a/src-docs/src/views/form_controls/range.js b/src-docs/src/views/form_controls/range.js index 283351dfd97..3121387c4d6 100644 --- a/src-docs/src/views/form_controls/range.js +++ b/src-docs/src/views/form_controls/range.js @@ -8,6 +8,8 @@ import { EuiSpacer, EuiFormHelpText, EuiCode, + EuiText, + EuiFormRow, } from '../../../../src/components'; import makeId from '../../../../src/components/form/form_row/make_id'; @@ -32,6 +34,7 @@ export default class extends Component { this.state = { value: '120', validatedValue: '3000', + validationErrorMsg: null, }; } @@ -49,6 +52,12 @@ export default class extends Component { }); }; + onValidationError = message => { + this.setState({ + validationErrorMsg: message + }); + } + render() { return ( @@ -144,7 +153,8 @@ export default class extends Component { -
+ +

Validation

onChange is called with the DOM Event and requires you to extract the value and validate that the value is a Number within the specified range. @@ -154,21 +164,30 @@ export default class extends Component { changes to a Number that is within the specified range and leave all that error checking to EuiRange.

-
+

+ Provide onValidationError to receive validation error messages. +

+ - + + + ); diff --git a/src/components/form/range/__snapshots__/range.test.js.snap b/src/components/form/range/__snapshots__/range.test.js.snap index f11c09b04b0..74c51d4cf30 100644 --- a/src/components/form/range/__snapshots__/range.test.js.snap +++ b/src/components/form/range/__snapshots__/range.test.js.snap @@ -100,7 +100,7 @@ exports[`EuiRange props extra input should render 1`] = ` max="10" min="1" name="name" - style="max-width:4em" + style="max-width:5em" type="number" value="8" /> diff --git a/src/components/form/range/range.js b/src/components/form/range/range.js index 600f3d3ea97..f386ad9a070 100644 --- a/src/components/form/range/range.js +++ b/src/components/form/range/range.js @@ -1,11 +1,10 @@ -import React, { Component, Fragment } from 'react'; +import React, { Component } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { range, find } from 'lodash'; import { EuiFieldNumber } from '../field_number'; -import { EuiFormErrorText } from '../form_error_text'; export const LEVEL_COLORS = ['primary', 'success', 'warning', 'danger']; @@ -50,6 +49,7 @@ export class EuiRange extends Component { valueAppend, // eslint-disable-line no-unused-vars onChange, // eslint-disable-line no-unused-vars onValidatedChange, // eslint-disable-line no-unused-vars + onValidationError, // eslint-disable-line no-unused-vars value, // eslint-disable-line no-unused-vars style, ...rest @@ -84,7 +84,7 @@ export class EuiRange extends Component { // Chrome will properly size the input based on the max value, but FF & IE does not. // Calculate the max-width of the input based on number of characters in min or max unit, whichever is greater. // Add 2 to accomodate for input stepper - const maxWidthStyle = { maxWidth: `${Math.max(String(min).length, String(max).length) + 2}em` }; + const maxWidthStyle = { maxWidth: `${Math.max(String(min + step).length, String(max + step).length) + 2}em` }; // Make this input the main control by disabling screen reader access to slider control sliderTabIndex = '-1'; @@ -125,38 +125,35 @@ export class EuiRange extends Component { } return ( - -
- {this.renderLabel('min')} - -
- - - {this.renderValue()} - {this.renderRange()} - {this.renderLevels()} - {this.renderTicks(tickObject)} -
- - {this.renderLabel('max')} - {extraInputNode} +
+ {this.renderLabel('min')} + +
+ + + {this.renderValue()} + {this.renderRange()} + {this.renderLevels()} + {this.renderTicks(tickObject)}
- {this.renderErrorMessage()} - + + {this.renderLabel('max')} + {extraInputNode} +
); } @@ -165,7 +162,8 @@ export class EuiRange extends Component { max, min, onChange, - onValidatedChange + onValidatedChange, + onValidationError, } = this.props; if (!onValidatedChange) { onChange(e); @@ -185,6 +183,11 @@ export class EuiRange extends Component { if (isValid) { onValidatedChange(newValue); } + + if (onValidationError) { + const message = isValid ? null : `Must be between ${min} and ${max}`; + onValidationError(message); + } } renderLabel = (side) => { @@ -216,10 +219,6 @@ export class EuiRange extends Component { return; } - if (isNaN(value) || value === '') { - return; - } - // Align with item labels across the range by adding // left and right negative margins that is half of the tick marks const ticksStyle = !!ticks ? undefined : { margin: `0 ${tickObject.percentageWidth / -2}%`, left: 0, right: 0 }; @@ -312,7 +311,7 @@ export class EuiRange extends Component { return; } - if (isNaN(value) || value === '') { + if (isNaN(value) || value === '' || value < min || value > max) { return; } @@ -370,18 +369,6 @@ export class EuiRange extends Component {
); } - - renderErrorMessage = () => { - if (!this.props.onValidatedChange || this.state.isValid) { - return null; - } - - return ( - - {`Must be between ${this.props.min} and ${this.props.max}`} - - ); - } } function calculateTicksObject(min, max, interval) { @@ -462,6 +449,7 @@ EuiRange.propTypes = { * Called with (Number). */ onValidatedChange: PropTypes.func, + onValidationError: PropTypes.func, /** * Create colored indicators for certain intervals */