Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Slider): check validity of number input after onChange #9132

Merged
merged 10 commits into from
Sep 13, 2021
4 changes: 4 additions & 0 deletions packages/components/src/components/slider/_slider.scss
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@
}
}

.#{$prefix}--slider-text-input.#{$prefix}--text-input--invalid {
padding-right: 1rem;
}

.#{$prefix}--slider__thumb:focus ~ .#{$prefix}--slider__filled-track {
background-color: $interactive-04;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Slider/Slider-story.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const Default = () => (
<Slider
labelText="Slider Label"
value={50}
min={0}
min={30}
jnm2377 marked this conversation as resolved.
Show resolved Hide resolved
max={100}
step={1}
stepMultiplier={10}
Expand Down
77 changes: 72 additions & 5 deletions packages/react/src/components/Slider/Slider-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ describe('Slider', () => {
describe('key/mouse event processing', () => {
const handleChange = jest.fn();
const handleRelease = jest.fn();
const handleBlur = jest.fn();

const wrapper = mount(
<Slider
id="slider"
Expand All @@ -139,6 +141,7 @@ describe('Slider', () => {
stepMultiplier={5}
onChange={handleChange}
onRelease={handleRelease}
onBlur={handleBlur}
/>
);

Expand Down Expand Up @@ -224,24 +227,36 @@ describe('Slider', () => {
});
});

it('does not set incorrect state when typing a invalid value in input field', () => {
it('allows user to set invalid value when typing in input field', () => {
const evt = {
target: {
value: '999',
checkValidity: () => false,
},
};

wrapper.instance().onChange(evt);
expect(wrapper.state().value).toEqual(100);
expect(handleChange).toHaveBeenLastCalledWith({ value: 100 });
expect(wrapper.state().value).toEqual(999);
expect(handleChange).toHaveBeenLastCalledWith({ value: 999 });
});

it('checks for validity onBlur', () => {
const checkValidity = jest.fn();

const evt = {
target: {
value: '',
checkValidity: checkValidity,
},
};

wrapper.instance().onBlur(evt);
expect(checkValidity).toHaveBeenCalled();
});

it('sets correct state when typing a valid value in input field', () => {
const evt = {
target: {
value: '12',
checkValidity: () => true,
},
};

Expand All @@ -251,6 +266,51 @@ describe('Slider', () => {
});
});

describe('syncs invalid state and props', () => {
const handleChange = jest.fn();
const handleRelease = jest.fn();
const handleBlur = jest.fn();

const wrapper = mount(
<Slider
id="slider"
className="extra-class"
value={50}
min={0}
max={100}
step={1}
onChange={handleChange}
onRelease={handleRelease}
onBlur={handleBlur}
invalid={false}
/>
);

it('overrides invalid state for invalid prop', () => {
const changeEvt = {
target: {
value: '200',
},
};

const blurEvt = {
target: {
checkValidity: () => false,
},
};

wrapper.instance().onChange(changeEvt);
wrapper.instance().onBlur(blurEvt);
expect(wrapper.state().value).toEqual(200);
expect(wrapper.state().isValid).toEqual(true);
expect(
wrapper
.find(`.${prefix}--slider-text-input`)
.hasClass(`${prefix}--text-input--invalid`)
).toEqual(false);
});
});

describe('error handling', () => {
const handleChange = jest.fn();
const handleRelease = jest.fn();
Expand Down Expand Up @@ -292,6 +352,13 @@ describe('Slider', () => {
expect(handleChange).not.toHaveBeenCalled();
});

it('gracefully tolerates empty event passed to onBlur', () => {
const evt = {};
wrapper.instance().onBlur(evt);
expect(wrapper.state().value).toEqual(''); // from last test
expect(handleChange).not.toHaveBeenCalled();
});

it('gracefully tolerates empty event passed to onKeyDown', () => {
const evt = {};
wrapper.instance().onKeyDown(evt);
Expand Down
71 changes: 55 additions & 16 deletions packages/react/src/components/Slider/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export default class Slider extends PureComponent {
value: this.props.value,
left: 0,
needsOnRelease: false,
isValid: true,
};

/**
Expand Down Expand Up @@ -302,7 +303,7 @@ export default class Slider extends PureComponent {
});

// Set needsOnRelease flag so event fires on next update
this.setState({ needsOnRelease: true });
this.setState({ needsOnRelease: true, isValid: true });
};

/**
Expand Down Expand Up @@ -332,7 +333,7 @@ export default class Slider extends PureComponent {
}

const { value, left } = this.calcValue({ clientX });
this.setState({ value, left });
this.setState({ value, left, isValid: true });
};

/**
Expand Down Expand Up @@ -383,7 +384,8 @@ export default class Slider extends PureComponent {
? Math.floor(this.state.value / this.props.step) * this.props.step
: this.state.value) + delta,
});
this.setState({ value, left });

this.setState({ value, left, isValid: true });
};

/**
Expand All @@ -393,6 +395,7 @@ export default class Slider extends PureComponent {
*
* @param {Event} evt The event.
*/

onChange = (evt) => {
// Do nothing if component is disabled
if (this.props.disabled) {
Expand All @@ -410,18 +413,34 @@ export default class Slider extends PureComponent {
if (isNaN(targetValue)) {
this.setState({ value: evt.target.value });
} else {
// Recalculate the state's value and update the Slider
// if it is a valid number
if (evt.target.checkValidity() === false) {
return;
}

const { value, left } = this.calcValue({
value: targetValue,
useRawValue: true,
});
this.setState({ value, left, needsOnRelease: true });
this.setState({
value,
left,
needsOnRelease: true,
});
}
};

/**
* Checks for validity of input value after clicking out of the input. It also
* Handles state change to isValid state.
*
* @param {Event} evt The event.
*/
onBlur = (evt) => {
// Do nothing if we have no valid event, target, or value
if (!evt || !('target' in evt) || typeof evt.target.value !== 'string') {
return;
}
// determine validity of input change after clicking out of input
const validity = evt.target.checkValidity();
this.setState({
isValid: validity,
});
};

/**
Expand Down Expand Up @@ -488,6 +507,25 @@ export default class Slider extends PureComponent {
return { value: steppedValue, left: steppedPercent * 100 };
};

// syncs invalid state and prop
static getDerivedStateFromProps(props, state) {
const { isValid } = state;
// will override state in favor of invalid prop
if (props.invalid === true && isValid === true) {
return {
isValid: false,
};
}

if (props.invalid === false && isValid === false) {
return {
isValid: true,
};
}
//if invalid prop is not provided, state will remain the same
return null;
}

render() {
const {
ariaLabelInput,
Expand All @@ -510,13 +548,13 @@ export default class Slider extends PureComponent {
disabled,
name,
light,
invalid,
...other
} = this.props;

delete other.onRelease;
delete other.invalid;

const { value, left } = this.state;
const { value, left, isValid } = this.state;

const scope = this.context;
let enabled;
Expand All @@ -541,7 +579,7 @@ export default class Slider extends PureComponent {
`${prefix}--slider-text-input`,
{
[`${prefix}--text-input--light`]: light,
[`${prefix}--text-input--invalid`]: this.props.invalid,
[`${prefix}--text-input--invalid`]: isValid === false,
}
);

Expand Down Expand Up @@ -579,7 +617,7 @@ export default class Slider extends PureComponent {
onKeyDown={this.onKeyDown}
role="presentation"
tabIndex={-1}
data-invalid={invalid || null}
data-invalid={isValid ? null : true}
{...other}>
<div
className={`${prefix}--slider__thumb`}
Expand Down Expand Up @@ -620,8 +658,9 @@ export default class Slider extends PureComponent {
max={max}
step={step}
onChange={this.onChange}
data-invalid={invalid || null}
aria-invalid={invalid || null}
onBlur={this.onBlur}
data-invalid={isValid ? null : true}
aria-invalid={isValid ? null : true}
/>
</div>
</div>
Expand Down
4 changes: 4 additions & 0 deletions packages/styles/scss/components/slider/_slider.scss
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@
}
}

.#{$prefix}--slider-text-input.#{$prefix}--text-input--invalid {
padding-right: 1rem;
}

.#{$prefix}--slider__thumb:focus ~ .#{$prefix}--slider__filled-track {
background-color: $interactive;
}
Expand Down