Material-ui: [Checkbox] setting the state in onCheck doesn't display the checked box

[itaied246]

Setting the state in the callback of onCheck event cancels the [V] mark on the selected box.
Without setting the state, the checkbox works as expected.

Working fine

<Checkbox
                label="price"
                onCheck={this._handleCBPrice.bind(this)} />
_handleCBPrice(event, checked){}

Doesn't mark the box with a [V]

<Checkbox
                label="price"
                onCheck={this._handleCBPrice.bind(this)} />

_handleCBPrice(event, checked){
     this.setState({someKey:someValue})
  }

[sicknesz]

Same issue, my code :

<ListItem leftCheckbox={<Checkbox onCheck={this._onClickHandler} />} key={0} primaryText="someText"/>,

_onClickHandler: function(event, checked) {
    var {selectedItems} = this.state;
    selectedItems.push( someData ); 
    this.setState({selectedItems})   
}

[byronrau]

having this same issue as well

[chechenev]

+1
same issue with setState in "onCheck"

[oliviertassinari]

I don't understand. How is this even supposed to work?

[chechenev]

Wow, I found an unexpected solution: if I wrap < Checkbox / > in something like < Paper > or < Popover > - all works fine with setting state.

[xiam]

Happened to me too, I can not imagine the reason behind this but here's a quick and dirty workaround with window.setTimeout:

class Whatever extends React.Component {
  constructor(props) {
    super(props);

    this.handleCheck = this.handleCheck.bind(this);
    this.state = {
      foo: "",
    }
  }

  handleCheck(ev, checked) {
    window.setTimeout(
      function() {
        this.setState({foo: "bar"});
      }.bind(this),
      0
    );
  }

  render() {
    return (
      <div style={styles.block}>
        <b>{this.state.foo}</b>
        {(this.props.options || []).map(function(option, i) {
          return <Checkbox
            onCheck={this.handleCheck}
            key={option.value}
            value={option.value}
            label={option.label}
          />
        }.bind(this))}
      </div>
    );
  }
}

[oliviertassinari]

@xiam Looks like your are not using the React API correctly.

[xiam]

@oliviertassinari Yeah, probably not, but it seems like we're facing the same problem and this workaround allowed me to continue working on my project.

[oliviertassinari]

@xiam Actually, your code seems fine. Looks like this issue can be confirmed.

[dalizwei]

I don't know if that make sens. but it seem that, every update, the checkBox is re-affecting the defaultChecked so if you update this props at the same time defaultChecked=this.state.myCheckBox it solve the problem. at least mine :) .

[ClemMakesApps]

I'm having the same issue too. @xiam, thanks for the temporary workaround. I tried @dalizwei's solution but it did not work for me. Main issue I was having was with setState. The state would update based on my react dev tools but the UI wouldn't update to reflect that change.

[robinnewhouse]

+1 Just ran in to this issue

[Steague]

+1

Finally found a thread where other people are seeing this behavior. Any real fix in the pipeline other than a setTimeout hack?

[paran0a]

Using @xiam 's solution for now. The problem seem to persisting for quite some time now. Are there any "real" fixes beside settimeout hack?

[puranjayjain]

@oliviertassinari there are some changes with the newer version of the checkbox js and internal components so that solution won't be valid. Will there be a PR accepted for this bug?

Solution: (to be applied for all three switch based components)
e.g for https://github.com/callemall/material-ui/blob/master/src/Checkbox/Checkbox.js#L162
changed to this:

setTimeout(() => this.setState({switched: newSwitched}))

Tested, it works. not checked for switch and radio but it will work there as well.

[mikroware]

Isn't it in React the general idea that once you implement onChange (or onCheck in this case), the component becomes a so called "controlled component"? See https://facebook.github.io/react/docs/forms.html for reference.

Controlled components render purely based on the input props. In the examples which were given in this issue, there is no checked prop passed. Which I believe means that once a re-render happens (mostly after this.setState) the Checkbox component is re-rendered with its original props (non-checked props for example).

While using this pattern I never had any trouble with situations described in this issue. With that I mean using onCheck to keep track of the checked state in the parents component state and setting the checked prop to reflect the component state back to the CheckBox.

[jjgumucio]

@mikroware Is completely right on this!

[puranjayjain]

@mikroware is completely right, please close this issue as his solution is what we need.

[oliviertassinari]

Isn't it in React the general idea that once you implement onChange (or onCheck in this case), the component becomes a so-called "controlled component"? See https://facebook.github.io/react/docs/forms.html for reference.

As far as I know, the property that is triggering the _controlled_ behavior on a regular component is the value one.
But yes, the _controlled_ and _uncontrolled_ behavior of this component seems to work correctly based on the docs examples.
I'm closing this issue as it seems to be linked to a misuse of the API.

@mikroware @jjgumucio @puranjayjain Thanks for your help.

[Evilcat325]

After viewing the source code and confused by handleStateChange rename to onParentShouldUpdate; onCheck rename to handleCheck then rename to onSwitch and massive unifying code on the EnhancedSwitch,
I think my recent project found the root of this issues. ( I really think the source code need to refactor a bit to read it easier.
//sorry for the complaining but it is really confusing and hard to keep track of the function name

Root of the problem

Applying on onCheck() usually cause an setState() on the owner Element

This will also trigger a render() lifecycle event on the owner Element
In React when Owner render() it triggers the componentWillReceiveProps() on its child, in our case the <Checkbox />
In the componentWillReceiveProps

componentWillReceiveProps(nextProps) {
    this.setState({
      switched: this.props.checked !== nextProps.checked ?
        nextProps.checked :
        this.state.switched,
    });
  }

Normally if onCheck() and checked are provided the update looks like this

input.onChange(event) => EnhancedSwitch.handleChange(event) => 
EnhancedSwitch.onSwitch(event, isInputChecked) === Checkbox.handleCheck(e, v) === Checkbox.onCheck(e, v) => 
Owner.onCheck(v) => Owner.setState() => Owner.render() => 
Checkbox.componentWillReceiveProps() => Checkbox.setState({switched:nextProps.checked}); =>
Checkbox.render() => EnhancedSwitch.componentWillReceiveProps() => 
EnhancedSwitch.setState(switched: nextProps.checked) => input.value = isInputChecked

But If onCheck() is provided but no check or value provided Checkbox lifecycle event becomes

componentWillReceiveProps(nextProps) {
    this.setState({
      switched: this.props.checked !== nextProps.checked ? // undefined !== undefined => always false
        nextProps.checked :
        this.state.switched,
    });
  }
 => this.setState({switched: this.state.switched}); 

this is a duplicate setState that we did not anticipate. Since we do not provide a check props to the Checkbox, the Checkbox should be the state container for EnhancedSwitch and handle its own state, but Owner.render() is called in the same call stack this.setState() inside Checkbox.handleStateChange.

So the actual render we get is the
this.setState() inside Checkbox.handleStateChange
and
this.setState() inside Checkbox.componentWillReceiveProps
batched together.

A fix should not be a setTimeout( () => this.setState() on the Owner. Checkbox nor EnhancedSwitch

It should be a simple type check inside the componentWillReceiveProps

componentWillReceiveProps(nextProps) {
  if(nextProps.checked !== undefined) {
    this.setState({
      switched: this.props.checked !== nextProps.checked ? nextProps.checked : this.state.switched
    });
  }

setTimeout simply change the order of the render and setState;

Checkbox.handleStateChange(event, true);

//normal setState
Checkbox setState({switch:true}) Finished
Owner setState() Finished
Owner rendering
Checkbox componentWillReceiveProps
    Checkbox state: false => Checkbox setState({switch:false});
    //React auto batched two setState() then render
Checkbox rendering
EnhancedSwitch rendering

//setTimeout setState
Checkbox setState() Finished
Owner setTimeout setState() Finished  //push setState() to event loop out of the current call stack 
Checkbox rendering()              //on rendering the Checkbox state updated
EnhancedSwitch rendering()   //this prioritized Checkbox setState() => render() to run next
Owner rendering()                   //current stack is empty, next call in event loop is Owner
Checkbox componentWillReceiveProps
    Checkbox state: true => Checkbox setState({switch:true});
Checkbox rendering()             
EnhancedSwitch render()

If you want to learn more about the eventloop and setTimeout hack

Philip Roberts: What the heck is the event loop anyway? | JSConf EU 2014
This is a really good video explaining what is happening.

one note on controlled component

Only if you set the value of the input, it should be considered as a controlled component. For example, if I ever want to use google analytic to true my input actual usage I should only need to track it with onCheck rather than turn it into a controlled component.

And allowing onCheck without making it a controlled component makes it skips tons of parent update.

In my case I render 30 check box as a feature list and I don't want my Owner component to render every time a checkbox change but only match the state to the selected. This allows me to skip Array.map many times and I notices this reduce the visual response time at least by 300ms.
This library needs a lot performance and lifecycle hook refactoring. And this should be a good start.

I am pretty new to how to open a PR or anything, but I will try it when I have time.

If you read this comment and understand the reason I think it should be a bugfix without changing anything else.
Or we could discuss the state of switch and how to remove the setState in WillReceiveProps, this is a typical A implies B does not imply B implies A design issue.
We should enforce a single source of truth, either as a state at top level component then the child will only use props as the source or as the actual value in the DOM input which need to connect with the correct lifecycle event to map to a state.

Related and could correctly closed issue:
Checkboxes and toggles issue
Checkbox not updating

Related PR:
[Toggle][Checkbox] setting the state in onCheck doesn't display the checked box
fix [Toggle][Checkbox] setting the state in onCheck doesn't display the checked box

[dalizwei]

@Evilcat325 : it's really a good explanation and that mach the problem I was running in.
And I was obliged, like you said, to change my CheckBox to a controlled component to get it work.
+1

[oliviertassinari]

I have managed to reproduce this issue.

The root of the issue is described in the React documentation.

setState() does not immediately mutate this.state but creates a pending state transition. Accessing this.state after calling this method can potentially return the existing value.

[GuillaumeCisco]

@oliviertassinari Thank you for this ref from the documentation.
I have a similar problem, but slightly weird...
My checkbox works in development mode (using webpack hot module server), but in minified code for production (webpack build), it doesn't anymore...
I'm using redux-form, the checkbox is in a form inside a material-ui Dialog.
It is really confusing...

EDIT: After some research, I've managed to make it work with material-ui and the V6 API.
You can see my comment with updated code here : https://github.com/erikras/redux-form/issues/334#issuecomment-236596060
By the way, I still don't understand why it did not work before. Minified code didn't seem the problem...

[hyalkaf]

I don't know if this issue is Fixed entirely but here is my scenario:

I was passing props to a component that has Checkbox then create a state in it with check: this.props.checked. When I create Checkbox, I would do

<Checkbox
  label=""
  defaultChecked={this.state.checked}
  onCheck={this.onCheck}/>

Now, this wasn't working and rightly so as it says in the documentation to not use defaultChecked since it will make it uncontrolled component. The first time onCheck is called, isInputChecked will have correct value and it will work fine but all subsequent calls will have isInputChecked the same value usually true. so I changed it to this:

<Checkbox
  label=""
  checked={this.state.checked}
  onCheck={this.onCheck}/>

This also didn't work and it was giving the same issue. I don't why this didn't work but here is what I did next.

<Checkbox
  label=""
  checked={this.props.checked}
  onCheck={this.onCheck}/>

I stopped maintaining state in this component and made the parent control the state which is the better solution. This also didn't work which is also weird.

Eventually, I decided to remove checked all together

<Checkbox
  label=""
  onCheck={this.onCheck}/>

Of course this worked but now first state of checkbox is always false so I tried one more time adding checked:

<Checkbox
  label=""
  checked={this.props.checked}
  onCheck={this.onCheck}/>

This Worked!!!

I don't know if this gives an insight to what happened or you guys probably know this. I could have problems with hot reloading or whatever but this is exactly what happened. I thought I would share this even when I have my checkbox working.

[GuillaumeCisco]

Your last code block and the prior prior to last code block are the same...

[hyalkaf]

That was my point. They behaved differently.

[benjiwheeler]

One simple solution using state:

  constructor(props) { // list of objects
    super(props);
    this.state = {
      is_checked: false
    };
  }

  toggleCheckbox(event) {
    let newValue = (this.state.is_checked === "on" || this.state.is_checked === true) ? false : true;
    this.setState({
      is_checked: newValue
    });
  }

  render() {
    return (
      <div>
        <input type="checkbox" checked={this.state.is_checked}
          onChange={this.toggleCheckbox.bind(this)}
        />
      </div>
    );
  }

[adamtysonsmith]

It clicked for me when I realized that I could not rely on the input keeping track of the state.

Its much easier for me to manage keeping track of the state with a simple method in our class:

constructor() {
  super();
  this.state = {
    isChecked: true // set initial state as you please
  }
}

toggleCheckbox(toggle, evt) {
  const isChecked = !toggle;  // simply negate the previous state passed in
  this.setState({ isChecked })
}

Then in our view (using stateless component and passing state as props in my case):

<Checkbox defaultChecked={props.isChecked} 
   onCheck={ props.toggleCheckbox.bind(null, props.isChecked) }

This way the state is configuring the initial checked value, and passing that value through on the checked event, and our simple method negates that value and sets the state to the new value. This has worked for me, my two cents.

[skosch]

I have noticed that the checkbox will think it's uncontrolled when checked={undefined}, and then throw that warning when you click to check it.

If you want checked to equal a value that may be undefined, then you need to turn it into a boolean, i.e. write e.g. checked={yourObject[someKey] || false}.

[oliviertassinari]

@skosch I would expect a native chexkbox to behave the same. If that's not the case, then feel free to open an issue, that's something we could fix to stay consistant.

[skosch]

@oliviertassinari I don't think it's a bug per se, it's just something that's easy to miss. The warning says "EnhancedSwitch is changing an uncontrolled input of type checkbox to be controlled ..." and I went back and forth for a few hours scratching my head. I also realize that's a react complaining and not material-ui; my comment was just to save others googling the same error some time.

[cmattick]

If someone like me is looking for a reason that the onCheck handler provides the wrong value when in development mode with a browser sync setup: set browser-sync option ghostMode to false and the problem is fixed.

[GuillaumeCisco]

Fascinating @cmattick, I've tested running my app on the proxified url from browserSync and the original url.
With the proxified url (only a different port in my case), the checkbox does not work properly with checked and onCheck handler.
But it does work correctly with the original url, OnCheck give me the correct values.
I've tried using ghostMode option without success.

I wonder why this kind of issue appear, this is clearly very hard to catch! And I still do not understand why it occurs.

[cmattick]

@GuillaumeCisco After reading https://github.com/BrowserSync/browser-sync/issues/1102 I updated browser sync to the recent version, maybe that helps?

[GuillaumeCisco]

Thank you @cmattick!
I confirm this behavior!
I was using:

"browser-sync": "2.18.2",
"browser-sync-webpack-plugin": "1.1.3",

just upgraded to:

"browser-sync": "2.18.8",
"browser-sync-webpack-plugin": "1.1.4",

With the last version, using ghostMode: false does the trick! But without, the behavior is the same.

Maybe Material ui doc could write a chapter about compatibility with browserSync plugin ;)
It would be great!

[wootencl]

+1 For having a note about material-ui and compatibility with browserSync in the README. Lost a decent amount of time trying to figure out why this wasn't working as intended. Happy to make a PR to add a little note if something like that would be accepted.

/cc @oliviertassinari

[oliviertassinari]

@wootencl Is Material-UI v1 affected by this issue? Isn't an issue enough as documentation? I don't think that putting it in the README will help. People won't read it. It will only add noise. Instead, I would rather incentivize browserSync to fix the issue.

[wootencl]

@oliviertassinari v1 Yes. Uncertain regarding next. And valid point. More disappointed by the time I lost. My only concern with this issue being the sole documentation is that there are several issues related to switch clicks not working in this repo and stackoverflow (some with solutions and others unknown). Certainly possible to find this answer but not immediate. Suppose my thoughts are that browsersync is a widely enough used tool that this issue may be common enough to be documented somewhere (whether it be the README or elsewhere).

Ultimately up to you of course though.

Outside of this I've had a splendid time using material-ui and appreciate all the hard work that you (and others) have put into it 🙂

[oliviertassinari]

Suppose my thoughts are that browsersync is a widely enough used tool that this issue may be common enough to be documented somewhere

@wootencl Anyway I could reproduce the issue? I would rather start looking into solutions before giving up with a warning :).

[wootencl]

@oliviertassinari Certainly! Threw together a little example app:
https://github.com/wootencl/m-ui-bsync-issue-example