fix: refactor useTreeData #8125
Conversation
## API Changes
@react-stately/data/@react-stately/data:TreeData TreeData <T extends {}> {
append: (Key | null, Array<{}>) => void
+ getDescendantKeys: (TreeNode<{}>) => Array<Key>
getItem: (Key) => TreeNode<{}> | undefined
insert: (Key | null, number, Array<{}>) => void
insertAfter: (Key, Array<{}>) => void
insertBefore: (Key, Array<{}>) => void
move: (Key, Key | null, number) => void
moveAfter: (Key, Iterable<Key>) => void
moveBefore: (Key, Iterable<Key>) => void
prepend: (Key | null, Array<{}>) => void
remove: (Array<Key>) => void
removeSelectedItems: () => void
selectedKeys: Set<Key>
setSelectedKeys: (Set<Key>) => void
update: (Key, {}) => void
} |
| expect(result.current.items[1].key).toEqual('Eli'); | ||
| expect(result.current.items[2].key).toEqual('Suzie'); |
There was a problem hiding this comment.
I'm surprised that this has Eli before Suzie, shouldn't Suzie be right after John since it was the child of John?
There was a problem hiding this comment.
ah never mind, I see that moveBefore will extract the child item and move it onto the same level as its parent so it specifically matters in what order the user selects the items.
I wonder is that expected? Or would a user expect that the selection order shouldn't matter? Should the parent child hierarchy be preserved?
There was a problem hiding this comment.
heh, I came here to comment the same thing. I think this ordering is unexpected. The order of the keys passed in to moveBefore shouldn't not affect the end order of items. I think we can think of the moveBefore as accepting a key and a Set, in which order doesn't matter nor is it guaranteed. It's a little incidental that we happen to pass an array, I think that was a mistake, but we can't undo it because then it wouldn't match out other hook.
There was a problem hiding this comment.
I pulled down this branch's tests and ran it against the current useTreeData. Assuming the ordering issue is resolved as it was, then these are the only changes I found necessary
https://github.com/adobe/react-spectrum/compare/useTreeData-new-tests?expand=1
Sorry if I missed some conversation you had with the rest of the team. What is in this PR? are there more tests that are missing? I don't know what I'm supposed to be looking at while reviewing.
Testing to see if this fixes some of the issues found in testing. We'll need to add test cases for the issues we were observing.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: