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

Button component has no default type #2806

Closed
pveierland opened this issue Jan 24, 2023 · 3 comments · Fixed by #2807
Closed

Button component has no default type #2806

pveierland opened this issue Jan 24, 2023 · 3 comments · Fixed by #2807
Labels
bug Something isn't working component: Button Issues related to the Button component react

Comments

@pveierland
Copy link

Description

The Button component does not have a default type attribute. In e.g. react-bootstrap, the Button component by default has type set to button. Without such a default, the type will default to submit within forms, yielding the following warning when using e.g. Formik:

You submitted a Formik form using a button with an unspecified type attribute. Most browsers default button elements to type="submit". If this is not a submit button, please add type="button".

As it is normal to add multiple buttons within a form, it seems safer and more intuitive to have a default type set to button, and then explicitly specify which button to use for submission.

Steps to reproduce

  1. Create page with the following form:
<form>
<Button type="submit">Submit</Button>
<Button>Recalculate<Button>
</form>
  1. The second button will default to type=submit instead of type=button. Expected behavior would be that the second Button type would default to button.

Version

v35.2.0

Browser

Firefox

@pveierland pveierland added the bug Something isn't working label Jan 24, 2023
@iansan5653
Copy link
Contributor

iansan5653 commented Jan 24, 2023

👍 I completely agree - setting the default type to button seems like a no-risk change to avoid some very unintuitive browser behavior. Another common case where we see this turn into bugs is with "Cancel" buttons, ie:

<form onSubmit={submitHandler}>
  {/* ... */}

  <Button onClick={cancelHandler}>Cancel</Button>
  <Button type="submit" variant="primary">Primary</Button>
</form>

Pressing "Cancel" in this form will unintentionally submit the form because the button doesn't have type="button".

In fact this is so unexpected that we've considered using a linting rule to force all buttons in Memex to have explicit type props.

@joshblack joshblack added the component: Button Issues related to the Button component label Jan 24, 2023
@joshblack
Copy link
Member

Opened up a quick PR here: #2807 to put it on Button itself. Curious if you all would expect it to be there or potentially at a lower-level like ButtonBase for other button-like components that might use it 👀

@pveierland
Copy link
Author

I'd vote for adding it to ButtonBase, as otherwise the issue would still affect LinkButton, and could sneak up with future Button variants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component: Button Issues related to the Button component react
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants