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

v3: no longer possible to know whether a form has been submitted #889

Closed
macrozone opened this issue Mar 2, 2021 · 18 comments · Fixed by #906
Closed

v3: no longer possible to know whether a form has been submitted #889

macrozone opened this issue Mar 2, 2021 · 18 comments · Fixed by #906
Assignees
Labels
Type: Feature New features and feature requests
Milestone

Comments

@macrozone
Copy link
Contributor

we validate form fields on the fly, that means we show an error when a field has been filled and somethign is wrong

we achieved this with using validate="onChange", but only show an error if the field was either touched or changed.
We also showed all errors when the user tried to submit the form.

we set touched=true when either the field was blurred or when the form was "submitting".

Problem is now that submitting is never true, so that does not work anymore (we also needed to useForm().submitting)

is there a way to find out if the form has ever been submitted?

@macrozone
Copy link
Contributor Author

ok i found a fix:

on the ValidatedForm there is state.validate which is true when the form has been submitted but failed the validation.

While the name is maybe a bit missleading, this is exactly what i need

i prepared a small PR #890

@radekmie radekmie self-assigned this Mar 3, 2021
@radekmie radekmie added the Type: Feature New features and feature requests label Mar 3, 2021
@radekmie radekmie added this to Needs triage in Open Source (migrated) via automation Mar 3, 2021
@radekmie
Copy link
Contributor

radekmie commented Mar 3, 2021

Hi @macrozone. I'll comment on both your request and #890 here. There is no way to know whether the form was submitted in general. I see that you found the validate flag of the ValidatedForm. Indeed, it does exactly that, but it's there just to make onChangeAfterSubmit work. I think it makes sense to move this functionality up to the BaseForm and make it part of the public API. Would you like to work on it?

EDIT: Also, the title says no longer - I'm pretty sure this information was never part of the API - could you explain what you meant?

@radekmie radekmie moved this from Needs triage to To do in Open Source (migrated) Mar 3, 2021
@radekmie radekmie mentioned this issue Mar 3, 2021
@macrozone
Copy link
Contributor Author

Hi @macrozone. I'll comment on both your request and #890 here. There is no way to know whether the form was submitted in general. I see that you found the validate flag of the ValidatedForm. Indeed, it does exactly that, but it's there just to make onChangeAfterSubmit work. I think it makes sense to move this functionality up to the BaseForm and make it part of the public API. Would you like to work on it?

What do you suggest as property? It's meaning i "form has been tried to be submitted, but validation failed or validation is not yet done". Something like a "submitCount" could maybe also be used for that, paired with "submitSuccessCount" to highlight that "submitCount" also counts unsuccessful submits (where validation failed)

EDIT: Also, the title says no longer - I'm pretty sure this information was never part of the API - could you explain what you meant?

previously, you could know whether the form was submitted, because the fields would receive submitting: true for a short time (before validation). So you could update a state on the fields that this form had been tried to submit. I understand that this was probably not a clean solution to begin with, but it worked without problems.

@radekmie
Copy link
Contributor

radekmie commented Mar 5, 2021

Let's start with this flag meaning. You've said, "form has been tried to be submitted, but validation failed or validation is not yet done". I fully agree with the first part ("form has been tried to be submitted") and not with the rest of it. I believe you assume that all forms are closed or reset after submission - then it's true but it's not always the case. Also, BaseForm and QuickForm have no concept of validation, therefore it does not make sense.

I think that submitted makes sense and is a good name. It does not imply that it succeeded or finished - user submitted the form as in pressed submit. I'm open for suggestions though.

I have also thought about a counter instead of a flag, but I see no use case for it. And I find the "counter of unsuccessful submits" quite artificial.

And about the workaround in v2. Now I get it. And I agree it was not clear at all. Thanks for the explaination.

@hmvp
Copy link

hmvp commented Mar 5, 2021

In our case we have a submitting state value in the parent component that gets toggled in the onSubmit handler before and after handling the submit..

Something like:

    submit = async (userInput: any = {}) => {
        const { submitting } = this.state;

        if (!submitting) {
            this.setState({ submitting: true });

            try {
                await this.props.onSubmit(userInput);
                this.setState({ submitting: false });
            } catch (error) {
                this.setState({ submitting: false });
            }
        }
    }

@radekmie
Copy link
Contributor

radekmie commented Mar 5, 2021

@hmvp: since #449 the submitting property is available in the form context - you could use it instead of tracking it manually.

@macrozone
Copy link
Contributor Author

@radekmie that does not work, as this submitting property is never true when validation is sync.

@radekmie
Copy link
Contributor

radekmie commented Mar 5, 2021

@macrozone: based on the signature of the submit function I believe it's a onSubmit handler. And as such it's called only if the form is valid.

@macrozone
Copy link
Contributor Author

@macrozone: based on the signature of the submit function I believe it's a onSubmit handler. And as such it's called only if the form is valid.

we used submitting in v2, that was true for a brief moment (tick) even when the form was submitted, but before validation.

in @hmvp 's implementation submitting will also never be true for a sync onSubmit function. Its probably not good to rely on something that is true for a short time, its better to have something persistent, like a counter or so

@radekmie
Copy link
Contributor

radekmie commented Mar 9, 2021

@macrozone: will you continue working on #890? If not, please do close it - we'll pick it up from here.

@macrozone
Copy link
Contributor Author

@radekmie currently i solved in userspace by extending ValidedForm:

import { Context, ValidatedForm } from "uniforms";
import BaseForm from "./BaseForm";

const Validated = (parent) => {
  // tslint:disable-next-line:class-name
  class _ extends ValidatedForm.Validated(parent) {
    public static Validated = Validated;

    getContext(): Context<unknown> {
      return {
        ...super.getContext(),
        // see https://github.com/vazco/uniforms/issues/889
        validate: this.state.validate,
      };
    }
  }
  return _;
};

export default Validated(BaseForm);

this works, but i validate is not a good name for a general solution and submitting is already taken. I think submitting would be a good name, but the problem is, that its not working if your validation is sync and even if its async, its just true for a short time.

for my case its enough to know whether submitting has been tried at least once, in principle thats the same condition which makes validate="onChangeAfterSubmit" to validate and show errors in the form, so only ValidatedForm would have this state and not the base form

@radekmie
Copy link
Contributor

radekmie commented Mar 9, 2021

Have you read #889 (comment)? I suggested a submitted flag in the BaseForm.

@macrozone
Copy link
Contributor Author

submitted

submitted is wrong i think. I would assume that "submitted" would only be true if onSubmt was (successfully) called.

But i need a flag that is true when the form has been tried to be submitted, but independent whether validation or onSubmit was successful.

@radekmie
Copy link
Contributor

radekmie commented Mar 9, 2021

I understand. As I already wrote:

[...] It does not imply that it succeeded or finished - user submitted the form as in pressed submit. I'm open for suggestions though.

If you have a suggestion, then let me know,

@macrozone
Copy link
Contributor Author

I understand. As I already wrote:

[...] It does not imply that it succeeded or finished - user submitted the form as in pressed submit. I'm open for suggestions though.

If you have a suggestion, then let me know,

alright, that is also valid point of view i think. So let's go with that, i'll update my pr then

@radekmie radekmie moved this from To do to In progress in Open Source (migrated) Mar 10, 2021
@radekmie
Copy link
Contributor

@macrozone: will you continue this PR? If not, please do close it and we'll take it from here.

@macrozone
Copy link
Contributor Author

@radekmie i'll do it now

@macrozone
Copy link
Contributor Author

macrozone commented Mar 17, 2021

@radekmie see PR #906

Open Source (migrated) automation moved this from In progress to Closed Mar 22, 2021
@radekmie radekmie added this to the v3.3 milestone Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants