react: why does the onclick get executed even without having clicked?

Question

Warning: Severe React beginner.

I have a class:

 export default class ItemsView extends React.Component {

    //...

    render() { 
      return (
        <div>
           <Container>
            <Grid container>
              <Grid item xs={4}>
                 <ul style={{listStyleType:"none"}}>
                   {   
                     this.state.items.map((item) => {
                     return <li key={item.number} style={{cursor: "pointer"}}><Item onClick={this.handleSelected(item)} value={item.timestamp}/></li>
                     })  
                   }   
                 </ul>
              </Grid>
              <Grid item xs={4}>
                <ItemDetail item={this.selected} />
              </Grid>
            </Grid>
          </Container>

          </div>
      )
   }
}

handleSelected(item) {
   console.log("handle");
   this.selected = item;
 }

What I want is that when I click on the Item div, which is rendered dynamically as a list of elements, the details of the item would appear in the ItemDetails component.

Apart from the fact that probably my design isn't really "React"y, why does the handleSelected get called when iterating, and not when I click on it?

you're executing the function with this.handleSelected(item), you should pass in a function like () => this.handleSelected(item) — Chris Li
– Chris Li, Commented Mar 2, 2020 at 22:01

John Ruddell · Accepted Answer · 2020-03-02 22:18:57Z

4

You are invoking the function rather than passing a function reference to be executed on click. You should either define the handler to return a function or use a lambda / fat arrow function in the click handler

onClick={() => this.handleSelected(item)}

remember, this.handleSelected is the functions reference. this.handleSelected() is the return value of an already invoked function.

edited Mar 2, 2020 at 22:18

John Ruddell

25.9k7 gold badges60 silver badges88 bronze badges

answered Mar 2, 2020 at 22:08

Vincenzo Ninni

1952 silver badges12 bronze badges

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

3 Comments

Brian Thompson Over a year ago

Code only answers aren't the best idea. An explanation of why your proposed code works would be much better and help future readers.

John Ruddell Over a year ago

@BrianThompson I helped him out :)

unsafe_where_true Over a year ago

This answers my question, thus I am gonna accept this one. @JohnRuddell 's extended answer was very helpful though to understand what I have to do!

John Ruddell · Accepted Answer · 2020-03-02 22:44:19Z

Aside from the answer Vincenzo posted, you need to also use component state here. A handler that updates a property on the class will not result in a new render cycle. I can see that you are using this.selected in the render as a prop to ItemDetail

<ItemDetail item={this.selected} />

This is problematic as changing the value of this.selected will not trigger a new render. This is a great use case to use component state

export default class ItemsView extends React.Component {
    state = { selected: null, items: [] }
    handleSelected = (selected) => (event) => {
// ----------------------^------------^----
// return a function in the handler to pass a callback to the click handler
      console.log("handle");
      this.setState({ selected });
// -----------^------------^----
// set the selected item on component state when clicked
    }

    //...

    render() { 
      return (
        <div>
           <Container>
            <Grid container>
              <Grid item xs={4}>
                 <ul style={{listStyleType:"none"}}>
                   { this.state.items.map((item) => <li key={item.number} style={{cursor: "pointer"}}>
                       <Item onClick={this.handleSelected(item)} value={item.timestamp}/>
// --------------------------------------------^---------------
// no change necessary here since the handler returns a function.
                     </li>
                   )}   
                 </ul>
              </Grid>
              <Grid item xs={4}>
                <ItemDetail item={this.state.selected} />
// --------------------------------------^---------------
// reference state here
              </Grid>
            </Grid>
          </Container>

          </div>
      )
   }
}

Thanks for this extensive answer, it brought me further on the wider issue....:)

John Ruddell · Accepted Answer · 2020-03-02 22:40:14Z

0

You are mapping over a list of items and want to pass the item to a handler callback (handleSelected in this case). You usually would pass that through by invoking the function with parenthesis (). However, the side effect of this is the function is immediately executed. To fix that you can place it inside an arrow function, allowing it to execute on the click instead.

Hence: onClick={() => this.handleSelected(item)}

edited Mar 2, 2020 at 22:40

John Ruddell

25.9k7 gold badges60 silver badges88 bronze badges

answered Mar 2, 2020 at 22:28

mxdi9i7

6974 silver badges15 bronze badges

Collectives™ on Stack Overflow

react: why does the onclick get executed even without having clicked?

3 Answers 3

3 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related