Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed onBlur callback on NumberInput and AutocompleteInput #9730

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fixed onBlur callback on NumberInput and AutocompleteInput #9730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2443553
Fixed `useInput`'s return type
yanchesky Mar 18, 2024
a5d0d14
Added event object to onBlur function parameter to the `NumberInput`
yanchesky Mar 18, 2024
df9ab51
Added event object to onBlur function parameter to the `AutocompleteI…
yanchesky Mar 18, 2024
08b241b
Fixed `NumberInput`'s test to verify that the event object is properl…
yanchesky Mar 18, 2024
bc137d0
Added test to verify that the event object is properly passed to the …
yanchesky Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/ra-core/src/form/useInput.ts
View file
Open in desktop
Expand Up @@ -153,7 +153,9 @@ export type InputProps<ValueType = any> = Omit<
export type UseInputValue = {
id: string;
isRequired: boolean;
field: ControllerRenderProps;
field: Omit<ControllerRenderProps, 'onBlur'> & {
onBlur: (...event: any[]) => void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you use React's built-in FocusEventHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I relied on implementation of onBlur function in useInput

react-admin/packages/ra-core/src/form/useInput.ts

Lines 97 to 102 in e87f8f4

const onBlur = useEvent((...event: any[]) => {
controllerField.onBlur();
if (initialOnBlur) {
initialOnBlur(...event);
}
});

and onBlur of InputProps where type is written in such way.

There are also 2 cases where the FocusEventHandler will fail the type contract.

  1. A component needs to blur the input (BooleanInput, RichTextInput, FileInput, CheckboxGroupInput) because FocusEventHandler requires event as an argument.
const handleChange = useCallback(
        event => {
            field.onChange(event);
            // Ensure field is considered as touched
            field.onBlur();
        },
        [field]
    );
  1. RichTextInput provides EditorEvents['blur'] instead, which doesn't match with the FocusEvent.

Here is the solution I came up that matches type contracts. If you like it i'll commit it

 field: Omit<ControllerRenderProps, 'onBlur'> & {
        onBlur: (
            event?:
                | FocusEvent<HTMLInputElement | HTMLTextAreaElement>
                | EditorEvents['blur']
        ) => void;
    };

};
formState: UseFormStateReturn<Record<string, string>>;
fieldState: ControllerFieldState;
};
24 changes: 24 additions & 0 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.spec.tsx
View file
Open in desktop
Expand Up @@ -1282,6 +1282,30 @@ describe('<AutocompleteInput />', () => {
});
});

it('should pass the event object to the onBlur callback', async () => {
const onBlur = jest.fn();
render(
<AdminContext dataProvider={testDataProvider()}>
<SimpleForm onSubmit={jest.fn()}>
<AutocompleteInput
{...defaultProps}
choices={[{ id: 0, name: 'foo' }]}
onBlur={onBlur}
/>
</SimpleForm>
</AdminContext>
);
const input = screen.getByLabelText(
'resources.users.fields.role'
) as HTMLInputElement;

fireEvent.blur(input);

expect(onBlur).toHaveBeenCalledWith(
expect.objectContaining({ type: 'blur' })
);
});

describe('Inside <ReferenceInput>', () => {
it('should work inside a ReferenceInput field', async () => {
render(<InsideReferenceInput />);
Expand Down
37 changes: 20 additions & 17 deletions packages/ra-ui-materialui/src/input/AutocompleteInput.tsx
View file
Open in desktop
Expand Up @@ -418,24 +418,27 @@ If you provided a React element for the optionText prop, you must also provide t
]
);

const finalOnBlur = useCallback((): void => {
if (clearOnBlur && !multiple) {
const optionLabel = getOptionLabel(selectedChoice);
if (!isEqual(optionLabel, filterValue)) {
setFilterValue(optionLabel);
debouncedSetFilter('');
const finalOnBlur = useCallback(
(event): void => {
if (clearOnBlur && !multiple) {
const optionLabel = getOptionLabel(selectedChoice);
if (!isEqual(optionLabel, filterValue)) {
setFilterValue(optionLabel);
debouncedSetFilter('');
}
}
}
field.onBlur();
}, [
clearOnBlur,
field,
getOptionLabel,
selectedChoice,
filterValue,
debouncedSetFilter,
multiple,
]);
field.onBlur(event);
},
[
clearOnBlur,
field,
getOptionLabel,
selectedChoice,
filterValue,
debouncedSetFilter,
multiple,
]
);

useEffect(() => {
if (!multiple) {
Expand Down
4 changes: 3 additions & 1 deletion packages/ra-ui-materialui/src/input/NumberInput.spec.tsx
View file
Open in desktop
Expand Up @@ -484,7 +484,9 @@ describe('<NumberInput />', () => {
);
const input = screen.getByLabelText('resources.posts.fields.views');
fireEvent.blur(input);
expect(onBlur).toHaveBeenCalled();
expect(onBlur).toHaveBeenCalledWith(
expect.objectContaining({ type: 'blur' })
);
});

it('should display error message onBlur if required', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/input/NumberInput.tsx
View file
Open in desktop
Expand Up @@ -109,9 +109,9 @@ export const NumberInput = ({
hasFocus.current = true;
};

const handleBlur = () => {
const handleBlur = (event: React.FocusEvent<HTMLInputElement>) => {
if (onBlurFromField) {
onBlurFromField();
onBlurFromField(event);
}
hasFocus.current = false;
const stringValue = format(field.value);
Expand Down