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

Track "submitting" state and make it available in the context #441

Closed
BudgieInWA opened this issue Jun 26, 2018 · 2 comments · Fixed by #449
Closed

Track "submitting" state and make it available in the context #441

BudgieInWA opened this issue Jun 26, 2018 · 2 comments · Fixed by #449
Assignees
Labels
Type: Feature New features and feature requests

Comments

@BudgieInWA
Copy link
Contributor

Any time I use a form with an async onSubmit, I find myself wanting to disable the submit button while a submit is in progress so that the user sees something happening. Below is a form subclass that allows my SubmitFields to react to this state.

const Stateful = parent => class extends parent {
  static Stateful = Stateful;
  static displayName = `Stateful${parent.displayName}`;

  static childContextTypes = {
    ...parent.childContextTypes,

    uniformsExtra: propTypes.shape({
      submitting: propTypes.bool.isRequired,
      validating: propTypes.bool.isRequired,
    }).isRequired,
  };

  constructor() {
    super(...arguments);

    this.state = {
      ...this.state,

      submitting: false,
      validating: false,
    };

    this.onSubmit = this.onSubmit.bind(this);
    this.onValidateModel = this.onValidateModel.bind(this);
  }

  getChildContext() {
    return {
      ...super.getChildContext(),

      uniformsExtra: {
        submitting: this.state.submitting,
        validating: this.state.validating,
      }
    }
  }

  onSubmit(...args) {
    this.setState({ submitting: true });
    return super.onSubmit(...args).then(
      () => this.setState({ submitting: false }),
      () => this.setState({ submitting: false })
    );
  }

  onValidateModel(...args) {
    this.setState({ validating: true });
    return super.onValidateModel(...args).then(
      () => this.setState({ validating: false }),
      () => this.setState({ validating: false })
    )
  }
};

It seems to me that the submitting state is such a natural fit in BaseForm that I was surprised not to find it there to begin with. This is because BaseForm

  • deals with the onSubmit promise lifecycle with onSubmitSuccess/Failure, as well as
  • makes similar form state such as context.state.changed available.

I think that there is great value in all uniforms providing a rich and complete picture of the state of the form as the user interacts with them, so that it is natural and easy to implement forms that respond to all of their states.

I propose that this extra piece of state be tracked by default in BaseForm and made available as context.state.submitting (similarly ValidatedForm tracking the async validating). I might even go as far as making the built in SubmitFields disabled while submitting.

If this fits with the direction uniforms is going, I am happy to contribute to the implementation of this feature.

@radekmie radekmie self-assigned this Jun 26, 2018
@radekmie radekmie added the Type: Feature New features and feature requests label Jun 26, 2018
@radekmie
Copy link
Contributor

Hi @BudgieInWA. I've already implemented such submitting and validating flags few times (in different projects) and it might be worthy to do it in the core. As always, I'm a bit concerned about it, because most of the time we choose the small core way (which, as you can see, is not limiting us at all).

Also, your code has few flaws. For example, onSubmit and onValidateModel are both swallowing the rejections. If it's going to be in the core, it has to be tested very thoroughly.

Let's try! I definitely can help you, if you'll need that.

@BudgieInWA
Copy link
Contributor Author

I appreciate the desire to keep core small and robust. I think that if this feature is part of the base implementation the overhead van be kept quite low. I will start on an implementation and we can see if that proves true.

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.

2 participants