Evo-Forge / Essence

Essence - The Essential Material Design Framework
MIT License
415 stars 47 forks source link

Incorrect parameter in custom onChange event function of switches. #61

Closed volodimirian closed 8 years ago

volodimirian commented 8 years ago

If you use switch with switches type (I think that it doesn't matter what type to use) and implement your own onChange event the first parameter expects event where we could find current value of switcher.

But this parameter is undefined.

If it is correct behaviour, tell me please how I can find the current value without dom touching.

hetmann commented 8 years ago

Thanks @volodimirian for the feedback. I understand what you meant & I got the component changed. You can install the latest version _npm i essence-switch_ and let me know if this suits your needs.

Code snippet:

import React from 'react';
import {render} from 'react-dom';
import Switch from 'essence-switch';

class SwitchDemo extends React.Component {
  _checkboxChange() {
    const isChecked = !this.checked;
    const checkboxValue = this.value;
    console.log('checkbox', {
      isChecked,
      checkboxValue
    });
  }

  _radio1Change() {
    const isChecked = !this.checked;
    const checkboxValue = this.value;
    console.log('radio 1', {
      isChecked,
      checkboxValue
    });
  }

  _radio2Change() {
    const isChecked = !this.checked;
    const checkboxValue = this.value;
    console.log('radio 2', {
      isChecked,
      checkboxValue
    });
  }

  _switchChange() {
    const isChecked = !this.checked;
    const checkboxValue = this.value;
    console.log('switch', {
      isChecked,
      checkboxValue
    });
  }

  render() {
    return (
      <div>
        <Switch type={'checkbox'} text={'Default checkbox'} name={'switch-uncheckbox'} defaultValue={'checkbox'} onChange={this._checkboxChange} />
        <hr />
        <Switch type={'radio'} text={'1st option'} name={'switch-radio'} defaultValue={'switch-radio-1'} onChange={this._radio1Change} />
        <Switch type={'radio'} text={'2nd option'} name={'switch-radio'} defaultValue={'switch-radio-2'} onChange={this._radio2Change} />
        <hr />
        <Switch type={'switches'} afterText={'ON'} beforeText={'OFF'} name={'switch-switches'} defaultValue={'switch'} onChange={this._switchChange} />
      </div>
    )
  }
}

render(<SwitchDemo/>, document.querySelector('.app'));
volodimirian commented 8 years ago

Hi @hetmann. Before I submitted this bug, I had used version 1.0.6 of essence-switch. I updated it to 1.0.9. But there was not that I need in 1.0.9 version.

I found such code

onChange(event) {
        this.setState({
            checked: event.target.checked,
            value: event.target.value,
        });

        if (this.state.onChange) {
            return this.state.onChange();
        }
    }

in essence-switch src folder.
I suggest and I think it would be more convenient to forward parameter event to function

this.state.onChange

and developer could catch it and take current value of control.

I have no doubt that your snippet is worked but it seems not comfortable to write additional snippet.

If I am mistaken in some ways, tell it to me. I am a beginner in react implementation.

Thanks for understanding, Wait for your answere

hetmann commented 8 years ago

@volodimirian you're right, to pass the event target to the callback onChange function would be more elegant.

I'll update the component as follows:

onChange(event) {
  this.setState({
    checked: event.target.checked,
    value: event.target.value,
  });

  if (this.state.onChange) {
    return this.state.onChange(event);
  }
}
hetmann commented 8 years ago

@volodimirian version updated to _essence-switch@1.0.10_ Let me know if this is more comfortable to use it, as I didn't test it (sorry).

volodimirian commented 8 years ago

Ok, thanks. I'll write when I check 😉

volodimirian commented 8 years ago

@hetmann Yeah, that's what I need. Thank you a lot. This issue could be closed 😉

hetmann commented 8 years ago

@volodimirian I thank you for the help :) Happy coding!