-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add moveBefore and moveAfter to useTreeData #7854
base: main
Are you sure you want to change the base?
Conversation
// we don't copy child node references from parents inadvertently | ||
if (keyArray.includes(child.key)) { | ||
removedItems.push({...newMap.get(child.key)!, parentKey: toParent?.key ?? null}); | ||
let {items: nextItems, nodeMap: nextMap} = updateTree(newItems, child.key, () => null, newMap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's an opportunity to optimize here if we rewrite updateTree a little so we don't generate a new array and new map for every single removal and instead batch the update. I've opted to get a working baseline based on existing functionality first though with tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
## API Changes
@react-stately/data/@react-stately/data:TreeData TreeData <T extends {}> {
append: (Key | null, Array<{}>) => void
getItem: (Key) => TreeNode<{}> | undefined
insert: (Key | null, number, Array<{}>) => void
insertAfter: (Key, Array<{}>) => void
insertBefore: (Key, Array<{}>) => void
items: Array<TreeNode<{}>>
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>
update: (Key, {}) => void
} |
Resolution on this @reidbarber please merge these changes into your PR and we'll just consider them as part of that. We can close this PR once you do that |
Closes #5574
Matches the API for useListData and takes a target key instead of an index to simplify moving items around. Also accepts an array of keys to move in bulk.
Includes an error check to make sure you don't try to move an item inside itself.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: