Skip to content
This repository has been archived by the owner on Jul 8, 2024. It is now read-only.

Fix: DOS-237 - Added props.onchange to the callback dependency of numeric inputs and added onKeyDown property so we can bind to it #58

Merged
merged 5 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
5 changes: 5 additions & 0 deletions packages/ui-components/docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
title: Changelog
---

## NEXT

- Added `onKeyDown` prop to NumericInput to bubble up keydown events
- Added the `onChange` property to the `onChange` callback dependency

## 1.4.3

- Pinned fortawesome to `~6.4.0`
Expand Down
75 changes: 74 additions & 1 deletion packages/ui-components/src/numeric-input/numeric-input.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
* limitations under the License.
*/
/* eslint-disable jest/no-disabled-tests */
import { render } from '@testing-library/react';
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';

import { ThemeProvider, theme } from '@darajs/styled-components';

Expand All @@ -31,6 +32,9 @@ function RenderNumericInput(props: NumericInputProps): JSX.Element {
}

describe('Numeric Input', () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('should only accept numeric input', async () => {
const { getByRole, rerender } = render(<RenderNumericInput />);
const input = getByRole('textbox', { hidden: true });
Expand Down Expand Up @@ -101,6 +105,75 @@ describe('Numeric Input', () => {
expect(onChangeStub.mock.results[0].value).toEqual(21);
});

it('should update the callback method with latest values', async () => {
const onChangeStub = jest.fn((isChangeCalled, value) => [isChangeCalled, value]);

function TestItem(): React.ReactNode {
const [isButtonClicked, setIsButtonClicked] = React.useState<boolean>(false);

const onChange = React.useCallback(
(value: number): void => {
onChangeStub(isButtonClicked, value);
},
[isButtonClicked]
);
return (
<div>
<RenderNumericInput onChange={onChange} value={2} />
<input onClick={() => setIsButtonClicked(true)} type="button" value="Click" />
</div>
);
}

const { getByRole } = render(<TestItem />);
const input = getByRole('textbox', { hidden: true });

// change it for the first time, the isButtonClicked value should still be false
await act(async () => {
await userEvent.type(input, '1', { delay: 1 });
});

// expect the mock to be called with the array result of [21, false]
await waitFor(() => {
expect(onChangeStub).toHaveBeenCalledTimes(1);
expect(onChangeStub.mock.results[0].value[1]).toEqual(21);
expect(onChangeStub.mock.results[0].value[0]).toEqual(false);
});

// fire the click on a different act to make sure it happens before the changes
act(() => {
fireEvent.click(screen.getByRole('button', { name: 'Click' }));
});

// click the button, the value should be true
await act(async () => {
await userEvent.type(input, '5', { delay: 1 });
});

// expect the mock to be called with the array result of [21, true] since we clicked the button
await waitFor(() => {
expect(onChangeStub).toHaveBeenCalledTimes(2);
expect(onChangeStub.mock.results[1].value[1]).toEqual(25);
expect(onChangeStub.mock.results[1].value[0]).toEqual(true);
});
});

it('should pass keydown to the parent', async () => {
const onKeydownStub = jest.fn((value) => value.key);

const { getByRole } = render(<RenderNumericInput onKeyDown={onKeydownStub} value={2} />);
const input = getByRole('textbox');

act(() => {
fireEvent.keyDown(input, { key: 'Enter' });
});

await waitFor(() => {
expect(onKeydownStub).toHaveBeenCalledTimes(1);
expect(onKeydownStub.mock.results[0].value).toEqual('Enter');
});
});

it('should implement the stepper correctly', async () => {
const { getByRole, getAllByRole } = render(<RenderNumericInput stepper />);
const input = getByRole('textbox', { hidden: true });
Expand Down
7 changes: 6 additions & 1 deletion packages/ui-components/src/numeric-input/numeric-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ export interface NumericInputProps extends InteractiveComponentProps<number> {
onChange?: (value: number, e?: React.SyntheticEvent<HTMLInputElement>) => void | Promise<void>;
/** An optional event listener for complete events (enter presses) */
onComplete?: () => void | Promise<void>;
/** An optional event listener for keydown events if needed */
jsumiguin-causalens marked this conversation as resolved.
Show resolved Hide resolved
onKeyDown?: (e: React.KeyboardEvent<HTMLInputElement>) => void | Promise<void>;
/** An optional placeholder that will be used when the input is empty, defaults to '' */
placeholder?: string;
/** An optional property to set how many steps the stepper should take */
Expand Down Expand Up @@ -215,6 +217,9 @@ const NumericInput = React.forwardRef<HTMLInputElement, NumericInputProps>(
};

const onKeyDown = (e: KeyboardEvent<HTMLInputElement>): void => {
// run the keydown event handler if it exists
props.onKeyDown?.(e);

if (!props.stepper) {
return;
}
Expand Down Expand Up @@ -246,7 +251,7 @@ const NumericInput = React.forwardRef<HTMLInputElement, NumericInputProps>(
props.onChange?.(parsed, e);
},
// eslint-disable-next-line react-hooks/exhaustive-deps
jsumiguin-causalens marked this conversation as resolved.
Show resolved Hide resolved
[props.integerOnly, props.value]
[props.integerOnly, props.value, props.onChange]
);

useEffect(() => {
Expand Down
Loading