Reactjs - add/remove item from array and store using checkbox

Question

Prerquisite

I'm fetching a list of accounts (Ajax request) which I display on page load (with a checkbox next to them). By default all the accounts are selected and added to the store (redux).

Goal

Add/remove accounts from array & store (redux) when checkbox are checked/unchecked:

• checbox is checked --> add account to array & store
• checkbox is unchecked --> remove account from array & store

Logic

I created two separate actions & reducers:

• one to manage the checkbox status
• one to manage the addition/removal of the account to the array & store

When testing my code, it works fine at the beginning but eventually the accounts added/removed are not correct. The issue must be in savingAccount() but not sure what I'm doing wrong?

My code

Pre-populating data to the store in ComponentWillMount():

  componentWillMount = () => {
    let defaultAccount = this.props.account
    let defaultCheckbox = this.props.checkboxStatus

    for(let i =0; i < this.props.products.arrangements.items.length; i++){
      const data = {}
      data['id'] = i
      data['isSelected'] = true
      data['sortCode'] = this.props.products.arrangements.items[i].sortCode
      data['accountNumber'] = this.props.products.arrangements.items[i].accountNumber
      data['accountName'] = this.props.products.arrangements.items[i].accountName
      defaultAccount = defaultAccount.concat(data)

      const checkboxesArray = {}
      checkboxesArray['id'] = i
      checkboxesArray['checked'] = true
      defaultCheckbox = defaultCheckbox.concat(checkboxesArray)
    }

    this.props.addAccount(defaultAccount)
    this.props.toggleCheckbox(defaultCheckbox)
  }

Displaying list of accounts from Ajax response (this.props.products.arrangements.items)

render() {
  return (
    <div>
      {typeof(this.props.products.arrangements.items) !== 'undefined' &&
       (Object.keys(this.props.account).length > 0) &&
       (typeof(this.props.checkboxStatus) !== 'undefined') &&
       (Object.keys(this.props.checkboxStatus).length > 0) &&
       (Object.keys(this.props.products.arrangements.items).length > 0) &&
        <div>
          {this.props.products.arrangements.items.map((item,i) =>
            <div className="accountContainer" key={i}>
              <Checkbox
                required
                label={"Account Number "+item.accountNumber+" Product Name "+item.accountName}
                value={true}
                checked={this.props.checkboxStatus[i].checked === true? true: false}
                onChange = {(e) => {
                  this.toggleChange(this.props.checkboxStatus[i])
                  this.saveAccount(e, i, item.accountNumber, item.accountName)
                }}
              />
                </div>
          )}
        </div>
      }
      </div>
  )
}

Updating isSelected value when checkbox is checked/unchecked:

saveAccount = (e, i, accountNumber, productName) => {
  const data = {};
  data['id'] = i
  data['accountNumber'] = accountNumber
  data['productName'] = productName

  if(this.props.checkboxStatus[i].checked === true){
    let accountArray = Array.from(this.props.account)
    accountArray[i].isSelected = true
    this.props.addAccount(accountArray)
  }
  else {
    let accountArray = Array.from(this.props.account)
    accountArray[i].isSelected = false
    this.props.addAccount(accountArray)
  }
}

Reducer

function Eligible(state = { products: {}, account: [], checkboxStatus: [] }, action){
  switch (action.type){
    case ADD_PRODUCTS:
      return {
        ...state,
        products: action.data
      }
    case ADD_ACCOUNT:
      return {
        ...state,
        account: action.data
      }
    case TOGGLE_CHECKBOX:
      return {
        ...state,
        checkboxStatus: action.data
      }
    default:
      return state
  }
}

Actions

export const ADD_PRODUCTS = 'ADD_PRODUCTS'
export const ADD_ACCOUNT = 'ADD_ACCOUNT'
export const TOGGLE_CHECKBOX = 'TOGGLE_CHECKBOX'

export function addProducts(data){
  return {type: ADD_PRODUCTS, data}
}

export function addAccount(data) {
  return { type: ADD_ACCOUNT, data}
}

export function toggleCheckbox(data) {
  return { type: TOGGLE_CHECKBOX, data}
}

Updating checkbox status:

  toggleChange = (checkbox) => {
    let toggleCheckbox = this.props.checkboxStatus
    toggleCheckbox[checkbox.id].checked = !checkbox.checked
    this.props.toggleCheckbox(toggleCheckbox)
  }

Answer 1

I think the asynchronicity of this.setState is probably causing an issue.

this.state contains both accounts and checkboxes:

this.state = {
  accounts: [],
  checkboxes: []
}

In your change event handler, you call two functions:

onChange = {(e) => {
    this.toggleChange(this.props.checkboxStatus[i])
    this.saveAccount(e, i, item.accountNumber, item.accountName)
}}

First toggleChange:

toggleChange = (checkbox) => {
    let toggleCheckbox = [...this.state.checkboxes];

    toggleCheckbox[checkbox.id].checked = !checkbox.checked
    this.setState({
        checkboxes: toggleCheckbox
    })

    this.props.toggleCheckbox(this.state.checkboxes)
}

You're updating the checkboxes property of the state (via this.setState) - all good there. But on the last line, you're passing this.state.checkboxes out. Since this.setState is async, this will likely not reflect the changes you just made (you could send toggleCheckbox instead).

The next function called in the event handler is saveAccount, which contains (partially):

const addAccountState = this.state

if(this.props.checkboxStatus[i].checked === true){
    addAccountState.accounts = addAccountState.accounts.concat(data)
    this.setState(addAccountState)
    this.props.addAccount(addAccountState.accounts)
}

Here you're taking the current value of this.state (which may be old due to the async setState). You update the .accounts property of it, then send the whole thing (which includes .accounts and .checkboxes) to this.setState.

Since the .checkboxes state may have been old (the previous this.setState may not have fired yet), this would queue up the old .checkboxes state to overwrite the new state you tried to save in toggleChange().

A quick and dirty fix there could be to use this.setState({accounts: addAccountState.accounts}) instead, but there may be other issues floating around too (like the modifying of this.state properties directly).

Because setState is asynchronous, subsequent calls in the same update cycle will overwrite previous updates, and the previous changes will be lost.

Beware: React setState is asynchronous!

---

Regarding the separation of store and state... one option might be to not store the checkboxes separately at all, but rather compute them based on which accounts are selected.

It will depend on the needs of your application of course, so I'll be making a few assumptions for the sake of example...

• Your application needs a list of selected accounts
• Your component needs to show a list of all accounts
• Your component has checkboxes for each account: checked = selected = part of application's 'selected accounts' list.

In this case, I would have the list of selected accounts passed in via props.

Within the component I would have the list of all accounts in the local state (if your 'all accounts' list is passed in via props already, then just use that - no local state needed).

Within the render function of the component, I would compute whether the checkbox should be checked or not based on whether or not the account exists in the 'selected accounts' list. If it exists, it's checked. If not, not checked.

Then when the user clicks to check/uncheck the box, I would dispatch the function to add or remove the account from the 'selected accounts' list, and that's it. The account would be added or removed, which would cause your props to update, which would re-render your component, which would check or uncheck the boxes as appropriate.

That may not jive exactly with your particular application's needs, but I hope it gives you some ideas! :)