CODEWITHSUNDEEP
reactjscheckboxreact-reduxstatereact-state
Rakonjac
Rakonjac

Reputation: 87

React checkboxes. State is late when toggling the checkboxes

I have a group of 3 checkboxes and the main checkbox for checking those 3 checkboxes.

Can someone explain to me what actually is happening behind the scenes and help me somehow to solve this mystery of React state? Thanks!

Here is a code snnipet: 

state = {
  data: [
    { checked: false, id: 1 },
    { checked: false, id: 2 },
    { checked: false, id: 3 }
  ],
  main: false,
}

onCheckboxChange = id => {
  const data = [...this.state.data];

  data.forEach(item => {
    if (item.id === id) {
      item.checked = !item.checked;
    }
  })

  const everyCheckBoxIsTrue = checkbox.every(item => item === true);

  this.setState({ data: data, main: everyCheckBoxIsTrue });
}

onMainCheckBoxChange = () => {
  let data = [...this.state.data];

  data.forEach(item => {
    !this.state.main ? item.checked = true : item.checked = false
  })

  this.setState({
    this.state.main: !this.state.main,
    this.state.data: data,
  });
}

render () {
  const checkbox = this.state.data.map(item => (
      <input
        type="checkbox"
        checked={item.checked}
        onChange={() => this.onCheckboxChange(item.id)}
      />
    ))
  }

return (
  <input type="checkbox" name="main" checked={this.state.main} onChange={this.onMainCheckBoxChange} />
  {checkbox}
)

Upvotes: 2

Views: 1437

Answers (2)

JMadelaine
JMadelaine

Reputation: 2964

You shouldn't be storing state.main to determine whether every checkbox is checked.

You are already storing state that determines if all checkboxes are checked, because all checkboxes must be checked if every object in state.data has checked: true.

You can simply render the main checkbox like this:

<input
  type="checkbox"
  name="main"
  checked={this.state.data.every(v => v.checked)}
  onChange={this.onMainCheckBoxChange}
/>;

The line this.state.data.every(v => v.checked) will return true if all of the checkboxes are checked.

And when the main checkbox is toggled, the function can look like this:

onMainCheckBoxChange = () => {
  this.setState(prev => {
    // If all are checked, then we want to uncheck all checkboxes
    if (this.state.data.every(v => v.checked)) {
      return {
        data: prev.data.map(v => ({ ...v, checked: false })),
      };
    }

    // Else some checkboxes must be unchecked, so we check them all
    return {
      data: prev.data.map(v => ({ ...v, checked: true })),
    };
  });
};

It is good practice to only store state that you NEED to store. Any state that can be calculated from other state (for example, "are all checkboxes checked?") should be calculated inside the render function. See here where it says:

What Shouldn’t Go in State? ... Computed data: Don't worry about precomputing values based on state — it's easier to ensure that your UI is consistent if you do all computation within render(). For example, if you have an array of list items in state and you want to render the count as a string, simply render this.state.listItems.length + ' list items' in your render() method rather than storing it on state.

Upvotes: 0

Jee Mok
Jee Mok

Reputation: 6566

I can't make a working code snippet based on the code you provided, one of the issues was:

const everyCheckBoxIsTrue = checkbox.every(item => item === true);

where checkbox is not defined.

However, I think you confused about using the old state vs the new state, it'd be simpler to differentiate if you name it clearly, e.g.:

eventHandler() {
  const { data } = this.state; // old state

  const newData = data.map(each => ...); // new object, soon-to-be new state

  this.setState({ data }); // update state
}

Here's a working example for your reference:

class App extends React.Component {
  state = {
    data: [
      { checked: false, id: 1 },
      { checked: false, id: 2 },
      { checked: false, id: 3 }
    ],
    main: false,
  }
  
  onCheckboxChange(id) {
    const { data } = this.state;
    
    const newData = data.map(each => {
      if (each.id === id) {
        // Toggle the previous checked value
        return Object.assign({}, each, { checked: !each.checked });
      }
      return each;
    });
    
    this.setState({
      data: newData,
      // Check if every checked box is checked
      main: newData.every(item => item.checked === true),
    });
  }
  
  onMainCheckBoxChange() {
    const { main, data } = this.state;
     
    // Toggle the previous main value
    const newValue = !main;

    this.setState({
      data: data.map(each => Object.assign({}, each, { checked: newValue })),
      main: newValue,
    });
  }
  
  render () {
    const { data, main } = this.state;

    return (
      <div>
        <label>Main</label>
        <input
          type="checkbox"
          name="main"
          // TODO this should be automatically checked instead of assigning to the state
          checked={main}
          onChange={() => this.onMainCheckBoxChange()}
        />
        {
          data.map(item => (
            <div>
              <label>{item.id}</label>
              <input
                type="checkbox"
                checked={item.checked}
                onChange={() => this.onCheckboxChange(item.id)}
              />
            </div>
          ))
        }
      </div>
    );
  }
}

ReactDOM.render(
  <App />
, document.querySelector('#app'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.6.3/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.6.3/umd/react-dom.production.min.js"></script>

<div id="app"></div>

Side note: You might want to consider not to use the main state

Upvotes: 2

Related Questions