shakacode / re-formality

Form validation tool for reason-react
https://re-formality.now.sh
MIT License
244 stars 36 forks source link

Form being submitted even when validations are failing #77

Closed shubhamgupta731 closed 4 years ago

shubhamgupta731 commented 4 years ago

I have created the following form component:

module SearchUserForm = [%form
  type input = {
    mobile: string
  };
  let validators = {
    mobile: {
      strategy: OnSubmit,
      validate: (input) => {
        Js.log("validate called")
        switch (input.mobile) {
        | "" => Belt.Result.Error("Enter the Valid Mobile Number")
        | _ =>
          switch (ValidationHelper.MobileNumber.isValid(input.mobile)) {
          | false => Belt.Result.Error("Enter the Valid Mobile Number")
          | true => Belt.Result.Ok(input.mobile)
          }
        }
      }
    }
  }
]

[@react.component]
let make = (~caller_number: option(string), ~get_enterprise_user_info: string => unit) => {
  let form =
  SearchUserForm.useForm(
    ~initialInput={mobile: Belt.Option.getWithDefault(caller_number, "")},
    ~onSubmit=(output, _) => {
      Js.log("submit called " ++ output.mobile);
      get_enterprise_user_info(output.mobile);
    },
  );

  <form
    className="enterprise-user-search-container"
    onSubmit={(_) => {
      form.submit();
    }}>
    <p className="enterprise-user-search-heading"> {React.string("Fetch Caller Details")} </p>
    <div className="input-container">
      <FormInputV3
        label="Enter Caller Number"
        inputClassName="search-input-box"
        value={FormInputV3.String(form.input.mobile)}
        isDisabled=false
        fieldResult=form.mobileResult
        onChange={
          form.updateMobile((~target, _) => {
            mobile: target##value
          })
        }
      />
    </div>
    <div className="separator" />
    <button className="search-button"> {React.string("Search")} </button>
  </form>;
};

However, when we submit the form (using the submit button), the form is getting submitted even when the validation for the field returns an error. However, I am able to correct the behavior if I change the onSubmit in the form to:

    onSubmit={(event) => {
      event->ReactEvent.Synthetic.preventDefault;
      form.submit();
    }}>

Is there a mistake in the way I am using the ppx?

shubhamgupta731 commented 4 years ago

I think we need to prevent the default behavior of the submission event. Dont see that happening anywhere in the source code.

alex35mil commented 4 years ago

@shubhamgupta731 Yes, you are right that default behavior should be prevented and it is mentioned in the docs. It was left out to user land for 2 reasons:

  1. ReactNative doesn't have to deal with events so it would be impossible to use submit function in ReactNative app (though in recent commits I introduced targets so it's possible to separate implementations)
  2. I assumed, people who work on fat clients using react already have similar component so they don't need change their internal interfaces b/c of one package.

Since #1 can be addressed with introduction of targets, it's possible to change this behavior unless #2 stands true for most of the users.

shubhamgupta731 commented 4 years ago

Sure, I understand. But can we update form to Form so that it becomes clear for anyone that we are talking about a custom component and provide a hyperlink to the form component that you use in your examples?

alex35mil commented 4 years ago

Yeah, we should address it one way or another. I'm not sure which way though :)

Lemme articulate the options:

  1. Handle preventDefault on submit in form.submit handler when target is ReactDom.

    <form onSubmit={event => form.submit(event)} />

    Pros: users don't need to think about it. Cons:

    • users probably handle it in their apps and it might require update of their internal APIs
    • there might be cases when we would break users who don't use form tag for whatever reason
  2. Keep it in user land and elaborate more in the docs. Pros: no changes needed except small changes to docs. Cons: possible confusion.