React - onClick(item) of item in list using for loop

Question

I am making a react list, whose items should trigger an action to update state. Both the triggering and the state update occur, however, the onClick method is calling the action with the same argument every time. This should not occur. The argument passed to the action must depend on the item of the list clicked.

I can not seam to find the bug that is causing this error.

This is my dropdown class, where the list is rendered.

class DropDownMenu extends React.Component {
  constructor(props){
    super(props)
  }
  render(){
    let content = []
    var categories = this.props.categories
    for (var i=0; i < categories.length; i++) {
      var catName = categories[i]
      var line = (
        <li
          key ={i}
          onClick = {() => {this.props.onClick(catName)}}
        >
        <p>{catName}</p>
        </li>
      )
      content.push(line)
    }
    return (
      <div>
        <Dropdown ref="dropdown">
          <DropdownTrigger>Categories</DropdownTrigger>
          <DropdownContent>
            <ul>
              { content }
            </ul>
          </DropdownContent>
        </Dropdown>
      </div>
    )
  }
}

This is the container that manages the state and calls the above class

class MenuContainer extends React.Component {
  constructor(props) {
    super(props)
  }

  render(){
    return (
      <div>
        <div>
          <a onClick = {() => {this.props.changeView("main")}}>back</a>
          <a onClick = {() => {this.props.changeView("shoppingCart ")}}>my cart</a>
        </div>
        <div>
          <DropDownMenu
            categories = {this.props.categories}
            onClick = {(catName) => {this.props.selectCategory(catName)}}
          />
          <ItemList
            items = {this.props.items}
          />
        </div>
      </div>
    )
  }
}

const mapStateToProps = (state) => {
  return {
    categories: getCategories(state),
    items: getItems(state)
  }
}

const mapDispathToProps = (dispatch) => {
  return {
    changeView: (view) => {
      dispatch(setView(view))
    },
    selectCategory: (catName) => {
      dispatch(setCategory(catName))
    }
  }
}

export default connect(mapStateToProps, mapDispathToProps)(MenuContainer)

I reducer the error to the line:

   for (var i=0; i < categories.length; i++) {
      var catName = categories[i]
      var line = (
        <li
          key ={i}
          onClick = {() => {this.props.onClick(catName)}}
        >
        <p>{catName}</p>

I replaced this.props.onClick with console log of the catName, and it always logs the same, althoug it correctly shows the items in the

tag.

Prakash Sharma · Accepted Answer · 2017-11-20 18:19:07Z

7

Your for loop does not form a closure. Better use a map function like this

let content = categories.map((catName, i) => {
      return (
        <li
          key ={i}
          onClick = {() => {this.props.onClick(catName)}}
        >
        <p>{catName}</p>
        </li>
      )
    });

edited Nov 20, 2017 at 18:19

answered Nov 20, 2017 at 18:15

Prakash Sharma

16.6k6 gold badges35 silver badges38 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Rafael Marques Over a year ago

That worked perfectly, thank you. just a follow up question, how can i pass a unique ID to the <li> tag, since the {i} from the for function is gone.

Dakota Over a year ago

Map still provides the index as the second argument in your function. although using the index as a key is usually a bad idea.

Prakash Sharma Over a year ago

@RafaelMarques No its not gone. Second argument of map calback is index. Check updated answer

Rafael Marques Over a year ago

perfect, thank you. will mark this as answered in 4 minutes (allowed time)

T Porter Over a year ago

Just a quick comment: Using the index as a key isn't always a bad idea. If the items in the list are guaranteed to not change their order, then using the index is fine. Otherwise, you will want to use a unique identifier for each record being mapped.

Collectives™ on Stack Overflow

React - onClick(item) of item in list using for loop

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related