Skip to content

fix(NumberInput): allow user to back out number to type input #8304

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 15, 2022
Merged

fix(NumberInput): allow user to back out number to type input #8304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bd16eb0
fix(NumberInput): allow user to back out number to type input
kmcfaul Oct 28, 2022
b6f8424
fix test
kmcfaul Oct 31, 2022
0af897a
fix plus logic for undefined
kmcfaul Oct 31, 2022
27abeb3
pr feedback
kmcfaul Nov 1, 2022
e217cca
add example desc
kmcfaul Nov 3, 2022
087e0f8
update example desc
kmcfaul Nov 3, 2022
411c75d
update examples for empty input
kmcfaul Nov 3, 2022
afd936a
add integration test, update integration test name
kmcfaul Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/react-core/src/components/NumberInput/NumberInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { TextInput } from '../TextInput';

export interface NumberInputProps extends React.HTMLProps<HTMLDivElement> {
/** Value of the number input */
value?: number | null;
value?: number | '';
Copy link
Contributor

@nicolethoen nicolethoen Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will 'null' still work? Is this considered a breaking change? @tlabaj

Copy link
Contributor Author

@kmcfaul kmcfaul Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing null will still work as long as the user accounts for it in the plus/minus handlers. The only difference is that null previously would have been set to 0 in the component. I'm not sure why it was part of the type declaration.

I can probably add it back in and re-add that check only for a null value if that works out better.

Copy link
Contributor Author

@kmcfaul kmcfaul Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user will have to account for non-number values in the plus/minus handlers to handle the empty string value as well (but this is the new feature). So our current examples still work with null.

Copy link
Contributor

@tlabaj tlabaj Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as null still work it should be fine.

/** Additional classes added to the number input */
className?: string;
/** Sets the width of the number input to a number of characters */
Expand Down Expand Up @@ -89,7 +89,6 @@ export const NumberInput: React.FunctionComponent<NumberInputProps> = ({
plusBtnProps,
...props
}: NumberInputProps) => {
value = value || 0;
const numberInputUnit = <div className={css(styles.numberInputUnit)}>{unit}</div>;
const keyDownHandler =
inputProps && inputProps.onKeyDown ? inputProps.onKeyDown : defaultKeyDownHandler({ inputName, onMinus, onPlus });
Expand Down
8 changes: 4 additions & 4 deletions packages/react-core/src/components/NumberInput/__tests__/NumberInput.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,22 +195,22 @@ describe('numberInput', () => {
expect(input).toHaveDisplayValue('47.01');
});

test('renders 0 if no value passed', () => {
test('renders 0 (default value) if no value passed', () => {
render(<NumberInput />);
const input = screen.getByRole('spinbutton');
expect(input).toHaveDisplayValue('0');
});

test('renders 0 if undefined value passed', () => {
test('renders 0 (default value) if undefined value passed', () => {
render(<NumberInput value={undefined} />);
const input = screen.getByRole('spinbutton');
expect(input).toHaveDisplayValue('0');
});

test('renders 0 if null value passed', () => {
test('renders nothing if null value passed', () => {
render(<NumberInput value={null} />);
const input = screen.getByRole('spinbutton');
expect(input).toHaveDisplayValue('0');
expect(input).toHaveDisplayValue('');
});

test('does not throw an error if onChange is passed via inputProps as well as the onChange prop', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-core/src/components/NumberInput/examples/NumberInput.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ propComponents: ['NumberInput']

### With unit and thresholds

To enable a user entered value to snap to the nearest threshold if the entered input is out of bounds, define the blur event handler.

```ts file="./NumberInputUnitThreshold.tsx"
```

Expand Down
8 changes: 4 additions & 4 deletions packages/react-core/src/components/NumberInput/examples/NumberInputCustomStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputCustomStep: React.FunctionComponent = () => {
const [value, setValue] = React.useState(90);
const [value, setValue] = React.useState<number | ''>(90);
const step = 3;

const stepper = (stepValue: number) => {
setValue(value + stepValue);
setValue((value || 0) + stepValue);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
setValue(Number(target.value));
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
};

return (
Expand Down
9 changes: 4 additions & 5 deletions ...ages/react-core/src/components/NumberInput/examples/NumberInputCustomStepAndThreshold.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputCustomStepAndThreshold: React.FunctionComponent = () => {
const [value, setValue] = React.useState(90);
const [value, setValue] = React.useState<number | ''>(90);
const minValue = 90;
const maxValue = 100;
const step = 3;
Expand All @@ -19,13 +19,12 @@ export const NumberInputCustomStepAndThreshold: React.FunctionComponent = () =>
};

const stepper = (stepValue: number) => {
setValue(value + stepValue);
setValue((value || 0) + stepValue);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
const newValue = normalizeBetween(isNaN(+target.value) ? 0 : Number(target.value), minValue, maxValue);
setValue(newValue);
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
};

const onBlur = (event: React.FormEvent<HTMLInputElement>) => {
Expand Down
10 changes: 5 additions & 5 deletions packages/react-core/src/components/NumberInput/examples/NumberInputDefault.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputDefault: React.FunctionComponent = () => {
const [value, setValue] = React.useState(90);
const [value, setValue] = React.useState<number | ''>(90);

const onMinus = () => {
const newValue = value - 1;
const newValue = (value || 0) - 1;
setValue(newValue);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
setValue(Number(target.value));
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
};

const onPlus = () => {
const newValue = value + 1;
const newValue = (value || 0) + 1;
setValue(newValue);
};

Expand Down
10 changes: 5 additions & 5 deletions packages/react-core/src/components/NumberInput/examples/NumberInputDisabled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputDisabled: React.FunctionComponent = () => {
const [value, setValue] = React.useState(100);
const [value, setValue] = React.useState<number | ''>(100);
const minValue = 0;
const maxValue = 100;

const onMinus = () => {
const newValue = value - 1;
const newValue = (value || 0) - 1;
setValue(newValue);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
setValue(Number(target.value));
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
};

const onPlus = () => {
const newValue = value + 1;
const newValue = (value || 0) + 1;
setValue(newValue);
};

Expand Down
20 changes: 10 additions & 10 deletions packages/react-core/src/components/NumberInput/examples/NumberInputUnit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputUnit: React.FunctionComponent = () => {
const [value1, setValue1] = React.useState(90);
const [value2, setValue2] = React.useState(Number((1.0).toFixed(2)));
const [value1, setValue1] = React.useState<number | ''>(90);
const [value2, setValue2] = React.useState<number | ''>(Number((1.0).toFixed(2)));

const onMinus1 = () => setValue1(value1 - 1);
const onMinus1 = () => setValue1((value1 || 0) - 1);
const onChange1 = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
setValue1(Number(target.value));
const value = (event.target as HTMLInputElement).value;
setValue1(value === '' ? value : +value);
};
const onPlus1 = () => setValue1(value1 + 1);
const onPlus1 = () => setValue1((value1 || 0) + 1);

const onMinus2 = () => {
const newValue = Number((value2 - 0.01).toFixed(2));
const newValue = Number(((value2 || 0) - 0.01).toFixed(2));
setValue2(newValue);
};
const onChange2 = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
setValue2(Number(target.value));
const value = (event.target as HTMLInputElement).value;
setValue2(value === '' ? value : +value);
};
const onPlus2 = () => {
const newValue = Number((value2 + 0.01).toFixed(2));
const newValue = Number(((value2 || 0) + 0.01).toFixed(2));
setValue2(newValue);
};

Expand Down
22 changes: 16 additions & 6 deletions packages/react-core/src/components/NumberInput/examples/NumberInputUnitThreshold.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputUnitThreshold: React.FunctionComponent = () => {
const [value, setValue] = React.useState(0);
const [value, setValue] = React.useState<number | ''>(0);
const minValue = 0;
const maxValue = 10;

Expand All @@ -18,18 +18,27 @@ export const NumberInputUnitThreshold: React.FunctionComponent = () => {
};

const onMinus = () => {
const newValue = normalizeBetween(value - 1, minValue, maxValue);
const newValue = normalizeBetween((value as number) - 1, minValue, maxValue);
setValue(newValue);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
const newValue = normalizeBetween(isNaN(+target.value) ? 0 : Number(target.value), minValue, maxValue);
setValue(newValue);
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
};

const onBlur = (event: React.FocusEvent<HTMLInputElement>) => {
const blurVal = +event.target.value;

if (blurVal < minValue) {
setValue(minValue);
} else if (blurVal > maxValue) {
setValue(maxValue);
}
};

const onPlus = () => {
const newValue = normalizeBetween(value + 1, minValue, maxValue);
const newValue = normalizeBetween((value as number) + 1, minValue, maxValue);
setValue(newValue);
};

Expand All @@ -43,6 +52,7 @@ export const NumberInputUnitThreshold: React.FunctionComponent = () => {
max={maxValue}
onMinus={onMinus}
onChange={onChange}
onBlur={onBlur}
onPlus={onPlus}
inputName="input"
inputAriaLabel="number input"
Expand Down
22 changes: 11 additions & 11 deletions packages/react-core/src/components/NumberInput/examples/NumberInputVaryingSizes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,25 @@ import React from 'react';
import { NumberInput } from '@patternfly/react-core';

export const NumberInputVaryingSizes: React.FunctionComponent = () => {
const [input1Value, setInput1Value] = React.useState(1);
const [input2Value, setInput2Value] = React.useState(1234567890);
const [input3Value, setInput3Value] = React.useState(5);
const [input4Value, setInput4Value] = React.useState(12345);
const [input1Value, setInput1Value] = React.useState<number | ''>(1);
const [input2Value, setInput2Value] = React.useState<number | ''>(1234567890);
const [input3Value, setInput3Value] = React.useState<number | ''>(5);
const [input4Value, setInput4Value] = React.useState<number | ''>(12345);

const onChange = (
event: React.FormEvent<HTMLInputElement>,
updateFunction: React.Dispatch<React.SetStateAction<number>>
updateFunction: React.Dispatch<React.SetStateAction<number | ''>>
) => {
const target = event.target as HTMLInputElement;
updateFunction(Number(target.value));
const value = (event.target as HTMLInputElement).value;
updateFunction(value === '' ? value : +value);
};

const onMinus = (value, updateFunction: React.Dispatch<React.SetStateAction<number>>) => {
updateFunction(value - 1);
const onMinus = (value: number | '', updateFunction: React.Dispatch<React.SetStateAction<number | ''>>) => {
updateFunction((value || 0) - 1);
};

const onPlus = (value, updateFunction: React.Dispatch<React.SetStateAction<number>>) => {
updateFunction(value + 1);
const onPlus = (value: number | '', updateFunction: React.Dispatch<React.SetStateAction<number | ''>>) => {
updateFunction((value || 0) + 1);
};

return (
Expand Down
11 changes: 5 additions & 6 deletions packages/react-core/src/components/NumberInput/examples/NumberInputWithStatus.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,21 @@ export const NumberInputWithStatus: React.FunctionComponent = () => {
const [value, setValue] = React.useReducer((state, newVal) => Math.max(min, Math.min(max, Number(newVal))), 5);

const onPlus = () => {
const newVal = value + 1;
const newVal = (value || 0) + 1;
setValue(newVal);
validate(newVal);
};

const onMinus = () => {
const newVal = value - 1;
const newVal = (value || 0) - 1;
setValue(newVal);
validate(newVal);
};

const onChange = (event: React.FormEvent<HTMLInputElement>) => {
const target = event.target as HTMLInputElement;
const newVal = isNaN(+target.value) ? 5 : Number(target.value);
setValue(newVal);
validate(newVal);
const value = (event.target as HTMLInputElement).value;
setValue(value === '' ? value : +value);
validate(value);
};

const validate = newVal => {
Expand Down
9 changes: 8 additions & 1 deletion packages/react-integration/cypress/integration/numberInput.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,16 @@ describe('NumberInput Demo Test', () => {
.should('have.class', 'pf-c-input-group');
});

it('initial null value can be increment with the plus button', () => {
it('initial undefined value can be increment with the plus button', () => {
cy.get('#input3').should('have.value', 0);
cy.get('#plus-button3').click();
cy.get('#input3').should('have.value', 1);
});

it('out of bounds input value can be typed and snap to nearest threshold', () => {
cy.get('#input3').should('have.value', 1);
cy.get('#input3').type('5');
cy.get('body').click();
cy.get('#input3').should('have.value', 3);
});
});
15 changes: 13 additions & 2 deletions ...es/react-integration/demo-app-ts/src/components/demos/NumberInputDemo/NumberInputDemo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class NumberInputDemo extends Component<NumberInputDemoState> {

state: NumberInputDemoState = {
value: 0,
value2: null
value2: undefined
};

onMinus = () => {
Expand Down Expand Up @@ -49,10 +49,20 @@ export class NumberInputDemo extends Component<NumberInputDemoState> {

onPlus2 = () => {
this.setState({
value2: this.state.value2 + 1
value2: (this.state.value2 || 0) + 1
});
};

onBlur = () => {
if (this.state.value2 > 3) {
this.setState({
value2: 3
});
} else if (this.state.value2 < 0) {
this.setState({ value2: 0 });
}
};

render() {
const { value, value2 } = this.state;
const minValue = 0;
Expand Down Expand Up @@ -109,6 +119,7 @@ export class NumberInputDemo extends Component<NumberInputDemoState> {
onMinus={this.onMinus2}
onChange={this.onChange2}
onPlus={this.onPlus2}
onBlur={this.onBlur}
inputName="input 3"
inputAriaLabel="number input 3"
minusBtnAriaLabel="minus"
Expand Down