From e57c35c7a45bbba1666d4d67355085ab59c8c49e Mon Sep 17 00:00:00 2001 From: Stephen Lee Date: Tue, 27 Feb 2024 12:31:35 -0800 Subject: [PATCH] LG-3940: NumberInput onBlur runs with arrow buttons (#2225) * Extend Arrows with onBlur * Refactor Input subcomponent * NumberInput onBlur prop * Update NumberInput story to log onBlur * Changeset * Address feedback * Address additional feedback * Add comment * Add test block and change testid for buttons --- .changeset/short-games-double.md | 6 ++ packages/number-input/src/Arrows/Arrow.tsx | 1 + .../number-input/src/Arrows/Arrows.types.ts | 3 + packages/number-input/src/Input/Input.tsx | 19 ++-- .../number-input/src/NumberInput.story.tsx | 8 ++ .../src/NumberInput/NumberInput.spec.tsx | 95 ++++++++++++++----- .../src/NumberInput/NumberInput.types.ts | 11 ++- 7 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 .changeset/short-games-double.md diff --git a/.changeset/short-games-double.md b/.changeset/short-games-double.md new file mode 100644 index 0000000000..bb3e988db6 --- /dev/null +++ b/.changeset/short-games-double.md @@ -0,0 +1,6 @@ +--- +'@leafygreen-ui/number-input': patch +--- + +Fixes a bug where onBlur would not get invoked when arrow buttons were blurred. +[LG-3940](https://jira.mongodb.org/browse/LG-3940) diff --git a/packages/number-input/src/Arrows/Arrow.tsx b/packages/number-input/src/Arrows/Arrow.tsx index 42cbd5b0d3..9618eade1f 100644 --- a/packages/number-input/src/Arrows/Arrow.tsx +++ b/packages/number-input/src/Arrows/Arrow.tsx @@ -34,6 +34,7 @@ export const Arrow = ({ type="button" tabIndex={-1} // Mimicking native behavior; you cannot focus on an arrow. disabled={disabled} + data-testid={`lg-number_input-${direction}_button`} > ( { value: valueProp, onChange: onChangeProp, + onBlur, disabled = false, size = Size.Default, state = State.None, @@ -162,23 +164,28 @@ export const Input = React.forwardRef( } }; - const handleOnFocus = () => { + const handleFocus = () => { isFocusedRef.current = true; handleSetErrorTransition(); }; - const handleOnBlur = () => { + const handleBlur = (e: FocusEvent) => { isFocusedRef.current = false; handleRemoveErrorTransition(); + // If newly focused element is a child of the input container, we do not invoke onBlur + const inputContainer = e.currentTarget as Node; + const possibleChildOfInputContainer = e.relatedTarget as Node | null; + if (inputContainer.contains(possibleChildOfInputContainer)) return; + onBlur?.(e); }; return (
handleSetErrorTransition()} - onMouseLeave={() => handleRemoveErrorTransition()} - onFocus={() => handleOnFocus()} - onBlur={() => handleOnBlur()} + onMouseEnter={handleSetErrorTransition} + onMouseLeave={handleRemoveErrorTransition} + onFocus={handleFocus} + onBlur={handleBlur} aria-disabled={disabled} className={cx( wrapperClassName, diff --git a/packages/number-input/src/NumberInput.story.tsx b/packages/number-input/src/NumberInput.story.tsx index c1e6371aa9..d0277b89e2 100644 --- a/packages/number-input/src/NumberInput.story.tsx +++ b/packages/number-input/src/NumberInput.story.tsx @@ -133,6 +133,13 @@ const Template: StoryFn = ( setValue(e.target.value); }; + const handleBlur = (e: React.FocusEvent) => { + // eslint-disable-next-line no-console + console.log('story: event: ', e.target.blur); + // eslint-disable-next-line no-console + console.log('story: ref', inputRef.current?.blur); + }; + return ( = ( unitOptions={unitOptions} onSelectChange={handleSelectChange} onChange={handleChange} + onBlur={handleBlur} inputClassName={css` width: 100px; `} diff --git a/packages/number-input/src/NumberInput/NumberInput.spec.tsx b/packages/number-input/src/NumberInput/NumberInput.spec.tsx index aba7933e12..f9d11c7c7f 100644 --- a/packages/number-input/src/NumberInput/NumberInput.spec.tsx +++ b/packages/number-input/src/NumberInput/NumberInput.spec.tsx @@ -9,12 +9,14 @@ import { NumberInput } from '.'; const label = 'This is the label text'; const description = 'This is the description text'; const errorMessage = 'error message'; +const arrowTestId = { + up: 'lg-number_input-increment_button', + down: 'lg-number_input-decrement_button', +}; const defaultProps = { className: 'number-input-class', placeholder: 'This is some placeholder text', - onChange: jest.fn(), - onBlur: jest.fn(), }; const unitProps = { @@ -133,8 +135,10 @@ describe('packages/number-input', () => { }); test('value change triggers onChange callback', () => { + const onChange = jest.fn(); const { numberInput } = renderNumberInput({ label, + onChange, ...defaultProps, }); @@ -145,7 +149,7 @@ describe('packages/number-input', () => { }); expect((numberInput as HTMLInputElement).value).toBe('1'); - expect(defaultProps.onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledTimes(1); }); test('correct value is returned when using "e"', () => { @@ -202,45 +206,86 @@ describe('packages/number-input', () => { expect((numberInput as HTMLInputElement).value).toBe('111'); }); - test('blur triggers onBlur callback', () => { - const { numberInput } = renderNumberInput({ + test('value changes when "up" arrow is clicked', () => { + const { getByTestId, numberInput } = renderNumberInput({ label, ...defaultProps, }); - userEvent.tab(); // focus - expect(numberInput).toHaveFocus(); - userEvent.tab(); // blur + const upArrow = getByTestId(arrowTestId.up); - expect(defaultProps.onBlur).toHaveBeenCalledTimes(1); + userEvent.click(upArrow); + expect((numberInput as HTMLInputElement).value).toBe('1'); }); - test('value changes when "up" arrow is clicked', () => { - const { container, numberInput } = renderNumberInput({ + test('value changes when "down" arrow is clicked', () => { + const { getByTestId, numberInput } = renderNumberInput({ label, ...defaultProps, }); - const upArrow = container.querySelector( - 'button[aria-label="increment number"]', - ); + const downArrow = getByTestId(arrowTestId.down); - userEvent.click(upArrow as HTMLButtonElement); - expect((numberInput as HTMLInputElement).value).toBe('1'); + userEvent.click(downArrow); + expect((numberInput as HTMLInputElement).value).toBe('-1'); }); - test('value changes when "down" arrow is clicked', () => { - const { container, numberInput } = renderNumberInput({ - label, - ...defaultProps, + describe('onBlur', () => { + test('callback triggers when focus leaves number input', () => { + const onBlur = jest.fn(); + const { numberInput } = renderNumberInput({ + label, + onBlur, + ...defaultProps, + }); + + userEvent.tab(); // focus + expect(numberInput).toHaveFocus(); + userEvent.tab(); // blur + + expect(onBlur).toHaveBeenCalledTimes(1); }); - const upArrow = container.querySelector( - 'button[aria-label="decrement number"]', - ); + test('callback triggers when focus leaves arrow buttons', () => { + const onBlur = jest.fn(); + const { getByTestId } = renderNumberInput({ + label, + onBlur, + ...defaultProps, + }); - userEvent.click(upArrow as HTMLButtonElement); - expect((numberInput as HTMLInputElement).value).toBe('-1'); + const upArrow = getByTestId(arrowTestId.up); + + userEvent.click(upArrow); // focus + expect(upArrow).toHaveFocus(); + + userEvent.tab(); // blur + expect(onBlur).toHaveBeenCalledTimes(1); + }); + + test('callback does not trigger when focus changes to adjacent elements within number input', () => { + const onBlur = jest.fn(); + const { getByTestId, numberInput } = renderNumberInput({ + label, + onBlur, + ...defaultProps, + }); + + const upArrow = getByTestId(arrowTestId.up); + const downArrow = getByTestId(arrowTestId.down); + + userEvent.tab(); // focus + expect(numberInput).toHaveFocus(); + + userEvent.click(upArrow); // focus on up arrow + expect(upArrow).toHaveFocus(); + + userEvent.click(downArrow); // focus on down arrow + expect(downArrow).toHaveFocus(); + + userEvent.tab(); //blur + expect(onBlur).toHaveBeenCalledTimes(1); + }); }); test(`is disabled when disabled is passed`, () => { diff --git a/packages/number-input/src/NumberInput/NumberInput.types.ts b/packages/number-input/src/NumberInput/NumberInput.types.ts index 64010d4418..aa99414fd6 100644 --- a/packages/number-input/src/NumberInput/NumberInput.types.ts +++ b/packages/number-input/src/NumberInput/NumberInput.types.ts @@ -1,4 +1,8 @@ -import { ChangeEventHandler, ComponentPropsWithoutRef } from 'react'; +import { + ChangeEventHandler, + ComponentPropsWithoutRef, + FocusEventHandler, +} from 'react'; import { AriaLabelPropsWithLabel } from '@leafygreen-ui/a11y'; import { DarkModeProps } from '@leafygreen-ui/lib'; @@ -108,6 +112,11 @@ export interface BaseNumberInputProps */ onChange?: ChangeEventHandler; + /** + * Callback fired when the input or arrows lose focus + */ + onBlur?: FocusEventHandler; + /** * id associated with the PasswordInput component, referenced by `