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

fix: Prevent submitting a form before saving an input into the store #1030

Merged
merged 11 commits into from
Sep 1, 2021
Merged

fix: Prevent submitting a form before saving an input into the store #1030

merged 11 commits into from
Sep 1, 2021

Conversation

pboivin
Copy link
Contributor

@pboivin pboivin commented Jul 1, 2021

Description

To summarize the issue: In the create modal as well as in the main form view, when a user types into an input field and very quickly presses enter or clicks the 'Update' button, the submit event is handled before the updated value makes its way into the Vuex store.

I'm not 100% sure this is the right approach, so I've limited this to TextField for now. This could easily be extended to WYSIWYG and other fields that may be affected by this issue.

Related Issues

#1029 (comment)

#777 (comment)

@ifox
Copy link
Member

ifox commented Jul 2, 2021

Great stuff @pboi20! I haven't tested this yet, however reviewing the code I'm asking myself what happens for the end user. Should the submit button be disabled when preventSubmit is true?

@pboivin
Copy link
Contributor Author

pboivin commented Jul 2, 2021

Hey @ifox, good point!

At the moment, there is no visual feedback as part of this implementation. TextField has a debounce of 250ms before updating the store, so submit is prevented/ignored from the first keypress until 250ms after the last. Here's a quick test, toggling the opacity of the button as visual feedback :

Peek 2021-07-02 10-47

Curious to know your first impression! The WYSIWYG fields have a slower debounce time (600ms), which would cause less "flashing".

@pboivin
Copy link
Contributor Author

pboivin commented Jul 2, 2021

Another option would be to "defer" the request to submit until the store has been updated, instead of simply ignoring it. I believe this would remove the need for any visual feedback on the disabled state.

@antonioribeiro
Copy link
Member

I personally like the "defer" better than the visual feedback.

@ifox
Copy link
Member

ifox commented Jul 2, 2021

Another option would be to "defer" the request to submit until the store has been updated, instead of simply ignoring it. I believe this would remove the need for any visual feedback on the disabled state.

I think that might be a better option, I like it! Thanks for prototyping the visual feedback idea, I'm afraid the flashing isn't working.

@pboivin
Copy link
Contributor Author

pboivin commented Jul 5, 2021

Hey @ifox and @antonioribeiro,

I've got a first draft of the deferred/delayed submit feature. The API is relatively simple:

  • fields can call preventSubmit and allowSubmit
  • forms can check isSubmitPrevented from the store and act accordingly

A retrySubmit mixin was extracted for the feature that is common to main-form, ModalCreate and ModalUpdate.

Leaving this as a draft for now to take some time to test more extensively. Curious to know what you think!

@ifox
Copy link
Member

ifox commented Jul 5, 2021

This looks great @pboi20! I will give it a try.

@pboivin pboivin changed the title Prevent submitting a form before saving an input into the store fix: Prevent submitting a form before saving an input into the store Aug 19, 2021
@pboivin
Copy link
Contributor Author

pboivin commented Aug 19, 2021

Update:

  • rebased on 2.x
  • updated both WYSIWYG fields

Demo:

Peek 2021-08-19 11-27

@pboivin pboivin marked this pull request as ready for review August 19, 2021 15:34
@ifox ifox merged commit 8331989 into area17:2.x Sep 1, 2021
@pboivin pboivin deleted the fix/prevent-submit-before-store-update branch September 1, 2021 11:51
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.

3 participants