gcanti / tcomb-form

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

Add setState callback to onChange method. #179

Closed istarkov closed 9 years ago

istarkov commented 9 years ago

When I write my custom components, and use locals.onChange I need to know when the state has changed. This callback solves this problem.

gcanti commented 9 years ago

Hi, Thanks for your PR.

When I write my custom components, and use locals.onChange I need to know when the state has changed.

This is interesting, in theory when the state changes should trigger a re-rendering, but supposedly you are consuming that callback in a render() method. Please could you show me a use case where you need such a callback?

istarkov commented 9 years ago

I write custom components as like you in your templates, for simplicity overriding semantic template like

// base is original textbox template from lib/templates/semantic
export default function textbox(base) {
  return (locals) => {
    if (locals.type !== 'info') {
      // base is original textbox template from lib/templates/semantic
      const ctrlBase = base(locals);
      // override base 
      return {
        ...ctrlBase,
        children: ctrlBase.children.map(child => child && child.tag === 'input'
          ? wrapInput(child, locals)
          : child),
      };
    }

and i have no access here to Form component (and have no access to render or other Form methods), but for some controls i need additional events in Form which must occures strongly after onChange done (done means state already changed).

gcanti commented 9 years ago

but for some controls i need additional events in Form which must occures strongly after onChange done

Ok but without an example it seems like a very specific case to me, couldn't it be solved with a custom factory?

class MyTextbox extends t.form.Textbox {

  onChange(value, callback) {
    this.setState({value}, () => {
      this.props.onChange(value, this.props.ctx.path);
      if (callback && typeof callback === 'function') {
        callback(value, this.props.ctx.path);
      }
    });
  }

  getTemplate() {
    return (locals) => {
      var onChange = locals.onChange;
      locals.onChange = (value) => {
        onChange(value, () => console.log('ok'));
      };
      return super.getTemplate()(locals);
    };
  }

}

Usage

var Person = t.struct({
  name: t.Str
});

var options = {
  fields: {
    name: {
      factory: MyTextbox
    }
  }
};

const App = React.createClass({

  onSubmit(evt) {
    evt.preventDefault();
    var value = this.refs.form.getValue();
    if (value) {
      console.log(value);
    }
  },

  render() {
    return (
      <form onSubmit={this.onSubmit}>
        <t.form.Form
          ref="form"
          type={Person}
          options={options}
        />
        <button className="btn btn-primary">Save</button>
      </form>
    );
  }

});
istarkov commented 9 years ago

This is an example, i've gave it

// base is original textbox template from lib/templates/semantic
export default function textbox(base) {
  return (locals) => {
    if (locals.type !== 'info') {
      // base is original textbox template from lib/templates/semantic
      const ctrlBase = base(locals);
      // override base 
      return {
        ...ctrlBase,
        children: ctrlBase.children.map(child => child && child.tag === 'input'
          ? wrapInput(child, locals)
          : child),
      };
    }

is it clear?

and if i call somewhere locals.onChange(value ... it's need to have callback.

gcanti commented 9 years ago

Hi @istarkov, your example is clear but it's just a wrapper around the original textbox template. What is not clear to me is why you need a callback in a render method: since it will be called again when the state changes I can't see how it would be reliable having an handler there. I'd love to see a use case for which you need a callback in the render method, it seems weird and not reliable but maybe I'm missing something...

istarkov commented 9 years ago

For example the problem i have. I need to save form when component has changed - but not on onChange event, for input it is onBlur. So I pass onChanged callback via attrs and call it on onBlur event in input occurs, but for some types of my custom components, blur and change occurs immediately and if i have a code like this:

  return (locals) => {
    const attrs = {
      ...someAttrs,
      onChange: value => {
        locals.onChange(value);
        if (locals.attrs.onChanged) {
            locals.attrs.onChanged(value);
        }
      },
    }
    ...

I get onChanged event callback called in form before locals.onChange changes the state, and this can be easily be avoided if i have callback and do it like

  return (locals) => {
    const attrs = {
      ...someAttr,
      onChange: value => {
        locals.onChange(value, () => {
          if (locals.attrs.onChanged) {
            locals.attrs.onChanged(value);
          }
        });
      },
    };

(here onChange is my custom Component change)

PS: I forgot to say that such situation occurs if I call onChange inside React.addons.batchedUpdates, so setState works asynchronously, and does not update state immediately.

istarkov commented 9 years ago

I found workaround which works for react 0.13 0.14 to be sure that setState is not called asynchronously, I moved onChange calls to setTimeout. so updates not batched.

return (locals) => {
    const attrs = {
      ...someAttrs,
      onChange: value => {
        locals.onChange(value);
        if (locals.attrs.onChanged) {
            locals.attrs.onChanged(value);
        }
      },
    }
    ...
return (locals) => {
    const attrs = {
      ...someAttrs,
      onChange: value => setTimeout(() => {
        locals.onChange(value);
        if (locals.attrs.onChanged) {
            locals.attrs.onChanged(value);
        }
      }, 0)
    }
    ...

and this is enough for me to know that state already changed.

But this code is not clean as like as with callback.