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.

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

Make useSearchField behave close to browser #6404

Merged
merged 5 commits into from
May 22, 2024

Conversation

QzCurious
Copy link
Contributor

@QzCurious QzCurious commented May 17, 2024

This commit changes how useSearchField behave. It previously rely on onSubmit prop to get "Enter submission". For now, "Enter" would trigger a normal form submission by default browser behavior. For backward compatibility, if onSubmit prop is provide, onSubmit is called and default form submission is prevented.

related #6332

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Just render a SearchField and test it with/without onSubmit:

(code modified form packages/react-aria-components/stories/SearchField.stories.tsx)

import {Button, Input, Label, SearchField} from 'react-aria-components';
import {classNames} from '@react-spectrum/utils';
import React from 'react';
import styles from '../example/index.css';

export default {
  title: 'React Aria Components'
};

export const SearchFieldExample = () => {
  return (
    <form
      onSubmit={(e) => {
        alert('browser default form submit');
        e.preventDefault();
      }}>
      <SearchField
        // onSubmit={() => alert('onSubmit prop')}
        className={classNames(styles, 'searchFieldExample')}
        data-testid="search-field-example">
        <Label>Search</Label>
        <Input />
        <Button></Button>
      </SearchField>
    </form>
  );
};
Screen.Recording.2024-05-17.at.5.34.54.PM.mp4

🧢 Your Project:

https://github.com/QzCurious/react-spectrum/tree/fix/useSearchField

This commit changes how `useSearchField` behave. It previously
rely on `onSubmit` prop to get "Enter submission". For now,
"Enter" would trigger a normal form submission by default browser
behavior. For backward compatibility, if onSubmit prop is provide,
`onSubmit` is called and default form submission is prevented.

related adobe#6332
Copy link
Member

@ktabors ktabors left a comment

Choose a reason for hiding this comment

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

Thanks for updating the tests.

@@ -67,12 +67,10 @@ describe('useSearchField hook', () => {
onSubmit.mockClear();
});

it('preventDefault is called for Enter and not Escape', () => {
it('preventDefault is not called for Escape', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we combine this test and the following?

Suggested change
it('preventDefault is not called for Escape', () => {
it('preventDefault and stopPropagation are not called for Escape', () => {

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've merged the two test cases.

snowystinger
snowystinger previously approved these changes May 21, 2024
@QzCurious QzCurious requested a review from ktabors May 21, 2024 06:03
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Verified that the backcompat works and that omitting onSubmit properly causes native form validation, thanks for the fix!

@snowystinger snowystinger merged commit c57671e into adobe:main May 22, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants