0

This is a follow up question of this question. I'm trying to log the value of two check boxes in terms of true or false.
I have two checkbox in a <form>. The <form> have an onChange={this.checkBoxOnChange} assigned to it which fires up checkBoxOnChange(event){..} function on every change in the form. I am trying to map_(Log)_ the status (ie whether they are checked or not). So, I've initially taken there value as false in this.state() inside constructor() as they are not-checked(initially) then on each event I'm trying to change there value respectively (ie if false the true & vice versa).
But, when I'm trying to log their output they are printing in wrong order. Checking on checkbox is even changing value of another checkbox.

This is code for checkbox inside render method:

  render() {
    return (
      <div>
      <form onChange={this.checkBoxOnChange.bind(this)}>
        <input type="checkbox" name="invoicestatus" value="BILLDED" />BILLDED<br />
        <input type="checkbox" name="invoicestatus" value="PENDING" />PENDING<br />
      </form>
      <ListData data={this.state.data}/>
    </div>
    );
  }
};        

This is the checkBoxOnChange() function within that same class (Component):

checkBoxOnChange(event){
   console.log("BILLDED"+this.state.billed+"PENDING"+this.state.pending);
   (event.target.value=='BILLDED') && (this.setState({billed:(!this.state.billed)}));
   (event.target.value=='PENDING') ? (this.setState({pending:(!this.state.pending)})) : null;
    console.log("BILLED"+this.state.billed+" PENDING"+this.state.pending);
  }          

Initial value is being set as:

constructor(props){
    super(props);
    this.state = {
      data:[],
      billed:false,
      pending:false
     }
  }
  • Why I am getting wrong values?
  • How do I make it work in proper order?

This question is different from this SO post on following grounds:
- Because both the solution to this question mention about Uncontrolled Components vs Controlled Components
- Annnd there's also and awesome setState() feature of React prevState by which I was (& would be) unaware of, if I'd (or someone else) follows above mentioned post.

BlackBeard
  • 10,246
  • 7
  • 52
  • 62

2 Answers2

4

There are a few issues with your approach.

  1. You are using uncontrolled input elements but are still using storing their value to state. Either use uncontrolled or uncontrolled components, not a mix. More info here and here.

  2. When updating the state with a value that depends on the previous state value, you should not use the function updater form rather than the object form. More info, here.

    If you need to set the state based on the previous state, read about the updater argument.

  3. You should be aware that setState() is asynchronous, so logging to the console immediately after updating the state is most likely going to give you the previous state. More info here.

    setState() does not always immediately update the component. It may batch or defer the update until later. This makes reading this.state right after calling setState() a potential pitfall.


Here's how I would do it:

class MyApp extends React.Component {
  constructor() {
    super();
    this.state = {
      pending: false,
      billed: false
    };
  }

  togglePending = () => {
    this.setState((prevState) => ({pending: !prevState.pending}));
  }

  toggleBilled = () => {
    this.setState((prevState) => ({billed: !prevState.billed}));
  }

  render() {
    let {pending, billed} = this.state;
    return (
      <div>
        <input type="checkbox" name="invoicestatus" checked={billed} onChange={this.toggleBilled} value="BILLDED" />BILLDED<br />
        <input type="checkbox" name="invoicestatus" checked={pending} onChange={this.togglePending} value="PENDING" />PENDING<br />
      </div>
    );
  }
}

ReactDOM.render(<MyApp />, document.getElementById("app"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react-dom.js"></script>
<div id="app"></div>

If you insist on using your approach, here's how to fix it:

class MyApp extends React.Component {
  constructor() {
    super();
    this.state = {
      pending: false,
      billed: false
    };
  }

  checkBoxOnChange(event){
    console.log("BILLDED"+this.state.billed+"PENDING"+this.state.pending);
    let obj = {};
    if (event.target.value == 'BILLDED') {
      this.setState({billed: !this.state.billed}, () => {
        console.log("BILLED"+this.state.billed+" PENDING"+this.state.pending);
      });
    } else if(event.target.value=='PENDING') {
      this.setState({pending: !this.state.pending}, () => {
        console.log("BILLED"+this.state.billed+" PENDING"+this.state.pending);
      });
    }
  }

  render() {
    return (
      <div>
      <form onChange={this.checkBoxOnChange.bind(this)}>
        <input type="checkbox" name="invoicestatus" value="BILLDED" />BILLDED<br />
        <input type="checkbox" name="invoicestatus" value="PENDING" />PENDING<br />
      </form>
    </div>
    );
  }
}

ReactDOM.render(<MyApp />, document.getElementById("app"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react-dom.js"></script>
<div id="app"></div>
Community
  • 1
  • 1
Chris
  • 57,622
  • 19
  • 111
  • 137
  • In your approach where you would place the log in order to print both the values in correct order? – BlackBeard Sep 14 '17 at 08:03
  • @NiladriSekharBasu, I would log them normally just *before* each `setState()` - just like you did. Then, I would also log inside the callback for each `setState()`. See my second hidden snippet how I do that. – Chris Sep 14 '17 at 08:05
  • Just a follow up question. Why passing a log printer function(which is defined elsewhere within the same class) does not yield correct value? – BlackBeard Sep 14 '17 at 08:43
  • @NiladriSekharBasu, not sure what you mean. I'd have to take a closer look. Post a new question with code and then send me the link to it? – Chris Sep 14 '17 at 08:45
  • Will surely do :). – BlackBeard Sep 14 '17 at 09:04
  • And in your answer in first point u mean _controlled or uncontrolled components_ instead of _uncontrolled or uncontrolled components_ ri8? – BlackBeard Sep 14 '17 at 09:07
  • kindly see [this](https://stackoverflow.com/questions/46237553/why-only-one-component-is-re-rendering-immediately-and-other-is-not) question – BlackBeard Sep 15 '17 at 11:03
3

You are over complicating things in my opinion.
Instead of using uncontrolled component use use controlled components.
Just set a handler for each checkbox:

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      pending: false,
      billed: false
    };

    this.togglePending = this.togglePending.bind(this);
    this.toggleBilled = this.toggleBilled.bind(this);
  }

  togglePending() {
    this.setState({
      pending: !this.state.pending
    });
  }

  toggleBilled() {
    this.setState({
      billed: !this.state.billed
    });
  }

  render() {
    const { pending, billed } = this.state;
    return (
      <div>
        <input
          type="checkbox"
          name="invoicestatus"
          checked={billed}
          onChange={this.toggleBilled}
          value="BILLDED"
        />BILLDED<br />
        <input
          type="checkbox"
          name="invoicestatus"
          checked={pending}
          onChange={this.togglePending}
          value="PENDING"
        />PENDING<br />
      </div>
    );
  }
}

ReactDOM.render(<App />, document.getElementById("root"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script>
<div id="root"></div>
Sagiv b.g
  • 30,379
  • 9
  • 68
  • 99