Material UI | React | OnClick Event always sends the last index

Question

I am unable to select the listItem properly

I am a beginner to React and i am having trouble in iterating through List Item in Material UI . I am iterating through a json array and adding list item to a list. On clicking any list item , the list always selects the last item

 function handleListItemClick(index) {
    setSelectedIndex(index);

    props.user.updateState(index);
    props.user.setCourseName(props.mycourses[index].courseName);
  }

  function RenderListItem(props) {
    var indents = [];
    let ind = 0;

    for (var key in props.coursevals) {
      indents.push(
        // <span>

        <ListItem
          button
          selected={selectedIndex === key}
          onClick={() => handleListItemClick(key)}
          // onClick={event => handleListItemClick(event, 0)}
          className={classes.ListItem}
          key={key}
        >
          {/* {console.log(key)} */}
          <ListItemIcon>
            <img
              className={classes.courseimg}
              src="../../../public/images/book.png"
            />
          </ListItemIcon>
          <ListItemText primary={props.coursevals[key].courseId} />
          <img
            className={classes.rightarrow}
            src="../../../public/images/rightarrow.png"
          />
        </ListItem>
      );
    }
    return indents;



 }

OnClick Event always send the last index to the function. I want to send the index where i just clicked

Ernesto Stifano · Accepted Answer · 2019-09-03T13:25:00.500

The problem is that the variable key is the same for each iteration (it will be updated with the new key on each loop start). You're passing a callback which has a reference to that variable that at the end of the iteration will have the value of the last key.

I wasn't able to test the following code but it should work. Please let me know. The idea is to keep a reference to the "current key" for each loop, so each callback will reference to the correct value.

Note that I've replaced all key appearances inside the for body just to be more consistent. Expressions that are evaluated in real time (not at callback time) obviously doesn't have this problem.

Another, more fancy way would be to use Function.prototype.bind() (See on MDN). onClick={handleListItemClick.bind(undefined, key)}. It will pass a function with key embedded as the first argument.

Last alternative, using forEach() instead of a for loop would work because a new index will be created for each iteration. (Same result as with the code below).

 function handleListItemClick(index) {
    setSelectedIndex(index);

    props.user.updateState(index);
    props.user.setCourseName(props.mycourses[index].courseName);
  }

  function RenderListItem(props) {
    var indents = [];
    let ind = 0;

    for (var key in props.coursevals) {
      let currentKey = key;
      indents.push(
        // <span>

        <ListItem
          button
          selected={selectedIndex === currentKey}
          onClick={() => handleListItemClick(currentKey)}
          className={classes.ListItem}
          key={currentKey}
        >
          {/* {console.log(currentKey)} */}
          <ListItemIcon>
            <img
              className={classes.courseimg}
              src="../../../public/images/book.png"
            />
          </ListItemIcon>
          <ListItemText primary={props.coursevals[currentKey].courseId} />
          <img
            className={classes.rightarrow}
            src="../../../public/images/rightarrow.png"
          />
        </ListItem>
      );
    }
    return indents;



 }

@MuhammadZulqarnainSabir great! See my las edit for more info. — Ernesto Stifano, Sep 03 '19 at 13:25

score 0 · Answer 2 · answered Sep 03 '19 at 13:10

0

I think you forgot to pass the event as the first parameter

onClick={(event) => handleListItemClick(event, key)}

And your handleListItemClick function should be declared as

function handleListItemClick(event, index) {
    event.preventDefault()
    ...
}

answered Sep 03 '19 at 13:10

Peter B

144
1
8

still getting the same results, i have two items in the array and on clicking item 0 , the functions prints index 1 on console – Muhammad Zulqarnain Sabir Sep 03 '19 at 13:14
I would use the array.map function because it seems something is amiss with your key for (var key in props.coursevals) { becomes ``` props.coursevals.map((value, index) => { ... your code here ... }) ``` – Peter B Sep 03 '19 at 13:18

score 0 · Answer 3 · answered Sep 03 '19 at 13:14

Can you please use map instead of for loop. So your code should be look like below.

props.coursevals.map((courseval, index) => {
  indents.push(
    <ListItem
      key={index}
      button
      selected={selectedIndex === index}
      onClick={() => handleListItemClick(index)}
      className={classes.ListItem}
    >
      <ListItemIcon>
        <img
          className={classes.courseimg}
          src="../../../public/images/book.png"
        />
      </ListItemIcon>
      <ListItemText primary={courseval.courseId} />
      <img
        className={classes.rightarrow}
        src="../../../public/images/rightarrow.png"
      />
    </ListItem>
  );
});

Hope this will help you!

Can you please mark an answer as accepted and vote up for this answer? So this will helpful to other developers as well. — , Sep 03 '19 at 13:24

score 0 · Answer 4 · answered Sep 03 '19 at 13:16

It has nothing to do with React but Javascript itself. Beware when using var in for loops with callback functions. All these functions point to the same value, which is the last index at the end of iteration. Try using let instead of var or even better: don't use for loop at all:

const indents = Object.keys(props.coursevals).map(key => (
  <ListItem
      key={index}
      button
      selected={selectedIndex === index}
      onClick={() => handleListItemClick(index)}
      className={classes.ListItem}
    >
    ...

You can refer to this question: JavaScript closure inside loops – simple practical example

Material UI | React | OnClick Event always sends the last index

4 Answers4