fix: focus restoring in collections#9545
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
devongovett
left a comment
devongovett
left a comment
There was a problem hiding this comment.
I'm a little worried about the performance of traversing two collections. Maybe there's a simpler algorithm we can use where we walk the old collection starting from the focused key until we find an item that exists in the new collection. Though if the items were also re-ordered at the same time as items were removed the results could be unexpected. How much do we care about that case?
| // Count how many items were removed before the focused item's original index | ||
| const itemNodesKeys = new Set(itemNodes.map(node => node.key)); | ||
| const removedBeforeCount = cachedItemNodes.filter((node, idx) => | ||
| idx < (startItem?.index ?? 0) && !itemNodesKeys.has(node.key) |
There was a problem hiding this comment.
I think with new collections startItem.index will be relative to the section, not the entire list so this comparison may not work. It would also not account for non-item siblings which would affect its index.
…-restore-in-collections
|
Pushed a change to use this simpler algorithm. We'll see if there are any real-world downsides. |
|
Build successful! 🎉 |
5 participants
Closes
As an alternative to #9539
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: