Skip to content

Commit

Permalink
Fix: DO-3869 input component bug (#370)
Browse files Browse the repository at this point in the history
* numeric input fix

* changelog

* add test
  • Loading branch information
patricia-causalens authored Sep 30, 2024
1 parent de7f73c commit 52c3d46
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 2 deletions.
4 changes: 4 additions & 0 deletions packages/ui-components/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
title: Changelog
---

## NEXT

- Fixed an issue with `NumericInput` where in controlled mode if number ended in `.` this could not be erased.

## 1.12.7

- Fix an issue where body's `line-height` was not consistent across packages.
Expand Down
21 changes: 21 additions & 0 deletions packages/ui-components/src/numeric-input/numeric-input.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ function RenderNumericInput(props: NumericInputProps): JSX.Element {
);
}

const MockControlledNumericInput = (): JSX.Element => {
const [value, setValue] = React.useState();

const handleChange = (newValue): void => {
setValue(newValue);
};

return <RenderNumericInput value={value} onChange={handleChange} />;
};

describe('Numeric Input', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -95,6 +105,17 @@ describe('Numeric Input', () => {
expect(input).toHaveValue('5');
});

it('should erase . if decimal value was not fully entered', async () => {
const { getByRole } = render(<MockControlledNumericInput />);
const input = getByRole('textbox', { hidden: true });

await userEvent.type(input, '10.', { delay: 1 });
expect(input).toHaveValue('10.');

userEvent.keyboard('{backspace}');
expect(input).toHaveValue('10');
});

it('should listen to changes to input', async () => {
const onChangeStub = jest.fn((value) => value);

Expand Down
13 changes: 13 additions & 0 deletions packages/ui-components/src/numeric-input/numeric-input.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/
import { Meta } from '@storybook/react';
import * as React from 'react';

import { default as NumericInputComponent, NumericInputProps } from './numeric-input';

Expand All @@ -28,3 +29,15 @@ export const NumericInput = (args: NumericInputProps): JSX.Element => <NumericIn
NumericInput.args = {
stepper: true,
} as NumericInputProps;

export const ControlledNumericInput = (args: NumericInputProps): JSX.Element => {
const [value, setValue] = React.useState<number | undefined>(undefined);
args.onChange = setValue;
args.value = value;

return <NumericInputComponent {...args} />;
};

ControlledNumericInput.args = {
stepper: true,
} as NumericInputProps;
10 changes: 8 additions & 2 deletions packages/ui-components/src/numeric-input/numeric-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ const NumericInput = React.forwardRef<HTMLInputElement, NumericInputProps>(
onChange?.(parsed, e);
return;
}
// In controlled mode, we need to take over the input updates whenever the value is not a valid number
// This way onchange is only called with valid number updates and not when user is still entering a valid number

// if the value ends with a period, don't call onChange as it's not yet a valid number
if (v.endsWith('.')) {
setInput(v);
Expand All @@ -247,10 +250,13 @@ const NumericInput = React.forwardRef<HTMLInputElement, NumericInputProps>(
setInput(v);
return;
}
// When the input ends with . and the user backspaces, we should update the input as the value won't have changed
if (input.endsWith('.')) {
setInput(v);
}
onChange?.(parsed, e);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[props.integerOnly, value, onChange]
[props.integerOnly, value, onChange, input]
);

useEffect(() => {
Expand Down

0 comments on commit 52c3d46

Please sign in to comment.