-
Notifications
You must be signed in to change notification settings - Fork 598
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
Handle onAction for DropdownMenu Items #1194
Conversation
🦋 Changeset detectedLatest commit: 47e5770 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/5p3CbLaaByYaxnsdwGycw98fvLo9 |
e90fa6e
to
a6d24f9
Compare
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.
Looks great! Basically parallel to the other work and I think the places where you just inlined the callback make the code cleaner that what I had 👍
* Action Menu can be a controlled component with open state * update docs for ActionMenu * Create pink-lions-suffer.md * Handle onAction for DropdownMenu Items (#1194) * useProvidedStateOrCreate * linter fixes * update hook doc comment * Update src/hooks/useProvidedStateOrCreate.ts Co-authored-by: Dusty Greif <dgreif@users.noreply.github.com> * useProvidedOrCreateState has better implementation for generics * useProvidedOrCreateState has better implementation for generics * make string type explicit Co-authored-by: Dusty Greif <dgreif@users.noreply.github.com>
Refactoring
DropdownMenu
to use the newonAction
callback fromActionList.Item
, rather than handling click/key press manually. Also allowingitem.onAction
to callevent.preventDefault()
, which will now preventDropdownMenu
from closing and triggeringonChange
Merge checklist