Skip to content

Comments

feat: ctrl+k to focus search#1433

Closed
ls-root wants to merge 1 commit intonpmx-dev:mainfrom
ls-root:feature/ctrl-k-focus-search
Closed

feat: ctrl+k to focus search#1433
ls-root wants to merge 1 commit intonpmx-dev:mainfrom
ls-root:feature/ctrl-k-focus-search

Conversation

@ls-root
Copy link
Contributor

@ls-root ls-root commented Feb 12, 2026

Make ctrl+k focus the search.
Show new keyboard shortcut in keyboard shortcuts modal:

ctrl+k keyboard shortcut in keyboard shortcuts modal

@vercel
Copy link

vercel bot commented Feb 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 12, 2026 2:57pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 12, 2026 2:57pm
npmx-lunaria Ignored Ignored Feb 12, 2026 2:57pm

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

This pull request introduces a focusSearchOrNavigate helper function in app/app.vue to consolidate search focus and navigation logic. The existing '/' keyboard shortcut is refactored to use this helper, and a new Ctrl+K shortcut is added to trigger the same behaviour. The keyboard shortcuts modal in app/components/AppFooter.vue is updated to display the new Ctrl+K entry. No public API changes are made.

Possibly related PRs

  • feat: surface kb shortcuts #1221: Introduced the keyboard shortcuts modal in AppFooter.vue that displays shortcut entries, which this PR extends with a new Ctrl+K shortcut display.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly describes the changes: adding Ctrl+K keyboard shortcut to focus search and displaying it in the keyboard shortcuts modal.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
app/app.vue (1)

73-82: Consider cross-platform support and case-insensitivity.

The current implementation only matches Ctrl+K. On macOS, users often expect Cmd+K (metaKey) for such shortcuts. Additionally, the key check is case-sensitive, which could behave unexpectedly with Caps Lock in edge cases.

This is a nice-to-have improvement and can be deferred if the intent is specifically Ctrl-only.

♻️ Optional enhancement for broader compatibility
 onKeyDown(
-  // Explicitly check Ctrl+K for broader compatibility (especially Firefox)
-  e => e.ctrlKey && e.key === 'k',
+  // Support Ctrl+K (Windows/Linux) and Cmd+K (macOS) for cross-platform compatibility
+  e => (e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k',
   e => {
     if (isEditableElement(e.target)) return
     e.preventDefault()
     focusSearchOrNavigate()
   },
   { dedupe: true },
 )

If you adopt metaKey support, you may also want to update the modal in AppFooter.vue to show platform-appropriate hints (e.g., "⌘K" on macOS).


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

❌ Patch coverage is 13.33333% with 13 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/app.vue 0.00% 10 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@whitep4nth3r
Copy link
Contributor

What's the reason for adding an extra keyboard shortcut that replicates existing functionality?

@danielroe
Copy link
Member

yes, we already have / to focus search 👍

@whitep4nth3r
Copy link
Contributor

Closing this in the interest of efficiency. Thanks!

@ls-root
Copy link
Contributor Author

ls-root commented Feb 13, 2026

Many documentation sites use ctrl+k as shortcut for search or command palette. Examples:

The goal here was to alaign with common UX expectations rather than duplicate functionality.

@whitep4nth3r
Copy link
Contributor

@danielroe thoughts?

@danielroe
Copy link
Member

I think we need to merge the command palette pr 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants