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
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 newonDelete
function is generated, and the state’s value is associated with the moment when theonDelete
function is created.In fact, when you have 5 elements, and you click on the third element, the
newArray
value in theonDelete
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.
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
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):
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
.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.