Skip to content

Commit 2b0d109

Browse files
authored
fix(APP-3440): Fix NumberInput component to correctly update value on buttons click (#262)
1 parent ed2b841 commit 2b0d109

File tree

9 files changed

+86
-95
lines changed

9 files changed

+86
-95
lines changed

.github/dependabot.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ updates:
44
directory: "/"
55
schedule:
66
interval: "weekly"
7+
ignore:
8+
# From version 7.6.x, react-imask doesn't correctly trigger the `onAccept` callback when changing the value
9+
# programmatically (see https://github.com/uNmAnNeR/imaskjs/pull/1045)
10+
- dependency-name: "react-imask"
711
groups:
812
minor-and-patch:
913
update-types:

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1515
- Fix ENS name truncation on `<Wallet />` module component
1616
- Update `<Wallet />` module component to only resolve user ENS name when name property is not set
1717
- Fix expand behaviour of `TextAreaRichText` core component when used inside a dialog and hide the input label
18+
- Fix `NumberInput` component to correctly update values on plus / minus buttons click
1819

1920
### Added
2021

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
"framer-motion": "^11.3.0",
6969
"luxon": "^3.5.0",
7070
"react-dropzone": "^14.2.0",
71-
"react-imask": "^7.6.0",
71+
"react-imask": "7.5.0",
7272
"sanitize-html": "^2.13.0",
7373
"tiptap-markdown": "^0.8.10"
7474
},

src/core/components/forms/hooks/useNumberMask.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,16 @@ const getNumberSeparators = () => {
3333
};
3434

3535
// The imask.js library requires us to set a "scale" property as max decimal places otherwise it defaults to 0.
36-
const maxDecimalPlaces = 30;
36+
const defaultScale = 30;
3737

3838
export const useNumberMask = (props: IUseNumberMaskProps): IUseNumberMaskResult => {
3939
const { suffix, prefix, min, max, onChange, value } = props;
4040

4141
const { thousandsSeparator, radix } = getNumberSeparators();
4242

4343
const numberMask = `${prefix ?? ''} num ${suffix ?? ''}`.trim();
44+
const maskMax = max != null ? Number(max) : undefined;
45+
const maskMin = min != null ? Number(min) : undefined;
4446

4547
const handleMaskAccept = (_value: string, mask: InputMask<FactoryOpts>) => {
4648
// Update the lazy option to display the suffix when the user is deleting the last digits of the input
@@ -54,14 +56,7 @@ export const useNumberMask = (props: IUseNumberMaskProps): IUseNumberMaskResult
5456
mask: numberMask,
5557
eager: true, // Displays eventual suffix on user input
5658
blocks: {
57-
num: {
58-
mask: Number,
59-
radix,
60-
thousandsSeparator,
61-
scale: maxDecimalPlaces,
62-
max: max != null ? Number(max) : undefined,
63-
min: min != null ? Number(min) : undefined,
64-
},
59+
num: { mask: Number, radix, thousandsSeparator, scale: defaultScale, max: maskMax, min: maskMin },
6560
},
6661
},
6762
{ onAccept: handleMaskAccept },

src/core/components/forms/inputNumber/inputNumber.test.tsx

Lines changed: 53 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ import * as InputHooks from '../hooks';
55
import { InputNumber, type IInputNumberProps } from './inputNumber';
66

77
describe('<InputNumber /> component', () => {
8+
const useNumberMaskMock = jest.spyOn(InputHooks, 'useNumberMask');
9+
10+
beforeEach(() => {
11+
useNumberMaskMock.mockReturnValue({} as unknown as InputHooks.IUseNumberMaskResult);
12+
});
13+
14+
afterEach(() => {
15+
useNumberMaskMock.mockReset();
16+
});
17+
818
const createTestComponent = (props?: Partial<IInputNumberProps>) => {
919
const completeProps: IInputNumberProps = {
1020
...props,
@@ -13,6 +23,34 @@ describe('<InputNumber /> component', () => {
1323
return <InputNumber {...completeProps} />;
1424
};
1525

26+
const testChangeValueLogic = async (values?: {
27+
props?: Partial<IInputNumberProps>;
28+
expectedValue?: string;
29+
type: 'increment' | 'decrement';
30+
}) => {
31+
const { props, expectedValue, type } = values ?? {};
32+
const user = userEvent.setup();
33+
const setUnmaskedValue = jest.fn();
34+
35+
const hookResult = {
36+
setUnmaskedValue,
37+
unmaskedValue: props?.value,
38+
} as unknown as InputHooks.IUseNumberMaskResult;
39+
40+
useNumberMaskMock.mockReturnValue(hookResult);
41+
render(createTestComponent({ ...props }));
42+
43+
const [decrementButton, incrementButton] = screen.getAllByRole('button');
44+
45+
if (type === 'increment') {
46+
await user.click(incrementButton);
47+
} else {
48+
await user.click(decrementButton);
49+
}
50+
51+
expect(setUnmaskedValue).toHaveBeenCalledWith(expectedValue);
52+
};
53+
1654
it('renders an input with increment and decrement buttons', () => {
1755
render(createTestComponent());
1856

@@ -42,109 +80,61 @@ describe('<InputNumber /> component', () => {
4280
});
4381

4482
describe('increment button', () => {
45-
const useNumberMaskMock = jest.spyOn(InputHooks, 'useNumberMask');
46-
47-
afterEach(() => {
48-
useNumberMaskMock.mockReset();
49-
});
50-
51-
const testIncrementLogic = async ({
52-
expectedValue,
53-
...props
54-
}: Partial<IInputNumberProps> & { expectedValue: string }) => {
55-
const user = userEvent.setup();
56-
const setValue = jest.fn();
57-
const hookResult = {
58-
setValue,
59-
value: props.value,
60-
unmaskedValue: props.value,
61-
} as unknown as InputHooks.IUseNumberMaskResult;
62-
useNumberMaskMock.mockReturnValue(hookResult);
63-
64-
render(createTestComponent({ ...props }));
65-
66-
const [, incrementButton] = screen.getAllByRole<HTMLButtonElement>('button');
67-
await user.click(incrementButton);
68-
69-
expect(setValue).toHaveBeenCalledWith(expectedValue);
70-
};
71-
7283
it('should increment by one (1) with default parameters', async () => {
73-
await testIncrementLogic({ expectedValue: '1' });
84+
await testChangeValueLogic({ type: 'increment', expectedValue: '1' });
7485
});
7586

7687
it('should return the maximum when the newly generated value exceeds the maximum', async () => {
7788
const max = 5;
7889
const step = 2;
7990
const value = '4';
80-
await testIncrementLogic({ max, step, value, expectedValue: max.toString() });
91+
const props = { max, step, value };
92+
await testChangeValueLogic({ type: 'increment', props, expectedValue: max.toString() });
8193
});
8294

8395
it('should increment by floating point value when the step is a float', async () => {
8496
const value = '1';
8597
const step = 0.5;
86-
await testIncrementLogic({ step, value, expectedValue: (Number(value) + step).toString() });
98+
const props = { step, value };
99+
await testChangeValueLogic({ type: 'increment', props, expectedValue: (Number(value) + step).toString() });
87100
});
88101

89102
it('should round down to the nearest multiple of the step before incrementing by the step value', async () => {
90103
const value = '1';
91104
const step = 0.3;
92-
await testIncrementLogic({ step, value, expectedValue: '1.2' });
105+
const props = { value, step };
106+
await testChangeValueLogic({ type: 'increment', props, expectedValue: '1.2' });
93107
});
94108

95109
it('should increment to the minimum when no value is provided', async () => {
96110
const step = 6;
97111
const min = 5;
98112
const max = 10;
99-
await testIncrementLogic({ step, min, max, expectedValue: min.toString() });
113+
const props = { step, min, max };
114+
await testChangeValueLogic({ type: 'increment', props, expectedValue: min.toString() });
100115
});
101116
});
102117

103118
describe('decrement button', () => {
104-
const useNumberMaskMock = jest.spyOn(InputHooks, 'useNumberMask');
105-
106-
afterEach(() => {
107-
useNumberMaskMock.mockReset();
108-
});
109-
110-
const testDecrementLogic = async ({
111-
expectedValue,
112-
...props
113-
}: Partial<IInputNumberProps> & { expectedValue: string }) => {
114-
const user = userEvent.setup();
115-
const setValue = jest.fn();
116-
const hookResult = {
117-
setValue,
118-
value: props.value,
119-
unmaskedValue: props.value,
120-
} as unknown as InputHooks.IUseNumberMaskResult;
121-
useNumberMaskMock.mockReturnValue(hookResult);
122-
123-
render(createTestComponent({ ...props }));
124-
125-
const [decrementButton] = screen.getAllByRole<HTMLButtonElement>('button');
126-
await user.click(decrementButton);
127-
128-
expect(setValue).toHaveBeenCalledWith(expectedValue);
129-
};
130-
131119
it('should decrement by step', async () => {
132120
const value = '10';
133121
const step = 2;
134-
const expectedValue = (10 - 2).toString();
135-
await testDecrementLogic({ value, step, expectedValue });
122+
const props = { value, step };
123+
await testChangeValueLogic({ type: 'decrement', props, expectedValue: (10 - 2).toString() });
136124
});
137125

138126
it('should decrement to the minimum when no value provided', async () => {
139127
const step = 2;
140128
const min = 1;
141-
await testDecrementLogic({ step, min, expectedValue: min.toString() });
129+
const props = { step, min };
130+
await testChangeValueLogic({ type: 'decrement', props, expectedValue: min.toString() });
142131
});
143132

144133
it('should decrement to the closest multiple of the step smaller than the value', async () => {
145134
const value = '10';
146135
const step = 3;
147-
await testDecrementLogic({ value, step, expectedValue: '9' });
136+
const props = { value, step };
137+
await testChangeValueLogic({ type: 'decrement', props, expectedValue: '9' });
148138
});
149139
});
150140
});

src/core/components/forms/inputNumber/inputNumber.tsx

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ export interface IInputNumberProps
2424
*/
2525
prefix?: string;
2626
/**
27-
* Specifies the granularity of the intervals for the input value
27+
* Specifies the granularity of the intervals for the input value.
28+
* @default 1
2829
*/
2930
step?: number;
3031
/**
@@ -41,14 +42,14 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
4142
const {
4243
max = Number.MAX_SAFE_INTEGER,
4344
min = Number.MIN_SAFE_INTEGER,
44-
step: inputStep = 1,
45+
step = 1,
4546
prefix,
4647
suffix,
4748
onChange,
4849
...otherProps
4950
} = props;
5051

51-
const step = inputStep <= 0 ? 1 : inputStep;
52+
const processedStep = step <= 0 ? 1 : step;
5253
const { containerProps, inputProps } = useInputProps(otherProps);
5354

5455
const { className: containerClassName, disabled, ...otherContainerProps } = containerProps;
@@ -57,7 +58,7 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
5758
const {
5859
ref: maskRef,
5960
unmaskedValue,
60-
setValue,
61+
setUnmaskedValue,
6162
} = useNumberMask({
6263
min,
6364
max,
@@ -82,31 +83,31 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
8283

8384
// increment directly to the minimum if value is less than the minimum
8485
if (parsedValue < min) {
85-
setValue(min.toString());
86+
setUnmaskedValue(min.toString());
8687
return;
8788
}
8889

8990
// ensure value is multiple of step
9091
const newValue = (Math.floor(parsedValue / step) + 1) * step;
9192

9293
// ensure the new value is than the max
93-
setValue(Math.min(max, newValue).toString());
94+
setUnmaskedValue(Math.min(max, newValue).toString());
9495
};
9596

9697
const handleDecrement = () => {
9798
const parsedValue = Number(unmaskedValue ?? 0);
9899

99100
// decrement directly to the maximum if value is greater than the maximum
100101
if (parsedValue > max) {
101-
setValue(max.toString());
102+
setUnmaskedValue(max.toString());
102103
return;
103104
}
104105

105106
// ensure value is multiple of step
106107
const newValue = (Math.ceil(parsedValue / step) - 1) * step;
107108

108109
// ensure the new value is than the max
109-
setValue(Math.max(min, newValue).toString());
110+
setUnmaskedValue(Math.max(min, newValue).toString());
110111
};
111112

112113
return (
@@ -122,7 +123,7 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
122123
)}
123124
<input
124125
ref={mergeRefs([maskRef, ref])}
125-
step={step}
126+
step={processedStep}
126127
max={max}
127128
min={min}
128129
inputMode="numeric"

src/core/components/forms/inputNumberMax/inputNumberMax.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe('<InputNumberMax /> component', () => {
1010
beforeEach(() => {
1111
const numberMaskResult = {
1212
ref: createRef(),
13-
setValue: jest.fn(),
13+
setUnmaskedValue: jest.fn(),
1414
} as unknown as InputHooks.IUseNumberMaskResult;
1515
useNumberMaskMock.mockReturnValue(numberMaskResult);
1616
});
@@ -37,12 +37,12 @@ describe('<InputNumberMax /> component', () => {
3737
it('updates the mask value with the max property on max button click', async () => {
3838
const user = userEvent.setup();
3939
const max = 1_000_000;
40-
const setValue = jest.fn();
41-
const hookResult = { setValue } as unknown as InputHooks.IUseNumberMaskResult;
40+
const setUnmaskedValue = jest.fn();
41+
const hookResult = { setUnmaskedValue } as unknown as InputHooks.IUseNumberMaskResult;
4242
useNumberMaskMock.mockReturnValue(hookResult);
4343
render(createTestComponent({ max }));
4444
await user.click(screen.getByRole('button'));
45-
expect(setValue).toHaveBeenCalledWith(max.toString());
45+
expect(setUnmaskedValue).toHaveBeenCalledWith(max.toString());
4646
});
4747

4848
it('does not render the max button when input is disabled', () => {

src/core/components/forms/inputNumberMax/inputNumberMax.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ export const InputNumberMax: React.FC<IInputNumberMaxProps> = (props) => {
2222
const { variant, ...otherContainerProps } = containerProps;
2323
const { className: inputClassName, value, min, disabled, ...otherInputProps } = inputProps;
2424

25-
const { ref, setValue } = useNumberMask({ min, max, value, onChange });
25+
const { ref, setUnmaskedValue } = useNumberMask({ min, max, value, onChange });
2626

2727
const { copy } = useOdsCoreContext();
2828

29-
const handleMaxClick = () => setValue(max.toString());
29+
const handleMaxClick = () => setUnmaskedValue(max.toString());
3030

3131
return (
3232
<InputContainer variant={variant} {...otherContainerProps}>

yarn.lock

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ __metadata:
119119
react: "npm:^18.3.1"
120120
react-dom: "npm:^18.3.1"
121121
react-dropzone: "npm:^14.2.0"
122-
react-imask: "npm:^7.6.0"
122+
react-imask: "npm:7.5.0"
123123
rollup: "npm:^4.20.0"
124124
rollup-plugin-peer-deps-external: "npm:^2.2.4"
125125
rollup-plugin-postcss: "npm:^4.0.2"
@@ -11077,7 +11077,7 @@ __metadata:
1107711077
languageName: node
1107811078
linkType: hard
1107911079

11080-
"imask@npm:^7.6.1":
11080+
"imask@npm:^7.5.0":
1108111081
version: 7.6.1
1108211082
resolution: "imask@npm:7.6.1"
1108311083
dependencies:
@@ -15513,15 +15513,15 @@ __metadata:
1551315513
languageName: node
1551415514
linkType: hard
1551515515

15516-
"react-imask@npm:^7.6.0":
15517-
version: 7.6.1
15518-
resolution: "react-imask@npm:7.6.1"
15516+
"react-imask@npm:7.5.0":
15517+
version: 7.5.0
15518+
resolution: "react-imask@npm:7.5.0"
1551915519
dependencies:
15520-
imask: "npm:^7.6.1"
15520+
imask: "npm:^7.5.0"
1552115521
prop-types: "npm:^15.8.1"
1552215522
peerDependencies:
1552315523
react: ">=0.14.0"
15524-
checksum: 10c0/48b8c234fb77e4d8b8446712695c63f2b0351a896b17623510a648408dd1b7ab506496124453a821528781b05fe2f5b04514f902e260f8b7f4486c972061da88
15524+
checksum: 10c0/755969ce03067608bc543731397b71586d2820094f94cf73e4bcce6c19214ccca7574f160456eee66537f1f64349a542c041df0ecd4b1afeb300f5a270fc31e2
1552515525
languageName: node
1552615526
linkType: hard
1552715527

0 commit comments

Comments
 (0)