skip to Main Content

I have an array with components, each component has a handleRemove function that should remove a component from the array, but it does not work quite correctly, instead of deleting the selected element, it deletes the selected element and subsequent ones.

Let me give you an example, let’s say there is an array with the numbers [1, 2, 3, 4, 5], if you try to remove 3, then [1, 2] will remain in the array.

App.tsx:

function App() {
  const [categoryArray, setCategoryArray] = useState<ReactElement[]>([]);

  const handleAdd = () => {
    const newIndex = categoryArray.length;
    setCategoryArray((prevArray: ReactElement[]) => [
      ...prevArray,
      <Category
        key={Math.floor(Math.random() * 100001)}
        onDelete={() => handleRemove(newIndex)}
      />,
    ]);
  };


//HANDLE REMOVE FUNCTION
  const handleRemove = (index: number) => {
    let newArray = [...categoryArray];
    newArray.splice(index, 1);
    setCategoryArray(newArray);
  };

  return (
    <div className="App">
      <div className="categories">
        <div className="categories-block">
          <p>Categories</p>
        </div>
        <p className="plus" onClick={handleAdd}>
          +
        </p>
      </div>
      <div className="subcategories">
        {categoryArray.map((item, index) => item)}
      </div>
    </div>
  );
}

export default App;

Category.tsx:

import React, { useState, ReactElement } from "react";
import "../css/category.css";

interface IProps {
  onDelete: () => void;
}

function Category({ onDelete }: IProps) {

//This is responsible for rendering the buttons, ignore it
  const [categoryName, setCategoryName] = useState<string>("");
  const [isCategoryVisible, setCategoryVisible] = useState<boolean>(false);
  const [inputActive, setInputActive] = useState<boolean>(true);
  const [editActive, setEditActive] = useState<boolean>(false);
  const [currentName, setCurrentName] = useState<string>("");

  const save = () => {
    setCurrentName(categoryName);
  };

  const handleAddCategory = () => {
    if (categoryName.trim() !== "") {
      setCategoryVisible(true);
      setInputActive(false);
    }
    save();
  };

  const handleEdit = () => {
    setCategoryVisible(false);
    setInputActive(true);
    setEditActive(true);
  };

  const handleAply = () => {
    setEditActive(false);
    setCategoryVisible(true);
    setInputActive(false);
    save();
  };

  const handleCancel = () => {
    setCategoryVisible(true);
    setInputActive(false);
    setEditActive(false);
  };
//This is responsible for rendering the buttons, ignore it

  return (
    <div className="Category">
      <div className="category">
        <div className="category-block">
          <div className="category-text">
            <p className={isCategoryVisible ? "name-active" : "name"}>
              {currentName}
            </p>
            <input
              type="text"
              onChange={(e) => setCategoryName(e.target.value)}
              placeholder="Category name"
              className={inputActive ? "" : "input-disable"}
            />
          </div>
        </div>
        <div
          className={
            inputActive && !editActive
              ? "minus-plus-active"
              : "minus-plus-disable"
          }
        >
          <p className="minus">-</p>
          <p className="plus" onClick={handleAddCategory}>
            +
          </p>
        </div>
        <div
          className={editActive ? "cancel-aply-active" : "cancel-aply-disable"}
        >
          <p className="aply cancel-aply-item" onClick={handleAply}>
            ✔
          </p>
          <p className="cancel cancel-aply-item" onClick={handleCancel}>
            x
          </p>
        </div>
        <div className={inputActive ? "edit-menu" : "edit-menu-active"}>
          <p className="plus edit-menu-item">+</p>
          <p className="edit edit-menu-item" onClick={handleEdit}>
            E
          </p>
//function triggered on click here
          <p className="delete edit-menu-item" onClick={onDelete}>
            X
          </p>
        </div>
      </div>
    </div>
  );
}

export default Category;

The function seems to be written correctly, but I sincerely don’t understand what’s wrong, I would be very grateful if you could help me figure it out.

3

Answers


  1. In my opinion, the current structure appears to be an anti-pattern, and I would recommend choosing a more suitable structure to perform this task.

    To explain the cause of this problem, it’s because, with each call of the handleAdd function, a new onDelete function is generated, and the state’s value is associated with the moment when the onDelete function is created.

    In fact, when you have 5 elements, and you click on the third element, the ‍newArray value in the onDelete function contains three elements.

    To access the most up-to-date value within the onDelete function, you should use the setState callback, as demonstrated below.

    const handleRemove = (index: number) => {
      setCategoryArray((prevState) => {
        const newArray = [...prevState];
        newArray.splice(index, 1);
        return [...newArray];
      });
    };
    

    A Better Approach

    I strongly recommend using a different structure to perform this task. Instead of storing ReactElement in the categoryArray state, it would be better to assign a unique ID to each category and then add this ID to the categoryArray state. This approach can help prevent potential future bugs.

    Example

    
    import { v4 } from "uuid";
    
    
    function App() {
    const [categoryArray, setCategoryArray] = useState<{ id: string }[]>([]);
    
    const handleAdd = () => {
      setCategoryArray((prevArray) => {
        const newValue = [...prevArray];
        newValue.push({ id: v4() });
        return [...newValue];
      });
    };
    
    const handleRemove = (id: string) => {
      setCategoryArray((prevState) => {
        return [...prevState].filter((value) => value.id !== id);
      });
    };
    
    
    return (
      <div className="App">
        <div className="categories">
          <div className="categories-block">
            <p>Categories</p>
          </div>
          <p className="plus" onClick={handleAdd}>
            +
          </p>
        </div>
        <div className="subcategories">
          {categoryArray.map((item, index) => (
            <Category onDelete={() => handleRemove(item.id)} />
          ))}
        </div>
      </div>
    );
    }
    
    export default App;
    
    Login or Signup to reply.
  2. There is a problem with handleRemove as mentioned in the previous answer. Use the setState with callback as it mentions.

    However there is also a problem with the reliance on indices in handleAdd. Say you add 3 categories, the index situation is now [0, 1, 2]

    Removing the element at 0 causes a problem, since the array now has indices [1, 2] and trying to remove index 2 does nothing.

    To solve this you must update indices as you remove elements from the array. Alternatively you can use a map (pseudo code):

      const [categoryMap, setCategoryMap] = useState<Record<number, ReactElement>({});
    
      const handleAdd = () => {
        setCategoryMap((prevMap: Record<number, ReactElement>) => {
          ...prevMap,
          [Math.random() * 100001)]: <Category
            key={Math.floor(Math.random() * 100001)}
            onDelete={() => handleRemove(newIndex)}
          />
        });
      };
    
      const handleRemove = (key: number) => {
        setCategoryMap((prevMap) => delete prevMap[key]);
      };
    
      {Object.values(categoryMap).map(...)}
    

    I will stress that this approach is still an anti-pattern. You should probably not store react elements this way. Instead consider storing the content of each element and using it to render Category.

    Login or Signup to reply.
  3. I think you probably need an if statement to check the index and the length of your array before you remove an element.

    Here is how you can handle it.

    const handleRemove = (index: number) => {
      if (index >= 0 && index < categoryArray.length) {
        setCategoryArray(priv=> priv.filter((_, i) => i !== index));
      }
    };
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search