Skip to content
This repository has been archived by the owner on Aug 23, 2022. It is now read-only.

FormReducer touched and untouched property don't reflect aggregated touched properties of children Fields #44

Closed
lasergoat opened this issue Mar 3, 2016 · 3 comments
Labels

Comments

@lasergoat
Copy link
Contributor

The result of createFormReducer has properties like touched, valid, pristine etc.

Some of these properties need to change based on Fields within that Form. For instance, if I touch the email field below, then newUserForm.touched should be true, yet it stays false.

<form className="ui-form -middle">

  <Field className="ui-field " model="newUser.email" validators={{
    valid: validator.isEmail,
    required : (v) => !!v
  }}>
    <label className="ui-label">Email</label>
    <input type="text" className="ui-input" />
    {!getField(newUserForm, 'email').valid && getField(newUserForm, 'email').touched && (
      <div className="ui-error">
        Please check your email address
      </div>
    )}
  </Field>

  <button type="button" className="ui-button"
    disabled={ !newUserForm.valid }
    onClick={(e) => dispatch(saveUser(newUser))}>
    Submit
  </button>

</form>

I want the submit button to be enabled under the condition that the form is touched and the fields are valid. So it would be disabled if the form isn't touched or one or more of the fields are invalid.

So, if I changed the submit button disabled attribute to:

{ !newUserForm.valid || !newUserForm.touched }

The disabled property would be wrong because .touched and .untouched don't update. They need to change if any one field within the form changed.

Semantically the reason is this: "If I touch a field within the form, I have touched the form."

@lasergoat
Copy link
Contributor Author

One possible solution is this:

form-reducer.js:130


      case actionTypes.SET_TOUCHED:
        let formIsTouched;
        let setTouchedState;

        if (!localPath.length) {
          formIsTouched = true;

          setTouchedState = icepick.merge(state, {
            fields: mapValues(state.fields, field => ({
              ...field,
              dirty: false,
              touched: true,
            })),
          });
        } else {
          setTouchedState = setField(state, localPath, {
            dirty: false,
            touched: true,
          });

          formIsTouched = every(mapValues(setTouchedState.fields, field => field.touched));
        }

        return icepick.merge(setTouchedState, {
          dirty: !formIsTouched,
          touched: formIsTouched,
        });

      case actionTypes.BLUR:
        return setField(state, localPath, {
          blur: true,
          focus: false,
          touched: true,
          untouched: false,
        });

Virtually the same code as SET_PRISTINE

@lasergoat
Copy link
Contributor Author

This takes care of setting the form to TOUCHED. BUT it does not take care of the SET_TOUCHED ability. Imagine this:

a user touched a field and then the code calls SET_UNTOUCHED. The form was touched so we need to check if other children were touched, if so the form stays touched but if nothing else was touched, then the form becomes untouched. PRISTINE implements this logic, but touched doesn't.

This PR does fix the issue at hand though.

@lasergoat lasergoat mentioned this issue Mar 3, 2016
@davidkpiano
Copy link
Owner

Thanks @lasergoat ! Fixed in your PR: #45

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants