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

Commit

Permalink
Fix: DOS-237 - Added props.onchange to the callback dependency of num…
Browse files Browse the repository at this point in the history
…eric inputs and added onKeyDown property so we can bind to it (#58)

* Added props.onchange to the callback dependency as the onChange does not change in sync with the parent

* added tests and changelog

* fixed changelog

* removed if needed
  • Loading branch information
jsumiguin-causalens authored and PatriciaJacob committed Jan 5, 2024
1 parent 1686303 commit dfba205
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 2 deletions.
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 */
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
[props.integerOnly, props.value]
[props.integerOnly, props.value, props.onChange]
);

useEffect(() => {
Expand Down

0 comments on commit dfba205

Please sign in to comment.