The Goal
I am wrapping a list of message items with a component 'ListWrap'.This component is designed to wrap each item in the list and detect when the children items are added or removed from the list. It displays all items even if removed from the parent components list and just changes the color of the wrapper red for removed or green for added.
There is a greater purpose for this, but for demonstrating the problem I am simply adjusting the wrapper color.
The Problem
The problem Im facing is that when removing items that are not the last item in the list, it also removes all the subsequent items. So for example from 3 items, if item 2 is selected to be removed, then item 2 and 3 are removed.
Im not sure why this is because when removing the ListWrap component the list functions fine. When using ListWrap and passing the prop children directly to the return statement it works fine. But when I attempt to return a modified selection of children then the issue occurs. I have noticed that the parent messages state is directly effected by this so it appears some logic on the child element is causing the deletion of the extra list items even though only one is selected to be deleted.
Example provided shows that when clicking on a items close/remove button, the expected behaviour is it should only make that wrapper red (left colored box), not all the items below.
Test.js
import React, { useState } from "react";
import ListWrap from "./ListWrap";
const Test = () => {
const [messages, setMessages] = useState([]);
const addMessage = () => {
const id = Date.now();
setMessages([...messages, { id: id, text: `Message ${id}` }]);
};
const removeMessage = (id) => {
setMessages(messages.filter((message) => message.id !== id));
};
return (
<div className="container mx-auto p-4">
<div className="mb-4 flex justify-between">
<button
onClick={addMessage}
className="rounded bg-green-500 px-4 py-2 text-white hover:bg-green-600 focus:outline-none"
>
Add New
</button>
<button
onClick={() => setMessages(messages.slice(0, -1))}
className="rounded bg-orange-500 px-4 py-2 text-white hover:bg-orange-600 focus:outline-none"
>
Remove Last
</button>
<button
onClick={() => setMessages([])}
className="rounded bg-red-500 px-4 py-2 text-white hover:bg-red-600 focus:outline-none"
>
Remove All
</button>
</div>
<div className="space-y-2">
<ListWrap>
{messages.map((message) => (
<div
key={message.id}
className="relative rounded bg-gray-500 p-4 text-white shadow-lg"
>
<p>{message.text}</p>
<button
onClick={() => removeMessage(message.id)}
className="absolute right-0 top-0 mr-2 mt-2 rounded bg-red-500 px-2 py-1 text-white hover:bg-red-600 focus:outline-none"
>
X
</button>
</div>
))}
</ListWrap>
</div>
</div>
);
};
export default Test;
ListWrap.js
import React, { useState, useEffect } from "react";
const ListWrap = ({ children }) => {
const [allChildrenItems, setAllChildrenItems] = useState([]);
useEffect(() => {
const newChildren = React.Children.toArray(children);
const newChildrenKeys = newChildren.map((child) => child.key);
const currentChildrenKeys = allChildrenItems.map((item) => item.child.key);
const addedChildrenKeys = newChildrenKeys.filter(
(key) => !currentChildrenKeys.includes(key)
);
const removedChildrenKeys = currentChildrenKeys.filter(
(key) => !newChildrenKeys.includes(key)
);
const updatedChildrenItems = [
...allChildrenItems.map((item) => {
if (removedChildrenKeys.includes(item.child.key))
return { ...item, isActive: false };
return item;
}),
...newChildren
.filter((child) => addedChildrenKeys.includes(child.key))
.map((child) => ({ child, isActive: true })),
];
setAllChildrenItems(updatedChildrenItems);
}, [children]);
//Using a custom item based on the children data IS causing issue with parent messages state:
return (
<div>
{allChildrenItems.map((item) => (
<div key={item.child.key}>
<div
className={
item.isActive
? "mb-2 pl-10 z-50 bg-green-500"
: "mb-2 pl-10 z-50 bg-red-500"
}
>
{item.child}
</div>
</div>
))}
</div>
);
//Using children directly from prop NOT causing issue with parent messages state:
// return (
// <>
// {children.map((item) => (
// <div key={item.key}>
// <div className={item.isActive ? "active" : "inactive"}>{item}</div>
// </div>
// ))}
// </>
// );
// };
};
export default ListWrap;
You store stale versions of the children, which use a stale version of the removeMessage callback, which keep a stale version of the messages list. Therefore when removing one message, removeMessage uses an old messages list, which lacks next messages.
You should be good by fixing one of these 2 issues, although you could do both for safer practice:
When computing the new messages state value, do not start from its "current" value (which may actually be stale), but from the "previous" value that React will provide to the updater function:
const removeMessage = (id) => {
setMessages(
// Use an updater function, which will receive the most up-to-date "previous" state value
(previousMessages) => previousMessages.filter((message) => message.id !== id)
);
};
When computing the new allChildrenItems state, make sure to use the new children items, instead of re-using the old ones:
const updatedChildrenItems = [
...allChildrenItems.map((oldItem) => {
const child = newChildren.find((child) => child.key === oldItem.child.key)
const item = child ? {
child,
isActive: true
} : oldItem; // In case it is removed
// etc.
),
// etc.
];