Skip to content
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 a bug in the useMnemonics hook to make it compatible with textareas #2317

Merged
merged 5 commits into from
Nov 1, 2022

Conversation

willglas
Copy link
Contributor

@willglas willglas commented Sep 7, 2022

Fix a bug in the useMnemonics hook that makes it incompatible with textarea elements. This bug manifested itself when I was trying to use a MarkdownEditor in an ActionMenu.

Closes #2316

Screenshots

No visual changes.

Merge checklist

  • Added/updated tests
  • Added/updated documentation (didn't seem necessary)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@willglas willglas requested review from a team and colebemis September 7, 2022 17:36
@changeset-bot
Copy link

changeset-bot bot commented Sep 7, 2022

🦋 Changeset detected

Latest commit: 304846b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

Copy link
Contributor

@Lukeghenco Lukeghenco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as a reviewer from the Pull Requests' React team who is using this component.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2022

size-limit report 📦

Path Size
dist/browser.esm.js 78.81 KB (+0.02% 🔺)
dist/browser.umd.js 79.45 KB (+0.02% 🔺)

@willglas willglas temporarily deployed to github-pages September 9, 2022 15:57 Inactive
Copy link
Member

@siddharthkp siddharthkp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good for useMnemonics!

However, I don't this is useful for your use case, 😅 left a comment on https://github.com/github/pull-requests/issues/5499#issuecomment-1243542876

@siddharthkp siddharthkp added react 💓collab a vibrant hub of collaboration labels Sep 12, 2022
@siddharthkp
Copy link
Member

Looks CI is failing (linting), we're good to go after that's fixed

@siddharthkp
Copy link
Member

👋 Hi! Just checking, is this still useful?

@willglas willglas temporarily deployed to github-pages October 31, 2022 15:58 Inactive
@willglas willglas temporarily deployed to github-pages October 31, 2022 16:09 Inactive
@siddharthkp siddharthkp enabled auto-merge (squash) November 1, 2022 14:10
@siddharthkp siddharthkp enabled auto-merge (squash) November 1, 2022 14:16
@siddharthkp siddharthkp merged commit a60f0f8 into main Nov 1, 2022
@siddharthkp siddharthkp deleted the willglas-fix-use-mnemonics-bug branch November 1, 2022 14:18
@siddharthkp siddharthkp temporarily deployed to github-pages November 1, 2022 14:18 Inactive
@primer-css primer-css mentioned this pull request Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💓collab a vibrant hub of collaboration react
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useMnemonics hook should ignore keydown events from textarea elements
4 participants