fix: arrow key navigation in search skips keyword buttons by serhalp · Pull Request #1704 · npmx-dev/npmx.dev

serhalp · 2026-02-27T14:07:53Z

🔗 Linked issue

🧭 Context

Pressing keyboard up/down in search results navigated through keyword filter buttons within each card instead of jumping between packages. This is unintuitive and bad for a11y.

📚 Description

Root cause: ButtonBase elements had their own data-result-index, so getFocusableElements() returned them alongside the actual result links.

npmx.search.results.kbd.navigation.mp4

Pressing ArrowDown/ArrowUp in search results navigated through keyword filter buttons within each card instead of jumping between packages. Root cause: keyword ButtonBase elements in Package/Card.vue had data-result-index, so getFocusableElements() returned them alongside the actual result links. Also adds ArrowUp-to-input (from first result) and Escape-to-input, plus E2E tests covering all three behaviors. Fixes #1078

vercel · 2026-02-27T14:07:58Z

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

Project	Deployment	Actions	Updated (UTC)
npmx.dev	Ready	Preview, Comment	Feb 27, 2026 8:51pm

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
docs.npmx.dev	Ignored	Preview	Feb 27, 2026 8:51pm
npmx-lunaria	Ignored		Feb 27, 2026 8:51pm

codecov · 2026-02-27T14:09:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

coderabbitai · 2026-02-27T15:12:19Z

📝 Walkthrough

Walkthrough

Removed the data attribute bound to the keyword index on the keyword filter button in app/components/Package/Card.vue. Added a helper focusSearchInput() in app/pages/search.vue and updated ArrowUp handling so pressing ArrowUp from the first or unfocused result returns focus to the header search input; ArrowUp otherwise moves focus to the previous result. Added two end-to-end tests validating ArrowDown/ArrowUp keyboard navigation between the search input and results.

Possibly related PRs

fix: improve a11y for search results #990: Modifies the same data-attribute binding on the keyword button in Package/Card.vue.
perf: search improvements #1431: Changes search-focused behaviour and input focus handling in search-related components.
refactor: replace links and buttons for compare and code page #1243: Refactors ButtonBase/LinkBase usage and touches ButtonBase instances similar to the changed template.

Suggested reviewers

danielroe
whitep4nth3r

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description clearly explains the problem, root cause, and solution, relating directly to the changeset.
Linked Issues check	✅ Passed	The changes successfully address the core requirements from issue `#1078`: arrow key navigation now jumps between packages instead of keyword buttons.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing arrow key navigation in search results; no unrelated modifications detected.

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

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch serhalp/fix-search-arrow-nav

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.

❤️ Share

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

app/pages/search.vue (1)
459-464: Make search-input targeting deterministic.

focusSearchInput() currently grabs the first matching search-like input in DOM order, which can become brittle if additional search fields are introduced. Prefer targeting #header-search first, then fallback to a visible enabled search input.
Proposed refactor
 function focusSearchInput() {
-  const searchInput = document.querySelector<HTMLInputElement>(
-    'input[type="search"], input[name="q"]',
-  )
-  searchInput?.focus()
+  const headerSearch = document.querySelector<HTMLInputElement>('#header-search')
+  if (headerSearch && headerSearch.getClientRects().length > 0 && !headerSearch.disabled) {
+    headerSearch.focus()
+    return
+  }
+
+  const fallback = Array.from(
+    document.querySelectorAll<HTMLInputElement>('input[type="search"], input[name="q"]'),
+  ).find(el => el.getClientRects().length > 0 && !el.disabled)
+
+  fallback?.focus()
 }

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dcc19c and 4081cc9.

📒 Files selected for processing (3)

app/components/Package/Card.vue
app/pages/search.vue
test/e2e/interactions.spec.ts

💤 Files with no reviewable changes (1)

app/components/Package/Card.vue

coderabbitai · 2026-02-27T15:12:22Z

app/pages/search.vue

+  // Escape returns focus to the search input from anywhere on the page
+  if (e.key === 'Escape') {
+    e.preventDefault()
+    focusSearchInput()
+    return
+  }


⚠️ Potential issue | 🟡 Minor

Escape is handled too globally.

At Line 469, preventDefault() runs for every Escape press on the page, even outside result navigation. This can interfere with expected Escape behaviour in other UI contexts (e.g. overlays/popovers). Scope this branch to when a result/suggestion item is focused (or when search navigation state is active).

is this a fair point @serhalp?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Yes, we should have some checks here, since Escape is used for dismissing popover menus and dropdowns and, eventually, tooltips.

Also, FWIW, the / key also returns the focus to the search box. Do we need an additional key?

Thanks, friends! I was a bit overeager here. Removed the new Esc binding.

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4081cc9 and af31ca9.

📒 Files selected for processing (2)

app/pages/search.vue
test/e2e/interactions.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

app/pages/search.vue

coderabbitai · 2026-02-27T20:52:15Z

test/e2e/interactions.spec.ts

+  test('/search?q=vue → ArrowDown navigates only between results, not keyword buttons', async ({
+    page,
+    goto,
+  }) => {
+    await goto('/search?q=vue', { waitUntil: 'hydration' })
+
+    await expect(page.locator('text=/found \\d+|showing \\d+/i').first()).toBeVisible({
+      timeout: 15000,
+    })
+
+    const firstResult = page.locator('[data-result-index="0"]').first()
+    const secondResult = page.locator('[data-result-index="1"]').first()
+    await expect(firstResult).toBeVisible()
+    await expect(secondResult).toBeVisible()
+
+    // ArrowDown from input focuses the first result
+    await page.keyboard.press('ArrowDown')
+    await expect(firstResult).toBeFocused()
+
+    // Second ArrowDown focuses the second result (not a keyword button within the first)
+    await page.keyboard.press('ArrowDown')
+    await expect(secondResult).toBeFocused()
+  })
+
+  test('/search?q=vue → ArrowUp from first result returns focus to search input', async ({
+    page,
+    goto,
+  }) => {
+    await goto('/search?q=vue', { waitUntil: 'hydration' })
+
+    await expect(page.locator('text=/found \\d+|showing \\d+/i').first()).toBeVisible({
+      timeout: 15000,
+    })
+
+    // Navigate to first result
+    await page.keyboard.press('ArrowDown')
+    await expect(page.locator('[data-result-index="0"]').first()).toBeFocused()
+
+    // ArrowUp returns to the search input
+    await page.keyboard.press('ArrowUp')
+    await expect(page.locator('input[type="search"]')).toBeFocused()
+  })


⚠️ Potential issue | 🟡 Minor

Make the “from search input” precondition explicit in both new keyboard tests.

At Line 91 and Line 110, the tests assert behaviour “from input”, but they never focus/assert the header search input before the first ArrowDown. That allows false positives if global key handling changes.

Suggested hardening diff

test('/search?q=vue → ArrowDown navigates only between results, not keyword buttons', async ({ page, goto, }) => { await goto('/search?q=vue', { waitUntil: 'hydration' }) @@ + const headerSearchInput = page.locator('#header-search') + await headerSearchInput.focus() + await expect(headerSearchInput).toBeFocused() + const firstResult = page.locator('[data-result-index="0"]').first() const secondResult = page.locator('[data-result-index="1"]').first() @@ test('/search?q=vue → ArrowUp from first result returns focus to search input', async ({ page, goto, }) => { await goto('/search?q=vue', { waitUntil: 'hydration' }) @@ + const headerSearchInput = page.locator('#header-search') + await headerSearchInput.focus() + await expect(headerSearchInput).toBeFocused() + // Navigate to first result await page.keyboard.press('ArrowDown') await expect(page.locator('[data-result-index="0"]').first()).toBeFocused() @@ // ArrowUp returns to the search input await page.keyboard.press('ArrowUp') - await expect(page.locator('input[type="search"]')).toBeFocused() + await expect(headerSearchInput).toBeFocused() })

serhalp marked this pull request as ready for review February 27, 2026 15:09

coderabbitai bot reviewed Feb 27, 2026

View reviewed changes

danielroe approved these changes Feb 27, 2026

View reviewed changes

fix: undo overeager new escape hatch

af31ca9

vercel bot deployed to Preview – npmx.dev February 27, 2026 20:51 View deployment

coderabbitai bot reviewed Feb 27, 2026

View reviewed changes

serhalp added this pull request to the merge queue Feb 27, 2026

Merged via the queue into main with commit 85dd9fd Feb 27, 2026
21 checks passed

serhalp deleted the serhalp/fix-search-arrow-nav branch February 27, 2026 21:00

Uh oh!

Conversation

serhalp commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Linked issue

🧭 Context

📚 Description

Uh oh!

vercel bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 27, 2026

Codecov Report

Uh oh!

coderabbitai bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielroe Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

knowler Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knowler Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

serhalp Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

serhalp commented Feb 27, 2026 •

edited

Loading

vercel bot commented Feb 27, 2026 •

edited

Loading

coderabbitai bot commented Feb 27, 2026 •

edited

Loading

coderabbitai bot Feb 27, 2026 •

edited

Loading

knowler Feb 27, 2026 •

edited

Loading