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

react-hook-form button is not disabled for long enough #7

Open
xdivby0 opened this issue Feb 4, 2022 · 4 comments
Open

react-hook-form button is not disabled for long enough #7

xdivby0 opened this issue Feb 4, 2022 · 4 comments

Comments

@xdivby0
Copy link

xdivby0 commented Feb 4, 2022

What do you want and why?

I am using react-hook-form and the new.tsx pages that get generated always have this part:
<button type="submit" disabled={ctx.formState.isSubmitting}>

This means that after the form has been submitted successfully and isSubmitting is false again, we can double submit the form before the route changes (especially on slower connections) and create a second new entity by accident.

I'd like to do something like disabled={ctx.formState.isSubmitSuccessful || ctx.formState.isSubmitting} to prevent this accidental double submit.

Possible implementation(s)

Adding ctx.formState.isSubmitSuccessful should be possible pretty easy as documented in react-hook-form formState documentation.

@beerose
Copy link
Contributor

beerose commented Feb 4, 2022

Hi @xdivby0!

While I got the problem here, I don't think we can change it by providing disabled={ctx.formState.isSubmitSuccessful || ctx.formState.isSubmitting} by default. My reasoning is that sometimes, you do want the button enabled right after the click. Let's say you have a voting component and a button to add votes (something like Medium does). In that case you need to make sure users can click as much as they want.

Having said that, I think that modifying the form to fit particular use cases is something that should be done by the end users — after creating a new Blitz app, so that everyone gets the very basic thing, and then they can modify it depending on their needs.

Let me know what you think.

@xdivby0
Copy link
Author

xdivby0 commented Feb 4, 2022

@beerose I think this is a valid point for some use cases. Correct me if I'm wrong, but I see three reasons that speak against that:

  1. using the new.tsx for something like votes that are not inherently "new" objects should be the minor case in which modifications like removing the isSubmitSuccessful would be the thing people want, right?
  2. If I'm wrong in 1., then we need to remove the auto-redirect-after-form-submit by default too because of the same reason (the voting example would not work that way either).
  3. Seeing and removing the stuff you don't need (in this case the isSubmitSuccessful) is always easier than finding out what you need to add.

Does that make sense?

@beerose
Copy link
Contributor

beerose commented Feb 7, 2022

Hmm that makes sense, but what about the form in edit mode? Right now the forms are used for both creating and editing, while with edit users should be able to edit multiple times.

I don't have very strong opinions here, but want to make sure we won't worsen the experience for anyone.

@xdivby0
Copy link
Author

xdivby0 commented Feb 7, 2022

If I'm not wrong, currently edit forms as well as new forms both redirect after submit and I think this is the default people want (set / create something once). If you think more people want the edit to be used multiple times, you'd have to remove the redirect.

You are totally right with that new.tsx and edit.tsx share the same form component - and I think it should stay that way. If (as stated in the paragraph above) the behavior should be different for the edit and new version, maybe add a prop multiSubmit which can control whether the form is able to multiple times?

For me, there are 2 good choices now (which need further discussion or opinions):

  • make new.tsx and edit.tsx both redirect and both block the button longer
  • make new.tsx redirect and block the button longer, edit.tsx does not redirect postsubmit and does only block during the network request (this would need some sort of prop passing)

Leaving it in this inconsistent state is weird. It's inconsistent that the redirect then would only happen on new.tsx, but not edit.tsx, although both of them do allow a re-submit / multisubmit (concerning the button).

@dillondotzip dillondotzip transferred this issue from blitz-js/blitz Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants