prometheusresearch-archive / react-forms

Forms library for React.
1.16k stars 112 forks source link

schema aware update #116

Closed bsr203 closed 8 years ago

bsr203 commented 8 years ago

I have some issue with non text input field. It looks like the value returns in the change callback of an input field is string even if the input type is number and the initial value set is number.

In this spec, it says,

value = floating-point number
A string representing a number.

I guess, that may be the reason the value is string even for non text field!

onChange handler of field, just calls the formValue.update with the string value, and Value.update simply updates the field to callback value. This caused the validation to fail. A solution could be to update check the type of the field from schema and do necessary typecast.

andreypopp commented 8 years ago

Hmm... It is up to input component to return correct value type. Do you use just <input type="number" />? I'm not sure if it returns numbers and not strings.

bsr203 commented 8 years ago

hi @andreypopp thanks for the super quick response :-)

I have a small wrapper around input, but the rendered html is

<input type="number" class="form-control" value="123" data-reactid=".0.1.0.0.0.0.0.1.0.$integerType.1.2">

I guess, input, irrespective of type is retuning string. I solved by a custom input like.

const Number = ({onChange, ...rest}) => {
  function onNumberChange(value) {
    onChange(parseInt(value, 10));
  }
  return <Input onChange={onNumberChange} {...rest} />;
};

you can close this if the current behavior is better and don't wan't to consider special cases in value update.

bsr203 commented 8 years ago

Also, type=checkbox didn't work. had to define a custom component like

import React, {PropTypes} from 'react';

export default function Checkbox({value, onChange, ...rest}) {
  function onCheckedChange(e) {
    const checked = e.target.checked;
    onChange(checked);
  }

  return (
    <input {...rest}
      type="checkbox"
      onChange={onCheckedChange}
      checked={value}
    />
  );
}

Checkbox.defaultProps = {
  value: PropTypes.bool.isRequired,
  onChange: PropTypes.func.isRequired,
};

I guess the way value is set is different for checkbox as explained here https://facebook.github.io/react/docs/forms.html#why-controlled-components

I know older versions of this library provided something similar.

andreypopp commented 8 years ago

I know older versions of this library provided something similar.

Yes, and the older version was too much opinionated sadly. The current version just does one thing — data flow for form like structures.

It is possible though to make components on top of react-forms which render different inputs for different schema types (or use input component which can be defined on schema nodes).

I'll close this issue as it's out of scope of React Forms. Feel free to continue discussion though.

bsr203 commented 8 years ago

thanks Andrey. I provided above two cases as examples, and they work fantastic. It is quite trivial and takes very little code, so I am happy the way it is now.

But, would be useful to mention it in the doc, if you too observe that, built-in Input component is only suitable for input type="text". Cheers.

andreypopp commented 8 years ago

Sure, I'd like to use them as a basis for examples in docs, if you approve.

bsr203 commented 8 years ago

Sure deal. you saved tons of time for us and let ignore form complexities :-). Cheers.