Skip to content

Commit

Permalink
fix(NumberInput): allow value to be outside min/max, show validation (#…
Browse files Browse the repository at this point in the history
…9121)

* fix(NumberInput): allow value to be outside min/max, show validation

* test(NumberInput): update tests

Co-authored-by: Scott Strubberg <sstrubberg@protonmail.com>
  • Loading branch information
tw15egan and sstrubberg committed Jul 7, 2021
1 parent 0e2b53b commit ef38162
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
16 changes: 9 additions & 7 deletions packages/react/src/components/NumberInput/NumberInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ describe('NumberInput', () => {
id="test"
label="Number Input"
className="extra-class"
invalidText="Number is not valid"
/>
);
const getNumberInput = (wrapper) => wrapper.find('input');
Expand Down Expand Up @@ -185,10 +186,13 @@ describe('NumberInput', () => {
expect(numberInput.prop('value')).toEqual(1);
});

it('should set value to equal min when value < min', () => {
it('should set value when value < min and set invalid state', () => {
let wrapper = getWrapper(5, 100, 0);
let numberInput = wrapper.find('input');
expect(numberInput.prop('value')).toEqual(5);
let invalidText = wrapper.find(`.${prefix}--form-requirement`);
expect(numberInput.prop('value')).toEqual(0);
expect(invalidText.length).toEqual(1);
expect(invalidText.text()).toEqual('Number is not valid');
});

it('should set value when min is undefined', () => {
Expand Down Expand Up @@ -238,19 +242,17 @@ describe('NumberInput', () => {
expect(wrapper.find('NumberInput').instance().state.value).toEqual(2);
});

it('should cap the number given to value prop', () => {
it('should not cap the number given to value prop', () => {
// Enzyme doesn't seem to allow setState() in a forwardRef-wrapped class component
wrapper.find('NumberInput').instance().setState({ value: 0 });
wrapper.update();
wrapper.setProps({ value: 5, min: 10, max: 20 });
// Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
expect(wrapper.find('NumberInput').instance().state.value).toEqual(
10
);
expect(wrapper.find('NumberInput').instance().state.value).toEqual(5);
wrapper.setProps({ value: 25, min: 10, max: 20 });
// Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
expect(wrapper.find('NumberInput').instance().state.value).toEqual(
20
25
);
});

Expand Down
6 changes: 4 additions & 2 deletions packages/react/src/components/NumberInput/NumberInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class NumberInput extends Component {
translateWithId: (id) => defaultTranslations[id],
};

static getDerivedStateFromProps({ min, max, value }, state) {
static getDerivedStateFromProps({ value }, state) {
const { prevValue } = state;

if (useControlledStateWithValue && value === '' && prevValue !== '') {
Expand All @@ -214,10 +214,12 @@ class NumberInput extends Component {

// If `useControlledStateWithValue` feature flag is on, do nothing here.
// Otherwise, do prop -> state sync with "value capping".
//// Value capping removed in #8965
//// value: capMax(max, capMin(min, value)), (L223)
return useControlledStateWithValue || prevValue === value
? null
: {
value: capMax(max, capMin(min, value)),
value,
prevValue: value,
};
}
Expand Down

0 comments on commit ef38162

Please sign in to comment.