Skip to content

Comments

fix: allow modifier keys for import links in package code#1309

Merged
danielroe merged 1 commit intonpmx-dev:mainfrom
bluwy:modifier-import-link
Feb 10, 2026
Merged

fix: allow modifier keys for import links in package code#1309
danielroe merged 1 commit intonpmx-dev:mainfrom
bluwy:modifier-import-link

Conversation

@bluwy
Copy link
Contributor

@bluwy bluwy commented Feb 10, 2026

follow-up to #1307 (comment)

I copied the conditions from vue-router: https://github.com/vuejs/router/blob/49b925d2042d2b9e8a75dc8eeb2578045a07d6c7/packages/router/src/RouterLink.ts#L414

I dont think we need to copy its other guards for now 🤔

@vercel
Copy link

vercel bot commented Feb 10, 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 10, 2026 2:34am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 10, 2026 2:34am
npmx-lunaria Ignored Ignored Feb 10, 2026 2:34am

Request Review

@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

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

Files with missing lines Patch % Lines
app/components/Code/Viewer.vue 0.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

📝 Walkthrough

Walkthrough

The pull request modifies the import link click handler in the Code Viewer component. It introduces type safety by specifying the return type for the DOM query selector as HTMLAnchorElement and adds a conditional guard that prevents click handling when modifier keys (meta, alt, control, or shift) are pressed, allowing standard browser behaviour to occur instead.

Possibly related PRs

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly references a follow-up discussion and cites specific implementation details from vue-router, directly relating to the modifier key handling changes in the code.

✏️ 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/components/Code/Viewer.vue (1)

61-75: Let non‑left clicks pass through to preserve native new‑tab behaviour.
The guard only checks modifier keys, so middle‑clicks are still intercepted and can’t open in a new tab. Add a non‑primary button (and optional defaultPrevented) guard before preventing default.

Proposed change
-    anchor.addEventListener('click', event => {
-      if (event.metaKey || event.altKey || event.ctrlKey || event.shiftKey) return
+    anchor.addEventListener('click', event => {
+      if (event.defaultPrevented) return
+      if (event.button !== 0) return
+      if (event.metaKey || event.altKey || event.ctrlKey || event.shiftKey) return

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

@danielroe danielroe added this pull request to the merge queue Feb 10, 2026
Merged via the queue into npmx-dev:main with commit 8ca360f Feb 10, 2026
17 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