How do I get access to the item that I clicked on (so that I can delete it)?

Question

I don't understand how I'm to use the deleteItem function to delete the item in our items array that was clicked on.

Where I'm stuck is that I don't understand how to grab the index or text of the <li> that I clicked on so that I can then use that in my filter method to return a new array without the item that was clicked on.

I tried using props but that doesn't work. I don't understand how the deleteItem function relates to the todoitem component and the rest of it.

import React, { useState } from "react";
import ToDoItem from "./ToDoItem";

function App() {
  const [inputText, setInputText] = useState("");
  const [items, setItems] = useState([]);

  function handleChange(event) {
    const newValue = event.target.value;
    setInputText(newValue);
  }

  function addItem() {
    setItems((prevItems) => {
      return [...prevItems, inputText];
    });
    setInputText("");
  }

  function deleteItem() {
    // setItems((prevState) => {
    //   return prevState.filter(function (item) {
    //     return item === props.item;
    //   });
    // });
    // console.log(items[2]);
    console.log();
  }

  return (
    <div className="container">
      <div className="heading">
        <h1>To-Do List</h1>
      </div>
      <div className="form">
        <input onChange={handleChange} type="text" value={inputText} />
        <button onClick={addItem}>
          <span>Add</span>
        </button>
      </div>
      <div>
        <ul>
          {items.map((todoItem, index) => (
            <ToDoItem
              key={index}
              id={index}
              item={todoItem}
              deleteItem={deleteItem}
            />
          ))}
        </ul>
      </div>
    </div>
  );
}

export default App;

import React from "react";

function ToDoItem(props) {
  return <li onClick={props.deleteItem}>{props.item}</li>;
}

export default ToDoItem;

Does this answer your question? [React js onClick can't pass value to method](https://stackoverflow.com/questions/29810914/react-js-onclick-cant-pass-value-to-method) — Emile Bergeron, Jun 24 '21 at 00:16
As for deleting, there are lots of answers in this question: [Delete item from state array in react](https://stackoverflow.com/q/36326612/1218980) — Emile Bergeron, Jun 24 '21 at 00:19

Ryan Wheale · Answer 1 · 2021-06-24T16:40:06.187

1

I always do this - it's a little more code, but you don't end up with a separate delete function for every item or have to deal with crawling the DOM tree (parentNode and friends).

Put the item index on a data attribute:
```
<li data-item-index={props.id} ...>
```

In the click handler, get the index of the item and remove it:

function deleteItem(ev) {
    const itemIndex = parseInt(ev.target.dataset.itemIndex, 10);
    setItems([
        ...items.slice(0, itemIndex), 
        ...items.slice(itemIndex + 1)
    ]);
}

EDIT: while out of the scope of this question, it's worth mentioning the following:

Always try to avoid onClick={() => deleteItem(props.id)} style callbacks, especially when rendering a list of items. Those are useful for quick prototyping, online examples, et al - but in a real application you should almost always do something like the following:
```
const deleteItem = useCallback(() => { ... }, []);
<li onClick={deleteItem}>...</li>
```
NOTE: this is not "premature optimization", it's an extra set of keystrokes you use every time you need a callback.

To avoid breaking some dogmatic rules of computer science, you should create helper functions for setting and retrieving the values. Putting it all together in a much too wordy answer:

const setItemIdAttr = (id) => ({ 'data-item-id': id });
const getItemIdFromAttr = (el) => parseInt(el.dataset.itemId, 10);

const deleteItem = useCallback((ev) => {
    const itemIndex = getItemIdFromAttr(ev.target);
    setItems((oldItems) => [
        ...oldItems.slice(0, itemIndex), 
        ...oldItems.slice(itemIndex + 1)
    ]);
}, []);

<li {...setItemIdAttr(id)} onClick={deleteItem} ...>

edited Jun 24 '21 at 16:40

answered Jun 23 '21 at 19:54

Ryan Wheale

26,022
8
76
96

Oh wow okay thanks very much for this! – Mark D. Foti Jun 23 '21 at 20:05
While avoiding `parentNode` is a good thing, this answer is suggesting to break the Law of Demeter, where the parent component assumes the child component implementation and violates its internals to snoop the index from the data attribute. While not a huge issue in practice, it's done to avoid passing a new function to each item, intending to optimize rendering, but still end up passing a new function to each item every render since the `deleteItem` function is always re-created, completely defeating the initial goal. – Emile Bergeron Jun 23 '21 at 23:26
In the end, it would be easier to just pass `
props.deleteItem(props.id)}>` and avoid early optimization.

Emile Bergeron

Jun 23 '21 at 23:26

Then, if a bottleneck is (ever) measured and the cause is the new `onClick` callback, a simple `useCallback` in the `ToDoItem` would solve it. – Emile Bergeron Jun 23 '21 at 23:46

Emile - `useCallback` in the TodoItem would **not** solve the problem of 1000 items --> 1000 callbacks. The technique I described is 1 callback for 1000 items. As it is, the `deleteItem` callback is regenerated on every render, which is fixed by `useCallback`, but thats outside the scope of this question. === If you are really worried about breaking some deep theory law, then create helper functions for generating setting and retrieving the value. Whatever you do, ALWAYS try to avoid `onClick={() => deleteItem(id)}` style of callbacks - it's terribly inefficient... even with `useCallback`. – Ryan Wheale Jun 24 '21 at 16:13

pierre-lgb · Accepted Answer · 2021-06-23T19:53:14.093

You can access the li item clicked with event.target :

function deleteItem(event) {
    const item = event.target
    console.log("Item :", item);
    console.log("Index :", Array.from(item.parentNode.children).indexOf(item));
}

function ToDoItem(props) {
  return <li onClick={props.deleteItem}>{props.item}</li>;
}

function App() {
  const [inputText, setInputText] = React.useState("");
  const [items, setItems] = React.useState([]);

  function handleChange(event) {
    const newValue = event.target.value;
    setInputText(newValue);
  }

  function addItem() {
    setItems((prevItems) => {
      return [...prevItems, inputText];
    });
    setInputText("");
  }

  function deleteItem(event) {
    const listItem = event.target
    const listItemIndex = Array.from(listItem.parentNode.children).indexOf(listItem)
    setItems((prevState) => {
      return prevState.filter(function (item, index) {
        return index !== listItemIndex
      });
    });
    console.log("Item :", listItem);
    console.log("Index :", listItemIndex);
  }

  return (
    <div className="container">
      <div className="heading">
        <h1>To-Do List</h1>
      </div>
      <div className="form">
        <input onChange={handleChange} type="text" value={inputText} />
        <button onClick={addItem}>
          <span>Add</span>
        </button>
      </div>
      <div>
        <ul>
          {items.map((todoItem, index) => (
            <ToDoItem
              key={index}
              id={index}
              item={todoItem}
              deleteItem={deleteItem}
            />
          ))}
        </ul>
      </div>
    </div>
  );
}

ReactDOM.render(<App />, document.querySelector('#app'))

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.0/umd/react-dom.production.min.js"></script>
<div id="app"></div>

I've edited the code snippet so now it deletes the item when you click on it — pierre-lgb, Jun 23 '21 at 19:54

score -1 · Answer 3 · answered Jun 23 '21 at 20:11

-1

This is how i'd go about this, since you don't have an id on your items, just passing the index should work out

function deleteItem(index) {
    let aux = items;
    items.splice(index, 1);
    setItems([...aux]);
}

 <ToDoItem
    key={index}
    id={index}
    item={todoItem}
    deleteItem={deleteItem.bind(this, index)}
 />

answered Jun 23 '21 at 20:11

Gustavo Farias

393
5
15

Just be aware when using this technique that every single item will get its own copy of the `deleteItem` function because `.bind` creates a new function. If you have 1000 items, you will create 1000 copies of `deleteItem`. The same thing applies if you were to do `deleteItem={() => deleteItem(index)}`. If you want to avoid unnecessary copies of functions, then check out my answer above. – Ryan Wheale Jun 23 '21 at 20:48
`splice` [mutates the current state](https://stackoverflow.com/q/37755997/1218980). – Emile Bergeron Jun 23 '21 at 23:15

How do I get access to the item that I clicked on (so that I can delete it)?

3 Answers3