foxhound87 / mobx-react-form

Reactive MobX Form State Management
https://foxhound87.github.io/mobx-react-form
MIT License
1.09k stars 129 forks source link

Material UI SelectField onChange not working #218

Closed ricbermo closed 7 years ago

ricbermo commented 7 years ago

Hi, I have implemented mobx-react-form successfully in my project but now I'm trying to get a SelectField working.

This is what I have.

// MaterialSelectFieldComponent.js

import React from 'react';
import {observer} from 'mobx-react';
import SelectField from 'material-ui/SelectField';
import MenuItem from 'material-ui/MenuItem';

export default observer(({field}) => (
  <div>
    <SelectField {...field.bind()} >
      {field.options.map(({value,label}) => {
        return (<MenuItem key={value} value={value} primaryText={label} />)
      })}
    </SelectField>
  </div>
));
// bindings
  MaterialSelectField: {
    id: 'id',
    name: 'name',
    value: 'value',
    label: 'floatingLabelText',
    disabled: 'disabled',
    error: 'errorText',
    onChange: 'onChange',
  }
// fields
    a_dummy_field: {
      label: 'Nice Field',
      rules: 'required',
      bindings: 'MaterialSelectField',
      options: [{value:1, label: 'Yes'}]
    }

and <MaterialSelectField field={form.$('a_dummy_field')} />

With this the form, the field and its options are rendered, but selecting one option does not set the value in the select field. Am I missing something? Thanks in advance.

foxhound87 commented 7 years ago

I think you should reimplement the onChange handler:

signature form the material-ui docs:

function(event: object, key: number, payload: any) => void

The built-in onChange handler is a generic implementation, will not work out of the box for all components, I try to make it compatible with most of standard components implementations.

If it's the case I will make it compatible with the material-ui SelectField.

ricbermo commented 7 years ago

Ok, where should I reimplement that? On bindings? I am kinda lost now, Thank you

foxhound87 commented 7 years ago

You can implement a new bindings template or if you need a more simple implementation you can pass it to the bind() method for that specific component, in your case for the material SelectField should be:

const customOnChange = field => (e, key, val) => {
  field.set( ... ); // set field value
};

<SelectField {...field.bind({ onChange: customOnChange(field) })} >

In this article I explain both solutions

ricbermo commented 7 years ago

I was writing this comment when github view got updated. This is what I did.

// bindings

const onChange = field => (e, k, payload) => {
  field.set('value', payload); // not sure about this
  field.validate();
};

MaterialSelectField: ({ $try, field, props }) => ({
    id: $try(props.id, field.id),
    name: $try(props.name, field.name),
    value: $try(props.value, field.value),
    floatingLabelText: $try(props.label, field.label),
    hintText: $try(props.placeholder, field.placeholder),
    errorText: $try(props.error, field.error),
    disabled: $try(props.disabled, field.disabled),
    onChange: $try(props.onChange, onChange(field)),
});

Is this way correct?

Thank you so much

foxhound87 commented 7 years ago

Yes, all good. But you don't need field.validate() into onChange. The package has a validation observer already active on each field value.

field.set( ... ) is a shortcut of field.set('value', ... ). This only for 'value' of course, changing other props you must specify the name.

ricbermo commented 7 years ago

Awesome, thanks BTW: field.set(payload), in my case, is not working, what is working is field.set('value', payload).

foxhound87 commented 7 years ago

Payload is an object in your case?

ricbermo commented 7 years ago

No, it's a string, what the SelectField onChange function returns

foxhound87 commented 7 years ago

That's weird, it is tested.

/mobx-react-form--master/tests/data/forms/fixes/form.d.js:
   37      form.map((field) => { // eslint-disable-line
   38        if (field.key === 'itineraryItem') {
   39:         field.set('itinerary-item-value');
   40        }
   41      });

/mobx-react-form--master/tests/fixes.values.js:
   28  
   29  describe('$D Field values checks', () => {
   30:   it('$D itineraryItem value value should be equal to "itinerary-item-value"', () =>
   31:     expect($.$D.$('itineraryItem').value).to.be.equal('itinerary-item-value'));
   32  

if you can provide your form configuration I will test that too.

Thank You.

ricbermo commented 7 years ago

I basically copy pasted your material register from the demo repository.

export default class BaseForm extends MobxReactForm {

  plugins() {
    return {
      dvr: {
        package: validatorjs,
        extend: ($validator) => {
          $validator.useLang('es');
        }
      },
    };
  }

  options() {
    return {
      autoParseNumbers: true,
    };
  }

  bindings() {
    return bindings;
  }

  onSuccess(form) {
  }

  onError(form) {
  }
}

// form instances
class UserForm extends BaseForm {}
export default {
  usersForm: new UserForm(
    {...fields},
    {name: 'New User'}
  )
};

// bindings.
const onChange = field => (e, k, payload) => {
  field.set('value', payload);
};
export default {
  MaterialTextFieldReimplemented: ({ $try, field, props }) => ({
    type: $try(props.type, field.type),
    id: $try(props.id, field.id),
    name: $try(props.name, field.name),
    value: $try(props.value, field.value),
    floatingLabelText: $try(props.label, field.label),
    hintText: $try(props.placeholder, field.placeholder),
    errorText: $try(props.error, field.error),
    disabled: $try(props.disabled, field.disabled),
    onChange: $try(props.onChange, field.onChange),
    onFocus: $try(props.onFocus, field.onFocus),
    onBlur: $try(props.onBlur, field.onBlur)
  }),

  MaterialTextField: {
    id: 'id',
    name: 'name',
    type: 'type',
    value: 'value',
    label: 'floatingLabelText',
    placeholder: 'hintText',
    disabled: 'disabled',
    error: 'errorText',
    onChange: 'onChange',
    onFocus: 'onFocus',
    onBlur: 'onBlur'
  },

  MaterialSelectField: ({ $try, field, props }) => ({
    id: $try(props.id, field.id),
    name: $try(props.name, field.name),
    value: $try(props.value, field.value),
    floatingLabelText: $try(props.label, field.label),
    hintText: $try(props.placeholder, field.placeholder),
    errorText: $try(props.error, field.error),
    disabled: $try(props.disabled, field.disabled),
    onChange: $try(props.onChange, onChange(field))
  })
};

// the form NewUserForm
export default observer(({ form }) => (
  <MaterialSelectField field={form.$('select_field')} />
  <MaterialTextField field={form.$('text_field')} />
));

// a container where the form is used

const NewUserFormContainer = observer(
  class NewUserFormContainer extends Component {
    render() {
      return (
        <NewUserForm form={forms.usersForm} />
      );
    }
  }
);

I omitted some code like the import and export

foxhound87 commented 7 years ago

It should work, I will deepen this. Thank You so much

ricbermo commented 7 years ago

@foxhound87 payload is a number. In my fields for the User I have

    select_field: {
      label: 'Select Something',
      rules: 'required',
      bindings: 'MaterialSelectField',
      options: [{value:1, label: 'Selected Value'}]
    },
foxhound87 commented 7 years ago

That's why 👍

ricbermo commented 7 years ago

so, do I have something wrong there?

foxhound87 commented 7 years ago

No, it's a bug. I'm fixing it.

foxhound87 commented 7 years ago

Update the package and should work now!

foxhound87 commented 7 years ago

Another fix was needed because I ended up refactoring all the set() method.

If you can't update the value with field.set( ... ) use field.value = ...

ricbermo commented 7 years ago

got it, thanks.

EDIT: field.set( ... ) still not working field.value =does work but it also worked before updating the package

foxhound87 commented 7 years ago

set() not working yet? What's the input value? Show me also the form structure please, it is relevant for setting nested fields values.

ricbermo commented 7 years ago

The value is just number, like 1 which comes from here

select_field: {
  label: 'Select Something',
  rules: 'required',
  bindings: 'MaterialSelectField',
  options: [{value:1, label: 'Selected Value'}]
}

What do you need specifically? I think I have shown you all the relevant code 🤔

foxhound87 commented 7 years ago

I need the fields definitions.

foxhound87 commented 7 years ago

it's only that select_field?

ricbermo commented 7 years ago

I'm so sorry @foxhound87 I tried again, just updated the package version to 1.20.0 and now this is working

const onChange = field => (e, k, payload) => {
  field.set(payload);
};

where payload is the actual value returned from SelectField. I was my mistake.

Thank you so much

foxhound87 commented 7 years ago

Great to know :) thank you for your feedbacks!

anusree-mmlab commented 7 years ago

const onChange = field => (e, k, payload) => { field.set(payload); };

event.target.value is undefined with React Native TextInput ...field.bind(onChange: handleChange(field)).

How can i get the value inside handleChange?

foxhound87 commented 7 years ago

In react native you should use onChangeText: https://facebook.github.io/react-native/docs/textinput.html

foxhound87 commented 7 years ago

If you wanto to use onChangeText inside the bind() method, then you have to create custom bindings for your TextInput: https://foxhound87.github.io/mobx-react-form/docs/bindings/custom.html#implement-a-rewriter

anusree-mmlab commented 7 years ago

<TextInput value={this.state.email} {...this.field.bind({id:'email', type:'text', placeholder:null, onChange: this.handleOnChange(this.field) })} style={{width:100, height:40}}/>

This is current line

<TextInput value={this.state.email} {...this.field.bind({id:'email', type:'text', placeholder:null, onChangeText: (text) => this.handleOnChange(text,this.field) })} style={{width:100, height:40}}/>

Not working. Then where shall i add the inChangeText ?

anusree-mmlab commented 7 years ago

Yes i tried Custom Bindings too, the way i implemented is not working

foxhound87 commented 7 years ago

First of all, I suggest to use the package fields definition for the props instead of passing them to the bind(). this because passing the props to the bind method they will skip the mobx store. It's not the default behavior to use the package, it's just to "overwrite" them. Instead, If you define the props into the fields definitions they will be used by the store for correct behavior. The only exception is the onChange which is not allowed in the fields definition (I will fix that in future updates), so we pass it directly to the component.

anusree-mmlab commented 7 years ago

I just wanted the event.target.value worked in the onChange: handleChange method for my React Native TextInput on the ...field.bind({onChange: HandleChange(field)}) Can you please help me out in that ?

foxhound87 commented 7 years ago

React Native does not have an event object passed to the onChange, as you can see in the official doc, you need to use onChangeText, this should work without bindings:

const handleOnChange = (field) => (text) => {
  field.set(text);
};

<TextInput 
  {...field.bind({ onChange: () => {} })}
  onChangeText: {handleOnChange(field)}
/>
anusree-mmlab commented 7 years ago

Yes i got the input text. Is it possible to apply field.validate() after setting the value? I added it on handleOnChange, Later inside onChange: () => {field.validate()}, but none of them worked.

foxhound87 commented 7 years ago

You can set the Form option 'validateOnChange: true' and showErrorsOnChange: true' (it will work for all fields). Otherwise you can call 'field.validate({ showErrors: true })' just after setting the value to validate just the single field (maybe you need to just show errors)

foxhound87 commented 7 years ago

Or you can set the same form options to a single field definition too using an 'options' object!

anusree-mmlab commented 7 years ago

field.validate({ showErrors: true }), is not showing errors. I think, i should use it with normal react. With RN it seems to be difficult

foxhound87 commented 7 years ago

Are you familiar with mobx?

Are you using the mobx-react observer decorator on your React components?

anusree-mmlab commented 7 years ago

Thanks for your concern :)

I am newbie to mobx, react, react native [that is the issue]

Now i am doing it with observer, observable, action etc. I am creating my own validator like this

@observable assessment_form_obj = { assessor_name:{value:'', validation:['required','email'], regex:{'required':'req','email':''}, message:{'required':'*Required Field','email':'Enter Valid Email'}, hasError: false, validationFailed:['required'] } };

foxhound87 commented 7 years ago

That code is not related to mobx-react-form. Please, if you still need help, create a CodeSandbox with your form config. Thank You.