Skip to content

Commit

Permalink
LG-3940: NumberInput onBlur runs with arrow buttons (#2225)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
stephl3 authored Feb 27, 2024
1 parent 8142d30 commit e57c35c
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 32 deletions.
6 changes: 6 additions & 0 deletions .changeset/short-games-double.md
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions packages/number-input/src/Arrows/Arrow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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`}
>
<Icon
className={cx({
Expand Down
3 changes: 3 additions & 0 deletions packages/number-input/src/Arrows/Arrows.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,8 @@ export interface ArrowsProps {
}

export interface ArrowProps extends ArrowsProps {
/**
* Direction of arrow: increment or decrement
*/
direction: Direction;
}
19 changes: 13 additions & 6 deletions packages/number-input/src/Input/Input.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, {
ChangeEvent,
FocusEvent,
KeyboardEvent,
useEffect,
useRef,
Expand Down Expand Up @@ -43,6 +44,7 @@ export const Input = React.forwardRef<HTMLInputElement, InputProps>(
{
value: valueProp,
onChange: onChangeProp,
onBlur,
disabled = false,
size = Size.Default,
state = State.None,
Expand Down Expand Up @@ -162,23 +164,28 @@ export const Input = React.forwardRef<HTMLInputElement, InputProps>(
}
};

const handleOnFocus = () => {
const handleFocus = () => {
isFocusedRef.current = true;
handleSetErrorTransition();
};

const handleOnBlur = () => {
const handleBlur = (e: FocusEvent<HTMLDivElement>) => {
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 (
<div
ref={containerRef}
onMouseEnter={() => handleSetErrorTransition()}
onMouseLeave={() => handleRemoveErrorTransition()}
onFocus={() => handleOnFocus()}
onBlur={() => handleOnBlur()}
onMouseEnter={handleSetErrorTransition}
onMouseLeave={handleRemoveErrorTransition}
onFocus={handleFocus}
onBlur={handleBlur}
aria-disabled={disabled}
className={cx(
wrapperClassName,
Expand Down
8 changes: 8 additions & 0 deletions packages/number-input/src/NumberInput.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ const Template: StoryFn<StoryNumberInputProps> = (
setValue(e.target.value);
};

const handleBlur = (e: React.FocusEvent<HTMLElement>) => {
// 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 (
<NumberInput
{...rest}
Expand All @@ -142,6 +149,7 @@ const Template: StoryFn<StoryNumberInputProps> = (
unitOptions={unitOptions}
onSelectChange={handleSelectChange}
onChange={handleChange}
onBlur={handleBlur}
inputClassName={css`
width: 100px;
`}
Expand Down
95 changes: 70 additions & 25 deletions packages/number-input/src/NumberInput/NumberInput.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -133,8 +135,10 @@ describe('packages/number-input', () => {
});

test('value change triggers onChange callback', () => {
const onChange = jest.fn();
const { numberInput } = renderNumberInput({
label,
onChange,
...defaultProps,
});

Expand All @@ -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"', () => {
Expand Down Expand Up @@ -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`, () => {
Expand Down
11 changes: 10 additions & 1 deletion packages/number-input/src/NumberInput/NumberInput.types.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -108,6 +112,11 @@ export interface BaseNumberInputProps
*/
onChange?: ChangeEventHandler<HTMLInputElement>;

/**
* Callback fired when the input or arrows lose focus
*/
onBlur?: FocusEventHandler<HTMLDivElement>;

/**
* id associated with the PasswordInput component, referenced by `<label>` with the `for` attribute.
*/
Expand Down

0 comments on commit e57c35c

Please sign in to comment.