fix(RAC): use the correct approach to retrieve the items array#9539
fix(RAC): use the correct approach to retrieve the items array#9539lixiaoyan wants to merge 4 commits intoadobe:mainfrom
Conversation
`collection.getKeys()` doesn’t guarantee the correct order.
reidbarber
left a comment
There was a problem hiding this comment.
Looks like there is a failing test.
Can you say more about what issue this was causing?
|
@reidbarber
|
| return itemNode?.type === 'item' ? itemNode : null; | ||
| } | ||
| ).filter(node => node !== null); | ||
| const cachedItemNodes = Array.from(collection).filter(itemNode => itemNode.type === 'item'); |
There was a problem hiding this comment.
This is probably the typo causing the test failure. This should be using cachedCollection.current.
There was a problem hiding this comment.
Yes, that was a typo. But technically, it shouldn't impact single-item deletes because only the cachedCollection length is used to gate multi-item deletion. I've applied the fix, but the test is still failing.
|
Thanks for the PR, I've opened a new one #9545 that fixes the test and adds a test instead of storybook so we are less likely to regress. There was a little more complexity to the keys in a collection with section and the logic was also flawed moving focus by all the items removed instead of only the ones before. I think we were getting lucky earlier with this test passing, though it was also just a really hard to follow test that did a bunch of things that wouldn't happen in real life. |
collection.getKeys()doesn’t guarantee the correct order.Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: