Arrow function should not use as props in Onclick event

[reetesha]

I have below 3 suggestion to this on JSX onClick Events -

1. Actually, we don't need to use .bind() or Arrow function in our code. You can simple use <ul onClick={this.handleClick}> in your code.

2. You can also move onClick event from th(or ul) to tr(or li) to improve the performance. Basically you will have n number of "Event Listeners" for your n li element.

    So finally code will look like this:
   

        <ul onClick={this.onItemClick}>
            {this.props.items.map(item =>
                   <li key={item.id} data-itemid={item.id}>
                       ...
                   </li>
              )}
        </ul>
    
    // And you can access `item.id` in `onItemClick` method as shown below: 
    
        onItemClick = (event) => {
           console.log(e.target.getAttribute("item.id"));
        }

3. I agree with the approach mention above for creating separate React Component for ListItem and List. This make code looks good however if you have 1000 of li then 1000 Event Listeners will be created. Please make sure you should not have much event listener.

        import React from "react";
        import ListItem from "./ListItem";
        export default class List extends React.Component {
    
            /**
            * This List react component is generic component which take props as list of items and also provide onlick
            * callback name handleItemClick
            * @param {String} item - item object passed to caller
            */
            handleItemClick = (item) => {
                if (this.props.onItemClick) {
                    this.props.onItemClick(item);
                }
            }
    
            /**
            * render method will take list of items as a props and include ListItem component
            * @returns {string} - return the list of items
            */
            render() {
                return (
                    <div>
                      {this.props.items.map(item =>
                          <ListItem key={item.id} item={item} onItemClick={this.handleItemClick}/>
                      )}
                    </div>
                );
            }
    
        }
    
    
        import React from "react";
    
        export default class ListItem extends React.Component {
            /**
            * This List react component is generic component which take props as item and also provide onlick
            * callback name handleItemClick
            * @param {String} item - item object passed to caller
            */
            handleItemClick = () => {
                if (this.props.item && this.props.onItemClick) {
                    this.props.onItemClick(this.props.item);
                }
            }
            /**
            * render method will take item as a props and print in li
            * @returns {string} - return the list of items
            */
            render() {
                return (
                    <li key={this.props.item.id} onClick={this.handleItemClick}>{this.props.item.text}</li>
                );
            }
        }

[navyjax2]

Linting error occurs if I do render() { return ( <li key={this.props.item.id} onClick={this.handleItemClick}>{this.props.item.text}</li> ); } It will say: "Visible, non-interactive elements should not have mouse or keyboard event listeners jsx-a11y/no-static-element-interactions" The function will still work, but seems like the linting rules are saying there's supposed to be another way to do this?

[ljharb]

That’s unrelated to the topic of this PR; but yes, it’s saying you shouldn’t put an onClick on an <li>

[navyjax2]

Right, well, more than just on an <li>, but on any DOM object, I've found. How is it unrelated? Seems all of the code samples here all have it - you're using onClick on an element here, aren't you? The only way I found out of the mess was to use <Link to={"\MyPage\" + item.ID}>item.ID</Link> - and set the routing to pick up the ID. I had no clue how to avoid using an onClick on a DOM element, when that element needs a click to make a handler do its thing! But because we're using this linting that apparently Facebook and UPS and others also use, seems like it would be applicable to the conversation - because you guys are saying "put this onClick in here" (it's literally in all 3 options on the first comment on this thread by Reetesha), but the linting rules we're following at my job are saying not to. Was hoping to see what the professionals like you guys thought the rule was trying to say to do/use, instead - because all the code samples I've seen so far stick an onClick in the element, and if we're trying to improve the framework, it seems like we should start by following the rules, shouldn't we?

[ljharb]

<button> and <a> can certainly have an onClick - the purpose of that linting rule is to limit click behavior to things that are inherently clickable.

[navyjax2]

Thanks for that clarification. Seems they're rather strict on what can have an onClick, then, but at least now I know.