Skip to content

Commit

Permalink
fix: Fix input-number component to trigger onChange callback on + / -…
Browse files Browse the repository at this point in the history
… button click
  • Loading branch information
cgero-eth committed Oct 23, 2024
1 parent 411e571 commit c9ea9f5
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 73 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

### Fixed

- Fix `<InputNumber >` core component to trigger `onChange` callback on + / - button click

## [1.0.50] - 2024-10-23

### Changed
Expand Down
18 changes: 9 additions & 9 deletions src/core/components/forms/hooks/useNumberMask.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('useNumberMask hook', () => {
const formatNumberMock = jest.spyOn(formatterUtils, 'formatNumber');

beforeEach(() => {
const maskResult = { setValue: jest.fn() } as unknown as IUseNumberMaskResult;
const maskResult = { setUnmaskedValue: jest.fn() } as unknown as IUseNumberMaskResult;
maskMock.mockReturnValue(maskResult);
});

Expand All @@ -22,7 +22,7 @@ describe('useNumberMask hook', () => {
});

it('returns the result of the useIMask hook', () => {
const maskResult = { setValue: jest.fn(), setUnmaskedValue: jest.fn() } as unknown as IUseNumberMaskResult;
const maskResult = { setUnmaskedValue: jest.fn() } as unknown as IUseNumberMaskResult;
maskMock.mockReturnValue(maskResult);
const { result } = renderHook(() => useNumberMask({}));
expect(result.current).toEqual(maskResult);
Expand Down Expand Up @@ -77,18 +77,18 @@ describe('useNumberMask hook', () => {
expect(maskMock).toHaveBeenCalledWith(expect.objectContaining({ mask: `num ${suffix}` }), expect.anything());
});

it('updates the mask value on value property change for controlled inputs', () => {
it('updates the unmasked value on value property change for controlled inputs', () => {
const value = '100';
const setValue = jest.fn();
const maskResult = { setValue } as unknown as IUseNumberMaskResult;
const setUnmaskedValue = jest.fn();
const maskResult = { setUnmaskedValue } as unknown as IUseNumberMaskResult;
maskMock.mockReturnValue(maskResult);

const { rerender } = renderHook((props) => useNumberMask(props), { initialProps: { value } });
expect(setValue).toHaveBeenCalledWith(value);
expect(setUnmaskedValue).toHaveBeenCalledWith(value);

const newValue = '101';
rerender({ value: newValue });
expect(setValue).toHaveBeenCalledWith(newValue);
const unmaskedValue = '101';
rerender({ value: unmaskedValue });
expect(setUnmaskedValue).toHaveBeenCalledWith(unmaskedValue);
});

it('calls the onChange property with the unmasked value when value is valid', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/core/components/forms/hooks/useNumberMask.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ export const useNumberMask = (props: IUseNumberMaskProps): IUseNumberMaskResult
{ onAccept: handleMaskAccept },
);

const { setValue } = result;
const { setUnmaskedValue } = result;

// Update the masked value on value property change
useEffect(() => {
const parsedValue = value?.toString() ?? '';
setValue(parsedValue);
}, [setValue, value]);
setUnmaskedValue(parsedValue);
}, [setUnmaskedValue, value]);

return result;
};
11 changes: 11 additions & 0 deletions src/core/components/forms/inputNumber/inputNumber.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,15 @@ export const Controlled: Story = {
},
};

/**
* Usage example with min and max values.
*/
export const MinMax: Story = {
args: {
min: 10,
max: 20,
placeholder: '0',
},
};

export default meta;
72 changes: 42 additions & 30 deletions src/core/components/forms/inputNumber/inputNumber.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@ describe('<InputNumber /> component', () => {
}) => {
const { props, expectedValue, type } = values ?? {};
const user = userEvent.setup();

const setUnmaskedValue = jest.fn();
const onChange = jest.fn();

const hookResult = {
setUnmaskedValue,
unmaskedValue: props?.value,
unmaskedValue: props?.value ?? '',
} as unknown as InputHooks.IUseNumberMaskResult;

useNumberMaskMock.mockReturnValue(hookResult);
render(createTestComponent({ ...props }));
render(createTestComponent({ ...props, onChange }));

const [decrementButton, incrementButton] = screen.getAllByRole('button');

Expand All @@ -49,11 +51,11 @@ describe('<InputNumber /> component', () => {
}

expect(setUnmaskedValue).toHaveBeenCalledWith(expectedValue);
expect(onChange).toHaveBeenCalledWith(expectedValue);
};

it('renders an input with increment and decrement buttons', () => {
render(createTestComponent());

expect(screen.getByRole('textbox')).toBeInTheDocument();
expect(screen.getAllByRole('button').length).toEqual(2);
expect(screen.getByTestId(IconType.PLUS)).toBeInTheDocument();
Expand All @@ -62,79 +64,89 @@ describe('<InputNumber /> component', () => {

it('renders a disabled input with no spin buttons when disabled is set to true', () => {
render(createTestComponent({ disabled: true }));

expect(screen.getByRole('textbox')).toBeDisabled();
expect(screen.queryAllByRole('button').length).toEqual(0);
});

it('should default step to 1 when given value less than zero', () => {
it('defaults step to 1 when given value less than zero', () => {
const step = -15;
render(createTestComponent({ step }));
expect(screen.getByRole('textbox')).toHaveAttribute('step', '1');
});

it('should default step to 1 when given value is zero', () => {
it('defaults step to 1 when given value is zero', () => {
const step = 0;
render(createTestComponent({ step }));
expect(screen.getByRole('textbox')).toHaveAttribute('step', '1');
});

describe('increment button', () => {
it('should increment by one (1) with default parameters', async () => {
it('increments by 1 with default parameters', async () => {
await testChangeValueLogic({ type: 'increment', expectedValue: '1' });
});

it('should return the maximum when the newly generated value exceeds the maximum', async () => {
it('returns max value when new value is greater than max value', async () => {
const max = 5;
const step = 2;
const value = '4';
const props = { max, step, value };
await testChangeValueLogic({ type: 'increment', props, expectedValue: max.toString() });
});

it('should increment by floating point value when the step is a float', async () => {
it('increments by floating point value when the step is a float', async () => {
const value = '1';
const step = 0.5;
const props = { step, value };
await testChangeValueLogic({ type: 'increment', props, expectedValue: (Number(value) + step).toString() });
});

it('should round down to the nearest multiple of the step before incrementing by the step value', async () => {
it('increments by provided step', async () => {
const value = '10';
const step = 2;
const props = { value, step };
await testChangeValueLogic({ type: 'increment', props, expectedValue: '12' });
});

it('rounds down to the nearest multiple of the step before incrementing by the step value', async () => {
const value = '1';
const step = 0.3;
const props = { value, step };
await testChangeValueLogic({ type: 'increment', props, expectedValue: '1.2' });
});

it('should increment to the minimum when no value is provided', async () => {
const step = 6;
const min = 5;
const max = 10;
const props = { step, min, max };
await testChangeValueLogic({ type: 'increment', props, expectedValue: min.toString() });
});
});

describe('decrement button', () => {
it('should decrement by step', async () => {
const value = '10';
const step = 2;
const props = { value, step };
await testChangeValueLogic({ type: 'decrement', props, expectedValue: (10 - 2).toString() });
it('decrements by 1 with default parameters', async () => {
await testChangeValueLogic({ type: 'decrement', expectedValue: '-1' });
});

it('should decrement to the minimum when no value provided', async () => {
const step = 2;
const min = 1;
const props = { step, min };
it('returns min value when new value is less than min value', async () => {
const min = 3;
const step = 3;
const value = '5';
const props = { min, step, value };
await testChangeValueLogic({ type: 'decrement', props, expectedValue: min.toString() });
});

it('should decrement to the closest multiple of the step smaller than the value', async () => {
it('decrements by floating point value when the step is a float', async () => {
const value = '1';
const step = 0.5;
const props = { step, value };
await testChangeValueLogic({ type: 'decrement', props, expectedValue: (Number(value) - step).toString() });
});

it('decrements by provided step', async () => {
const value = '10';
const step = 3;
const step = 2;
const props = { value, step };
await testChangeValueLogic({ type: 'decrement', props, expectedValue: '8' });
});

it('rounds up to the nearest multiple of the step before decrementing by the step value', async () => {
const value = '1.3';
const step = 0.3;
const props = { value, step };
await testChangeValueLogic({ type: 'decrement', props, expectedValue: '9' });
await testChangeValueLogic({ type: 'decrement', props, expectedValue: '1.2' });
});
});
});
44 changes: 13 additions & 31 deletions src/core/components/forms/inputNumber/inputNumber.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,44 +70,26 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop

const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
if (e.key === 'ArrowUp') {
handleIncrement();
handleStepChange(1);
} else if (e.key === 'ArrowDown') {
handleDecrement();
handleStepChange(-1);
}

onKeyDown?.(e);
};

const handleIncrement = () => {
const parsedValue = Number(unmaskedValue ?? 0);
const handleStepChange = (change: number) => {
const parsedValue = Number(unmaskedValue);

// increment directly to the minimum if value is less than the minimum
if (parsedValue < min) {
setUnmaskedValue(min.toString());
return;
}

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

// ensure the new value is than the max
setUnmaskedValue(Math.min(max, newValue).toString());
};

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

// decrement directly to the maximum if value is greater than the maximum
if (parsedValue > max) {
setUnmaskedValue(max.toString());
return;
}
// Ensure value is multiple of step
const multipleValue = change > 0 ? Math.floor(parsedValue / step) : Math.ceil(parsedValue / step);
const stepValue = (multipleValue + change) * step;

// ensure value is multiple of step
const newValue = (Math.ceil(parsedValue / step) - 1) * step;
// Ensure value between min and max
const processedValue = Math.min(max, Math.max(min, stepValue)).toString();

// ensure the new value is than the max
setUnmaskedValue(Math.max(min, newValue).toString());
setUnmaskedValue(processedValue);
onChange?.(processedValue);
};

return (
Expand All @@ -116,7 +98,7 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
<Button
size="sm"
variant="tertiary"
onClick={handleDecrement}
onClick={() => handleStepChange(-1)}
iconLeft={IconType.MINUS}
className="ml-2 shrink-0"
/>
Expand All @@ -136,7 +118,7 @@ export const InputNumber = forwardRef<HTMLInputElement, IInputNumberProps>((prop
size="sm"
variant="tertiary"
iconLeft={IconType.PLUS}
onClick={handleIncrement}
onClick={() => handleStepChange(1)}
className="mr-2 shrink-0"
/>
)}
Expand Down

0 comments on commit c9ea9f5

Please sign in to comment.