2
\$\begingroup\$

I have been working of a basic from using React and I need your help to improve this part of my code, especially validate and onSubmit.

state = {
    email: "",
    emailError: "",
  }; 
  change = e => {
    this.setState({
      [e.target.name]: e.target.value
    });
  };

    validate = () => {
    let isError = false;
     const errors = {
      emailError: "",
    };

    if (this.state.email.indexOf("@") === -1) {
      isError = true;
      errors.emailError = "Requires valid email";
    }

    this.setState({
      ...this.state,
      ...errors
    });

    return isError;
  }; 
  onSubmit = e => {
    e.preventDefault();
    const err = this.validate();
    if (!err) {
      this.props.onSubmit(this.state);
      this.setState({

        email: "",
        emailError: "",
      });
    }
  };

  render() {
    return (
      <form>
        <TextField
          name="email"
          hintText="Email"
          floatingLabelText="Email"
          value={this.state.email}
          onChange={e => this.change(e)}
          errorText={this.state.emailError}
          floatingLabelFixed
        />
        <br />
        <RaisedButton label="Submit" onClick={e => this.onSubmit(e)} primary />
      </form>
    );
\$\endgroup\$
2
\$\begingroup\$

As far as I'm aware, CodeReview is for completed programs. I imagine that this question would be better suited in StackOverflow. That being said, I see no point in duping the question on a different site.

Without seeing the rest of the program, and solely in a time interest, I'd say you keep the change function and ignore the validations. If you want a form field to be required, you can add 'required' to the input.

<form>
  <input type="text" name="input1" value={this.state.input1} onChange={e => this.change(e)} required />
</form>

You can find more on the MDN web docs for the required attribute.

Side note: you can also bind this.change as this.change = this.change.bind(this) in the constructor if you intend on using it for multiple fields. It'll save you some bytes, but if it's just a one off, binding it in the element itself like you have should be fine.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.