-
Notifications
You must be signed in to change notification settings - Fork 163
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: focus #1945
fix: focus #1945
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## qa #1945 +/- ##
=======================================
Coverage 90.76% 90.76%
=======================================
Files 247 247
Lines 54593 54593
=======================================
Hits 49553 49553
Misses 5040 5040 ☔ View full report in Codecov by Sentry. |
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.
On minor comment, but otherwise LGTM 👍
e.preventDefault(); | ||
focusGrid(); | ||
}} | ||
> |
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.
Minor, take it or leave it: I don't know if we'll ever have another use for this menu, in which case you could just put this on the underlying MenubarContent
shadcn component.
I did that with onEscapeKeyDown
so I wouldn't have to put it on every use of the component in the file menu.
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.
done
No description provided.