gcanti / tcomb-form

Forms library for react
https://gcanti.github.io/tcomb-form
MIT License
1.16k stars 136 forks source link

Slow loading of client-side JS means data entered in form is not recorded #304

Closed rnewstead1 closed 8 years ago

rnewstead1 commented 8 years ago

I am using

I am using iso v4.2.0 to build an isomorphic react application using tcomb-form. I am having problems with submitting the form in situations where the client-side JS is slow to load.

If the user has a slow connection, the client-side JS might be loaded halfway through the user filling in the form. Any fields that are filled in before the client-side code is loaded do not cause this.state.value to be updated.

When the user submits the form, the validation fails.

Modifying these fields and re-submitting the form works.

Am I doing something wrong here? Is there some way I can call onChange when the client JS loads with any values that have already been entered?

JS for the client

import React from 'react';
import Iso from 'iso';
import ReactDOM from 'react-dom';
import ApplicationForm from '../../../app/ui/components/ApplicationForm';

process.env.BROWSER = true;
Iso.bootstrap((state, meta, container) => {
  const applicationFormApp = React.createElement(ApplicationForm, state);

  // setTimeout used here to simulate a slow connection, where the client-side JS might be
  // loaded halfway through the user filling in the form.
  setTimeout(() => {
    console.log('Client-side form rendered');
    ReactDOM.render(applicationFormApp, container, meta.iso);
  }, 3000);
}, {
  markupClassName: '__iso__markup__application__form',
  dataClassName: '__iso__state__application__form',
});

ApplicationForm.jsx

import React, { Component, PropTypes } from 'react';
import t from 'tcomb-form';
import transform from 'tcomb-json-schema';

const Form = t.form.Form;
class ApplicationForm extends Component {

  constructor(props) {
    super(props);
    this.onChange = this.onChange.bind(this);
  }

  state = {
    value: null,
    schema: null,
    options: null,
  };

  componentWillMount() {
    this.state.schema = transform(this.props.schema);
    this.state.options = this.props.options;
  }

  onSubmit(e) {
    e.preventDefault();

    const value = this.refs.form.getValue();

    if (value) {
      console.log(`submitted value: `, value);
    } else {
      console.log('Validation errors.');
    }
  }

  onChange(value, path) {
    this.setState({ value: value });
  }

  render() {
    return (
      <div className="form-component">
        <Form ref="form" type={this.state.schema} options={this.state.options}
              value={this.state.value}
              onChange={this.onChange}/>

        <div className="form-group">
          <button onClick={(e) => this.onSubmit(e)} className="btn btn-primary">Save</button>
        </div>
      </div>
    );
  }
}

export default ApplicationForm;
gcanti commented 8 years ago

Hi @rnewstead1,

It's tricky to fix that behaviuor. Every input is a controlled component (i.e. owns a value and onChange props) so in general tcomb-form doesn't know how to retrieve the values contained in its fields.

As a workaround you could:

gcanti commented 8 years ago

retrieve manually the values contained in the form

This is a proof of concept:

// main function
function retrieveValue(component) {
  const type = component.typeInfo.innerType
  switch (type.meta.kind) {
  case 'struct' :
    const value = {}
    for (const prop in type.meta.props) {
      if (type.meta.props.hasOwnProperty(prop)) {
        value[prop] = retrieveValue(component.refs[prop])
      }
    }
    return value
  case 'list' :
    return []
  default :
    // retrieving the correct value depends on the template used to render the input
    const getTemplateValue = component.getTemplate().retrieveValue
    if (getTemplateValue) {
      const parse = component.getTransformer().parse
      return parse(getTemplateValue(component))
    }
    return null
  }
}

// configuring the templates...

t.form.Form.templates.textbox.retrieveValue = component => {
  const node = document.getElementById(component.getId())
  return node ? node.value : null
}

t.form.Form.templates.checkbox.retrieveValue = component => {
  const node = document.getElementById(component.getId())
  return node ? node.checked : false
}

t.form.Form.templates.select.retrieveValue = component => {
  const node = document.getElementById(component.getId())
  return node ? node.value : null
}

t.form.Form.templates.radio.retrieveValue = component => {
  const type = component.typeInfo.innerType
  const len = Object.keys(type.meta.map).length
  for (let i = 0; i < len; i++) {
    const node = document.getElementById(component.getId() + '_' + i)
    if (node.checked) {
      return node.value
    }
  }
}

// ...other templates...

Usage:

const App = React.createClass({

  retrieveValue() {
    console.log(retrieveValue(this.refs.form.getComponent([])))
  },

  render() {
    return (
      <form onSubmit={this.onSubmit}>
        <t.form.Form
          ref="form"
          type={Type}
          options={options}
        />
        <div className="form-group">
          <button type="submit" className="btn btn-primary">Save</button>
          <button type="button" onClick={this.retrieveValue} className="btn btn-primary">Retrieve value</button>
        </div>
      </form>
    )
  }

})
rnewstead1 commented 8 years ago

Wow, thanks @gcanti!

The first option (disabling the form till the client JS is loaded) seems like pretty bad UX, although I noticed that it seemed to be an acceptable answer from a similar question on react-forms.

Thanks for the proof of concept, I didn’t really know where to start with doing this! I will probably need to call retrieveValue() in componentDidMount and set this.state.value as in your second option because I want to be able to save data that has been entered before the form is submitted. I will use this as a starting point.

gcanti commented 8 years ago

The first option seems like pretty bad UX

Agree. It's simple but kind of suboptimal.

Thanks for the proof of concept

After a few tests seems to work fine. However Date handling is missing (should be quite easy). More difficult is to handle lists:

...
case 'list' :
    return []
...    

This is ok only if you don't have lists in your form or if you have lists but you don't prefill the form on the server.

I will use this as a starting point.

Let me know if you need more help.

Cheers, Giulio