-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Feat: add shouldSelectOnPressUp prop to collection components #7922
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 shouldSelectOnPressUp prop to collection components #7922
Conversation
d4d3d1e to
b47c9f9
Compare
snowystinger
left a comment
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.
Thanks for the PR, would you mind adding tests in react-aria-components/test/GridList.test.js ?
I'd point you to ListBox's tests, but it seems it's tested implicitly by Menu, but it looks like there's no coverage for GridList.
If you don't have time, let us know.
Thank you!
|
@snowystinger Thank you for the reminder. I added some unit tests and verified the changes by following the test instructions included in the PR description. Please let me know if there are any suggestions |
snowystinger
left a comment
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.
Thanks so much! I added a story specifically for the use case to make sure that was resolved. It looks good!
|
Got it. Thank you @snowystinger! |
|
Doesn't look like this is currently a public prop in ListBox, only an option of the useListBox hook. Do we want to expose it on ListBox too? |
b43053e to
6db09e7
Compare
…shouldSelectOnPressUp-prop-to-GridList # Conflicts: # packages/react-aria-components/stories/GridList.stories.tsx # packages/react-aria-components/test/TagGroup.test.js
Closes #7887
shouldSelectOnPressUpprop to GridList following ListBoxshouldSelectOnPressUpto GridList story✅ Pull Request Checklist:
📝 Test Instructions:
react-aria-componentsmodule from the PR branch and set up a yarn linkreact-aria-componentsmodule in the StackBlitz projectshouldSelectOnPressUpprop astrueto GridList🧢 Your Project: