Skip to content

Conversation

@jhroemer
Copy link
Contributor

@jhroemer jhroemer commented Feb 3, 2026

#607 seems to have introduced a regression where the "?" keyboard shortcut no longer works (because the shift modifier is required on standard keyboards). @serhalp I considered if we should add a util for modifier keyboard combinations, but I'm not sure about it tbh. Both because afaik the key property should be robust in this case, but I'm generally also not fond of making abstractions when I do not yet have a pattern (like we only need this in a single instance currently). Wdyt?

@vercel
Copy link

vercel bot commented Feb 3, 2026

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

Project Deployment Actions Updated (UTC)
docs.npmx.dev Error Error Feb 3, 2026 10:02am
npmx.dev Ready Ready Preview, Comment Feb 3, 2026 10:02am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
npmx-lunaria Ignored Ignored Feb 3, 2026 10:02am

Request Review

@danielroe danielroe changed the title fix: shortcut highlight works again fix: for ? allow access with modifiers Feb 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

The pull request modifies the keyboard shortcut detection for the "?" key in the application. The implementation changes from using a helper function (isKeyWithoutModifiers) to a direct property check (e.key === '?'). A comment is added to document that the "?" character is typically produced with the Shift key. The functional behaviour remains unchanged, with the same preventDefault and keyboard hints enablement occurring. The change affects app.vue with two lines added and one removed, and includes a minor update to package.json.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the regression fix for the '?' keyboard shortcut and the decision to use the key property directly instead of a helper function.

✏️ 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

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

@danielroe danielroe added this pull request to the merge queue Feb 3, 2026
Merged via the queue into npmx-dev:main with commit 7c18e8f Feb 3, 2026
13 of 14 checks passed
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.

2 participants