Fix: Use j/k instead of arrow keys for page navigation - #3165
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughSearch result keyboard navigation now uses ChangesSearch navigation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hello! Thank you for opening your first PR to npmx, @elracing! 🚀 Here’s what will happen next:
|
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/pages/search.vue`:
- Around line 522-524: Update the keyboard handling around
getSearchResultNavigationDirection so events with Ctrl, Meta, or Alt modifiers
return before calling preventDefault or performing custom navigation. Continue
allowing Shift-modified j/k events for uppercase J/K handling.
- Around line 495-497: Update the keyboard shortcut guard in the search page’s
keydown handler so Enter events from the search input can reach the existing
input-Enter handling, while retaining the early return for other editable
elements and disabled shortcuts. Preserve the exact-match and deferred
navigation behavior implemented by the downstream Enter case.
- Line 565: Update the search result shortcut registration in onKeyDown to
include uppercase J and K alongside the existing j, k, and Enter keys, ensuring
getSearchResultNavigationDirection can receive uppercase navigation input. Add
regression coverage that dispatches key values J and K and verifies they reach
handleResultsKeydown.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d8ba0bd1-de0e-458d-aa32-4351740c9ca8
📒 Files selected for processing (3)
app/pages/search.vueapp/utils/search-navigation.tstest/unit/app/utils/search-navigation.spec.ts
| if (!keyboardShortcuts.value || isEditableElement(e.target)) { | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the search-input Enter flow reachable.
An Enter keydown from the search input has that input as e.target. isEditableElement(e.target) then returns true, so this guard exits before the input Enter handling at Line 500. Exact-match navigation and deferred navigation after results arrive no longer run.
Allow the existing input Enter case through this guard.
Proposed fix
- if (!keyboardShortcuts.value || isEditableElement(e.target)) {
+ const isInputEnter =
+ e.key === 'Enter' && document.activeElement?.tagName === 'INPUT'
+ if (!keyboardShortcuts.value || (isEditableElement(e.target) && !isInputEnter)) {
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!keyboardShortcuts.value || isEditableElement(e.target)) { | |
| return | |
| } | |
| const isInputEnter = | |
| e.key === 'Enter' && document.activeElement?.tagName === 'INPUT' | |
| if (!keyboardShortcuts.value || (isEditableElement(e.target) && !isInputEnter)) { | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/pages/search.vue` around lines 495 - 497, Update the keyboard shortcut
guard in the search page’s keydown handler so Enter events from the search input
can reach the existing input-Enter handling, while retaining the early return
for other editable elements and disabled shortcuts. Preserve the exact-match and
deferred navigation behavior implemented by the downstream Enter case.
| const direction = getSearchResultNavigationDirection(e.key) | ||
| if (direction) { | ||
| e.preventDefault() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not handle modified shortcut chords.
getSearchResultNavigationDirection(e.key) ignores modifier state. Ctrl, Meta, or Alt with j/k therefore enters this branch and calls preventDefault(). Keep Shift available for uppercase J/K, but return before custom handling when Ctrl, Meta, or Alt is active.
Proposed fix
+ if (e.metaKey || e.ctrlKey || e.altKey) return
+
const direction = getSearchResultNavigationDirection(e.key)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const direction = getSearchResultNavigationDirection(e.key) | |
| if (direction) { | |
| e.preventDefault() | |
| if (e.metaKey || e.ctrlKey || e.altKey) return | |
| const direction = getSearchResultNavigationDirection(e.key) | |
| if (direction) { | |
| e.preventDefault() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/pages/search.vue` around lines 522 - 524, Update the keyboard handling
around getSearchResultNavigationDirection so events with Ctrl, Meta, or Alt
modifiers return before calling preventDefault or performing custom navigation.
Continue allowing Shift-modified j/k events for uppercase J/K handling.
| } | ||
|
|
||
| onKeyDown(['ArrowDown', 'ArrowUp', 'Enter'], handleResultsKeydown) | ||
| onKeyDown(['j', 'k', 'Enter'], handleResultsKeydown) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)search\.vue$|package(-lock)?\.json$|pnpm-lock\.yaml$|yarn\.lock$' || true
echo "== outline search.vue =="
if [ -f app/pages/search.vue ]; then
wc -l app/pages/search.vue
ast-grep outline app/pages/search.vue --match onKeyDown --view compact || true
ast-grep outline app/pages/search.vue --match handleResultsKeydown --view compact || true
fi
echo "== relevant snippets =="
if [ -f app/pages/search.vue ]; then
sed -n '470,580p' app/pages/search.vue
fi
echo "== onKeyDown registrations and helper refs =="
rg -n "onKeyDown\\(|handleResultsKeydown|getSearchResultNavigationDirection|j|k|Enter" app/pages/search.vue || true
echo "== dependency versions =="
for f in package.json app/package.json; do
[ -f "$f" ] && { echo "--- $f"; jq -r '.dependencies, .devDependencies' "$f" 2>/dev/null | rg -i "vueuse|`@vueuse`" || cat "$f"; }
doneRepository: npmx-dev/npmx.dev
Length of output: 13424
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== getSearchResultNavigationDirection implementation =="
fd -a 'search-navigation\.(ts|js)$' . | while read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
done
echo "== onKeyDown source in installed npm package if present =="
node - <<'JS'
const path = 'node_modules/@vueuse/core/index.mjs'
try {
const fs = require('fs')
const p = require.resolve('`@vueuse/core`')
console.log('resolved:', p)
const m = fs.readFileSync(p, 'utf8')
const idx = m.indexOf('function onKeyDown')
const end = m.indexOf('\n\n', idx + 10)
console.log(m.slice(Math.max(0, idx - 300), Math.min(m.length, idx + 1500)))
} catch (err) {
console.log('not available:', err.message)
}
JS
echo "== package manager lock version entries =="
if [ -f pnpm-lock.yaml ]; then
rg -n '`@vueuse/core`|`@vueuse/shared`|`@vueuse/integrations`' pnpm-lock.yaml | head -80 || true
fiRepository: npmx-dev/npmx.dev
Length of output: 2837
🌐 Web query:
VueUse onKeyDown uppercase keys key as literal string documentation
💡 Result:
In VueUse, the onKeyDown function (and the related onKeyStroke utility) uses the KeyboardEvent.key property to perform key filtering [1][2]. When you pass a literal string to represent a key, you should use the exact string value provided by the browser's KeyboardEvent.key property [2]. For alphabetical keys, this value is case-sensitive and corresponds to the character produced by the key press [2]. For example: - To listen for the lowercase "a" key, you pass 'a' [2]. - To listen for the uppercase "A" key, you pass 'A' [2]. If you want to listen to both uppercase and lowercase versions of a key, you can pass an array of strings to the key parameter [2]. Example: import { onKeyDown } from '@vueuse/core' // Listens for both lowercase 's' and uppercase 'S' onKeyDown(['s', 'S'], (e) => { console.log('S or s was pressed') }) If you need more complex logic, such as ensuring a specific modifier key is held down (e.g., Shift+A), it is recommended to use a custom predicate function, which provides full access to the event object [2]: onKeyDown( (e) => e.key === 'A' && e.shiftKey, (e) => { console.log('Shift+A pressed') }) For a complete reference of standard key values (such as 'ArrowDown', 'Enter', etc.), refer to the MDN documentation for KeyboardEvent key values, as VueUse relies on these standard strings for its key filtering [2].
Citations:
- 1: https://github.com/vueuse/vueuse/blob/main/packages/core/onKeyStroke/index.ts
- 2: https://github.com/vueuse/vueuse/blob/main/packages/core/onKeyStroke/index.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for page-level tests mentioning search navigation keys =="
git ls-files | rg '(^|/)(\btest\b|tests?|__tests__|e2e|cypress|playwright|vitest|vite|nuxi)' || true
rg -n "Search|search|J|K|j|k|result navigation|getSearchResultNavigationDirection|handleResultsKeydown" . \
-g '!node_modules' -g '!pnpm-lock.yaml' \
-g '!**/dist/**' -g '!**/.output/**' \
| head -200
echo "== locate test files =="
git ls-files | rg '(test|spec|scenario|integration|e2e)' | head -200Repository: npmx-dev/npmx.dev
Length of output: 38004
Add uppercase J and K to the search result shortcut registration.
getSearchResultNavigationDirection supports J and K, but onKeyDown(['j', 'k', 'Enter'], handleResultsKeydown) filters those cases at the listener level, so uppercase navigation no longer reaches the page handler. Register both cases and add a regression test that dispatches key: 'J' and key: 'K'.
Proposed fix
-onKeyDown(['j', 'k', 'Enter'], handleResultsKeydown)
+onKeyDown(['j', 'J', 'k', 'K', 'Enter'], handleResultsKeydown)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onKeyDown(['j', 'k', 'Enter'], handleResultsKeydown) | |
| onKeyDown(['j', 'J', 'k', 'K', 'Enter'], handleResultsKeydown) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/pages/search.vue` at line 565, Update the search result shortcut
registration in onKeyDown to include uppercase J and K alongside the existing j,
k, and Enter keys, ensuring getSearchResultNavigationDirection can receive
uppercase navigation input. Add regression coverage that dispatches key values J
and K and verifies they reach handleResultsKeydown.
Source: MCP tools
1 participant
🔗 Linked issue
Resolves #2648
🧭 Context
The search results page was using arrow keys for custom result navigation, which conflicted with normal page scrolling and made the interaction feel inconsistent. This change updates the behavior so keyboard navigation uses j/k for moving between results while keeping Enter for activation.
📚 Description
This PR updates search result keyboard navigation to use j and k instead of the arrow keys. The new behavior makes the search experience more predictable and keeps regular arrow-key scrolling intact outside of this custom result-navigation flow.
I also extracted the mapping into a small helper and added regression coverage so the keyboard behavior stays protected going forward. The change includes unit tests for the new navigation mapping and key handling.