-
Notifications
You must be signed in to change notification settings - Fork 536
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
fix(SelectPanel2): only bind keydown event when necessary #4601
Conversation
🦋 Changeset detectedLatest commit: f332218 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 |
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.
Hello @bwittenberg 👋🏻 Thanks for pushing this PR! It looks good to me 👍🏻 I'll just ping @siddharthkp since he has greater context on SelectPanel to see if there is any concern from him!
Hello @broccolinisoup! Appreciate the 👀. If you'd like to see any changes, I'm happy to make revisions. |
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 good to me!
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.
🎉
* fix(SelectPanel2): only bind keydown event when necessary * chore(SelectPanel2): Add changeset
Closes #4581
Changelog
New
jest
test coverage for theSelectPanel
Changed
SelectPanel
so that event listeners are not added and removed on each render, but only when the effect dependencies change.Removed
Rollout strategy
Testing & Reviewing
SelectPanel
jest test should fail if changes toSelectPanel
are reverted.SelectPanel
in storybook (http:///?path=/story/drafts-components-selectpanel--default)Escape
key on the keyboard, assert panel closesMerge checklist
- [ ] Added/updated documentation- [ ] Added/updated previews (Storybook)