Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/components/BaseCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ defineProps<{
/** Whether this is an exact match for the query */
isExactMatch?: boolean
selected?: boolean
/** Index for keyboard navigation */
index?: number
}>()
</script>

<template>
<article
class="group bg-bg-subtle border border-border rounded-lg p-4 sm:p-6 transition-[border-color,background-color] duration-200 hover:(border-border-hover bg-bg-muted) cursor-pointer relative focus-within:outline-none focus-within:ring-2 focus-within:ring-offset-bg focus-within:ring-offset-2 focus-within:ring-fg/50 focus-within:bg-bg-muted focus-within:border-border-hover"
:tabindex="index != null ? 0 : undefined"
:data-result-index="index"
class="group bg-bg-subtle border border-border rounded-lg p-4 sm:p-6 transition-[border-color,background-color] duration-200 hover:(border-border-hover bg-bg-muted) cursor-pointer relative focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-bg focus-visible:ring-offset-2 focus-visible:ring-accent focus-visible:bg-bg-muted focus-visible:border-accent/50"
:class="{
'border-accent/30 contrast-more:border-accent/90 bg-accent/5': isExactMatch,
'bg-fg-subtle/15!': selected,
Expand Down
4 changes: 1 addition & 3 deletions app/components/Package/Card.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const numberFormatter = useNumberFormatter()
</script>

<template>
<BaseCard :selected="isSelected" :isExactMatch="isExactMatch">
<BaseCard :selected="isSelected" :isExactMatch="isExactMatch" :index="index">
<header class="mb-4 flex items-baseline justify-between gap-2">
<component
:is="headingLevel ?? 'h3'"
Expand All @@ -53,7 +53,6 @@ const numberFormatter = useNumberFormatter()
:to="packageRoute(result.package.name)"
:prefetch-on="prefetch ? 'visibility' : 'interaction'"
class="decoration-none after:content-[''] after:absolute after:inset-0"
:data-result-index="index"
dir="ltr"
>{{ result.package.name }}</NuxtLink
>
Expand Down Expand Up @@ -153,7 +152,6 @@ const numberFormatter = useNumberFormatter()
</div>

<ul
role="list"
v-if="result.package.keywords?.length"
:aria-label="$t('package.card.keywords')"
class="relative z-10 flex flex-wrap gap-1.5 mt-3 pt-3 border-t border-border list-none m-0 p-0 pointer-events-none items-center"
Expand Down
2 changes: 1 addition & 1 deletion app/components/Package/List.vue
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ defineExpose({
:result="item"
:heading-level="headingLevel"
:show-publisher="showPublisher"
:index="index"
:index="(currentPage - 1) * numericPageSize + index"
:search-query="searchQuery"
:class="
index >= newSearchBatchSize &&
Expand Down
151 changes: 151 additions & 0 deletions app/composables/useResultsKeyboardNavigation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { useEventListener } from '@vueuse/core'

/**
* Composable for keyboard navigation through search results and package lists
*
* Provides arrow key navigation (ArrowUp/ArrowDown) and Enter key support
* for navigating through focusable result elements.
*
* @param options - Configuration options
* @param options.includeSuggestions - Whether to include suggestion elements (data-suggestion-index)
* @param options.onArrowUpAtStart - Optional callback when ArrowUp is pressed at the first element
*/
export function useResultsKeyboardNavigation(options?: {

Check warning on line 13 in app/composables/useResultsKeyboardNavigation.ts

View workflow job for this annotation

GitHub Actions / 🤖 Autofix code

eslint-plugin-unicorn(consistent-function-scoping)

Function `focusElement` does not capture any variables from its parent scope

Check warning on line 13 in app/composables/useResultsKeyboardNavigation.ts

View workflow job for this annotation

GitHub Actions / 🤖 Autofix code

eslint-plugin-unicorn(consistent-function-scoping)

Function `isVisible` does not capture any variables from its parent scope
includeSuggestions?: boolean
onArrowUpAtStart?: () => void
}) {
const keyboardShortcuts = useKeyboardShortcuts()

const isVisible = (el: HTMLElement) => el.getClientRects().length > 0

/**
* Get all focusable result elements in DOM order
*/
function getFocusableElements(): HTMLElement[] {
const elements: HTMLElement[] = []

// Include suggestions if enabled (used on search page)
if (options?.includeSuggestions) {
const suggestions = Array.from(
document.querySelectorAll<HTMLElement>('[data-suggestion-index]'),
)
.filter(isVisible)
.sort((a, b) => {
const aIdx = Number.parseInt(a.dataset.suggestionIndex ?? '0', 10)
const bIdx = Number.parseInt(b.dataset.suggestionIndex ?? '0', 10)
return aIdx - bIdx
})
elements.push(...suggestions)
}

// Always include package results
const packages = Array.from(document.querySelectorAll<HTMLElement>('[data-result-index]'))
.filter(isVisible)
.sort((a, b) => {
const aIdx = Number.parseInt(a.dataset.resultIndex ?? '0', 10)
const bIdx = Number.parseInt(b.dataset.resultIndex ?? '0', 10)
return aIdx - bIdx
})
elements.push(...packages)

return elements
}

/**
* Focus an element and scroll it into view if needed
*/
function focusElement(el: HTMLElement) {
el.focus({ preventScroll: true })

// Only scroll if element is not already in viewport
const rect = el.getBoundingClientRect()
const isInViewport =
rect.top >= 0 &&
rect.left >= 0 &&
rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) &&
rect.right <= (window.innerWidth || document.documentElement.clientWidth)

if (!isInViewport) {
el.scrollIntoView({ block: 'nearest', behavior: 'smooth' })
}
}

function handleKeydown(e: KeyboardEvent) {
// Only handle arrow keys and Enter
if (!['ArrowDown', 'ArrowUp', 'Enter'].includes(e.key)) {
return
}

if (!keyboardShortcuts.value) {
return
}

const elements = getFocusableElements()
const currentIndex = elements.findIndex(el => el === document.activeElement)

if (e.key === 'ArrowDown') {
// If there are results available, handle navigation
if (elements.length > 0) {
e.preventDefault()
e.stopPropagation()

// If no result is focused, focus the first one
if (currentIndex < 0) {
const firstEl = elements[0]
if (firstEl) focusElement(firstEl)
return
}

// If a result is already focused, move to the next one
const nextIndex = Math.min(currentIndex + 1, elements.length - 1)
const el = elements[nextIndex]
if (el) focusElement(el)
}
return
}

if (e.key === 'ArrowUp') {
// Only intercept if a result is already focused
if (currentIndex >= 0) {
e.preventDefault()
e.stopPropagation()

// At first result
if (currentIndex === 0) {
// Call custom callback if provided (e.g., return focus to search input)
if (options?.onArrowUpAtStart) {
options.onArrowUpAtStart()
}
return
}
const nextIndex = currentIndex - 1
const el = elements[nextIndex]
if (el) focusElement(el)
}
return
}

if (e.key === 'Enter') {
// Handle Enter on focused card - click the main link inside
if (document.activeElement && elements.includes(document.activeElement as HTMLElement)) {
const card = document.activeElement as HTMLElement
// Find the first link inside the card and click it
const link = card.querySelector('a')
if (link) {
e.preventDefault()
e.stopPropagation()
link.click()
}
}
}
}

// Register keyboard event listeners using useEventListener for better control
// Use capture phase to intercept before other handlers
useEventListener(document, 'keydown', handleKeydown, { capture: true })
Comment on lines +73 to +145
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't steal ArrowUp/ArrowDown from native controls.

This handler runs at document scope and calls preventDefault() before the page's own controls can react. On both app/pages/org/[org].vue:267-288 and app/pages/search.vue:700-724, pressing ArrowDown inside the toolbar's selects or text inputs will jump to the first result instead of moving the caret or opening the control. Please ignore interactive targets unless focus is already on a result element or on an explicitly allowed hand-off source.

Possible direction
 export function useResultsKeyboardNavigation(options?: {
   includeSuggestions?: boolean
   onArrowUpAtStart?: () => void
+  canStartFrom?: (target: HTMLElement | null) => boolean
 }) {
@@
     const elements = getFocusableElements()
     const currentIndex = elements.findIndex(el => el === document.activeElement)
+    const target = e.target instanceof HTMLElement ? e.target : null
+    const isInteractiveTarget = !!target?.closest(
+      'input, textarea, select, button, [contenteditable="true"]',
+    )
+
+    if (currentIndex < 0 && isInteractiveTarget && !options?.canStartFrom?.(target)) {
+      return
+    }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe stealing the arrow keys is desired in this instance, but anyone please tell me if I'm wrong here 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Comment image shows an organization page at localhost:3000/org/atcute in a npm-like UI. Visible elements include:

  • Top bar with a search input labeled “/ search packages …”, and actions: compare, settings, connect.
  • Org header with avatar “A”, handle “@ATCUTE”, “58 public packages”, and an “npm” badge with “58,784 / week”.
  • A PACKAGES section: “58 packages”, a “Filters” button, a sort control showing “Downloads/wk” (native select), and list/grid view toggles.
  • A list of package cards (e.g., @atcute/time-ms, @atcute/bluesky) each with description, date, version, and weekly downloads.

@tylersayshi I love the consistency goal, but stealing ArrowUp/ArrowDown from native controls (inputs/selects/textarea/contenteditable) at the document level will block expected keyboard behavior (caret movement, opening selects, adjusting ranges) and hurts accessibility. Right now, ArrowDown is intercepted even when focus is inside the sort select or the search box, because we call preventDefault in capture before those controls can react.

A small compromise keeps the desired list navigation while preserving native control behavior:

  • Only start navigation when:
    • focus is already on a result element, or
    • the current target is explicitly whitelisted (e.g., the search input on /search).
  • Otherwise, let native controls handle Arrow keys.

Minimal patch to the composable

// app/composables/useResultsKeyboardNavigation.ts
export function useResultsKeyboardNavigation(options?: {
  includeSuggestions?: boolean
  onArrowUpAtStart?: () => void
  canStartFrom?: (target: HTMLElement | null) => boolean
}) {
  // ...

  function handleKeydown(e: KeyboardEvent) {
    if (!['ArrowDown', 'ArrowUp', 'Enter'].includes(e.key) || !keyboardShortcuts.value) return

    const elements = getFocusableElements()
    const currentIndex = elements.findIndex(el => el === document.activeElement)

    const target = e.target instanceof HTMLElement ? e.target : null
    const isInteractiveTarget = !!target?.closest('input, textarea, select, button, [contenteditable="true"]')

    // Don’t hijack Arrow keys from interactive widgets unless navigation is already active
    // or the caller explicitly allows starting from this target.
    if (currentIndex < 0 && isInteractiveTarget && !options?.canStartFrom?.(target)) {
      return
    }

    // ...existing ArrowDown/ArrowUp/Enter logic unchanged...
  }

  useEventListener(document, 'keydown', handleKeydown, { capture: true })
}

Call-site usage

  • Search page: allow starting from the search input only (so ArrowDown in the box moves to first result as today).
useResultsKeyboardNavigation({
  includeSuggestions: true,
  onArrowUpAtStart: focusSearchInput,
  canStartFrom: (el) => !!el?.closest('input[type="search"], [data-search-input="true"]'),
})
  • Org page: don’t pass canStartFrom so Arrow keys inside “Downloads/wk” select, Filters inputs, etc., keep their native behavior; navigation still works once focus is on a result card.

Optional guard

  • Also ignore modified keys to avoid surprises:
if (e.altKey || e.ctrlKey || e.metaKey) return

Tests to lock behavior

  • Add e2e asserting:
    • ArrowDown inside the sort does not move focus to the first result on /org/atcute. ArrowDown in the search input on /search starts navigation (whitelisted). ArrowDown on the document body starts at first result when nothing interactive is focused. If you’d prefer truly global “steal-from-anywhere,” I can roll with that, but it will block selects and text inputs across the page. The above tweak preserves your intended UX on /search while keeping native controls functional on the org page. Want me to prep a patch? 🧠 Learnings used Learnt from: btea Repo: npmx-dev/npmx.dev PR: 1739 File: app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue:259-262 Timestamp: 2026-02-28T07:17:28.172Z Learning: In app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue, the scroll-to-top button for the code preview container should be shown on both desktop and touch devices (gated only by scroll position using SCROLL_TO_TOP_THRESHOLD), not restricted to touch devices. This is different from the window-level ScrollToTop.client.vue component which is touch-device-only, because the code container scroll-to-top helps users navigate long code files where keyboard shortcuts are less practical for container-specific scrolling. Learnt from: graphieros Repo: npmx-dev/npmx.dev PR: 2273 File: app/components/Package/TrendsChart.vue:1648-1648 Timestamp: 2026-03-25T12:31:33.715Z Learning: In `app/components/Package/TrendsChart.vue`, the `tabindex` values on the view-toggle tab buttons (combined/split chart layout) are intentionally inverted from the standard WAI-ARIA pattern. The **selected** tab receives `tabindex="-1"` and the **unselected** tab receives `tabindex="0"`. Do not flag this as a bug or suggest reversing the values to match the WAI-ARIA tabs pattern. Learnt from: userquin Repo: npmx-dev/npmx.dev PR: 1596 File: app/pages/about.vue:318-341 Timestamp: 2026-02-23T05:31:58.922Z Learning: In the npmx.dev project, the contributor popover on the about page is intentionally a mouse-only progressive enhancement. Keyboard support (focus/blur with full popover interaction) was tested but rejected due to Firefox performance issues (3-4 second lag with per-contributor popovers in the Top Layer) and implementation complexity (aria-controls, aria-expanded, focus trapping, Escape handling). All contributor information remains accessible via the clickable link to the GitHub profile. Learnt from: ShroXd Repo: npmx-dev/npmx.dev PR: 2115 File: app/pages/search.vue:365-368 Timestamp: 2026-03-17T06:53:49.002Z Learning: In npmx-dev/npmx.dev (app/pages/search.vue), the `showClaimPrompt` computed intentionally returns `true` during a `status === 'pending'` fetch even when `avail.name` doesn't match `committedQuery.value.trim()`. This is by design to prevent the claim prompt from flickering/disappearing while the user is typing (avoiding layout shifts). The claim button is separately guarded with `:disabled="status === 'pending'"` to prevent accidental clicks on stale data. Do not suggest adding an `avail.name === committedQuery` guard to the pending branch. Learnt from: danielroe Repo: npmx-dev/npmx.dev PR: 1204 File: app/composables/npm/useUserPackages.ts:40-41 Timestamp: 2026-02-08T13:24:06.104Z Learning: In Nuxt 3, when using useAsyncData or useLazyAsyncData, allow and rely on reactive keys by typing them as MaybeRefOrGetter<string>. Keys can be a string, a Ref<string>, a ComputedRef<string>, or a getter () => string. Reactive keys should automatically trigger refetches when their value changes. In reviews, verify that data scripts pass a MaybeRefOrGetter<string> (not a plain string-only key if reactivity is intended), and that the logic handles dynamic key changes without stale data. Learnt from: MatteoGabriele Repo: npmx-dev/npmx.dev PR: 1922 File: app/composables/usePackageListPreferences.ts:52-53 Timestamp: 2026-03-05T10:14:50.799Z Learning: In Nuxt projects (here npmx-dev/npmx.dev), exports from shared/types/* (e.g., PAGE_SIZE_OPTIONS, DEFAULT_COLUMNS, DEFAULT_PREFERENCES, PageSize) are auto-imported by Nuxt for composables and components. Do not add explicit import statements for these constants/types when using files under shared/types/, and rely on the auto-imported bindings in files under app/composables (and similarly in components). This pattern applies to all TS files within app/composables that reference these shared/types exports.


return {
getFocusableElements,
focusElement,
}
}
3 changes: 3 additions & 0 deletions app/pages/org/[org].vue
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ function handleClearFilter(chip: FilterChip) {

const activeTab = shallowRef<'members' | 'teams'>('members')

// Keyboard navigation for package results
useResultsKeyboardNavigation()

// Canonical URL for this org page
const canonicalUrl = computed(() => `https://npmx.dev/@${orgName.value}`)

Expand Down
89 changes: 9 additions & 80 deletions app/pages/search.vue
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,6 @@ const displayResults = computed(() => {
})
})

const resultCount = computed(() => displayResults.value.length)

/**
* The effective total for display and pagination purposes.
* When sorting by non-relevance, we're working with a fetched subset (e.g. 250),
Expand Down Expand Up @@ -405,42 +403,6 @@ const exactMatchType = computed<'package' | 'org' | 'user' | null>(() => {
return null
})

const suggestionCount = computed(() => validatedSuggestions.value.length)
const totalSelectableCount = computed(() => suggestionCount.value + resultCount.value)

const isVisible = (el: HTMLElement) => el.getClientRects().length > 0

/**
* Get all focusable result elements in DOM order (suggestions first, then packages)
*/
function getFocusableElements(): HTMLElement[] {
const suggestions = Array.from(document.querySelectorAll<HTMLElement>('[data-suggestion-index]'))
.filter(isVisible)
.sort((a, b) => {
const aIdx = Number.parseInt(a.dataset.suggestionIndex ?? '0', 10)
const bIdx = Number.parseInt(b.dataset.suggestionIndex ?? '0', 10)
return aIdx - bIdx
})

const packages = Array.from(document.querySelectorAll<HTMLElement>('[data-result-index]'))
.filter(isVisible)
.sort((a, b) => {
const aIdx = Number.parseInt(a.dataset.resultIndex ?? '0', 10)
const bIdx = Number.parseInt(b.dataset.resultIndex ?? '0', 10)
return aIdx - bIdx
})

return [...suggestions, ...packages]
}

/**
* Focus an element and scroll it into view
*/
function focusElement(el: HTMLElement) {
el.focus()
el.scrollIntoView({ block: 'nearest', behavior: 'smooth' })
}

// Navigate to package page
async function navigateToPackage(packageName: string) {
await navigateTo(packageRoute(packageName))
Expand Down Expand Up @@ -482,9 +444,16 @@ function focusSearchInput() {
searchInput?.focus()
}

// Keyboard navigation for results (includes arrow keys and Enter on focused results)
useResultsKeyboardNavigation({
includeSuggestions: true,
onArrowUpAtStart: focusSearchInput,
})

// Additional Enter key handling for search input (exact match navigation)
const keyboardShortcuts = useKeyboardShortcuts()

function handleResultsKeydown(e: KeyboardEvent) {
function handleSearchInputEnter(e: KeyboardEvent) {
if (!keyboardShortcuts.value) {
return
}
Expand All @@ -508,49 +477,9 @@ function handleResultsKeydown(e: KeyboardEvent) {
pendingEnterQuery.value = inputValue
return
}

if (totalSelectableCount.value <= 0) return

const elements = getFocusableElements()
if (elements.length === 0) return

const currentIndex = elements.findIndex(el => el === document.activeElement)

if (e.key === 'ArrowDown') {
e.preventDefault()
const nextIndex = currentIndex < 0 ? 0 : Math.min(currentIndex + 1, elements.length - 1)
const el = elements[nextIndex]
if (el) focusElement(el)
return
}

if (e.key === 'ArrowUp') {
e.preventDefault()
// At first result or no result focused: return focus to search input
if (currentIndex <= 0) {
focusSearchInput()
return
}
const nextIndex = currentIndex - 1
const el = elements[nextIndex]
if (el) focusElement(el)
return
}

if (e.key === 'Enter') {
// Browser handles Enter on focused links naturally, but handle for non-link elements
if (document.activeElement && elements.includes(document.activeElement as HTMLElement)) {
const el = document.activeElement as HTMLElement
// Only prevent default and click if it's not already a link (links handle Enter natively)
if (el.tagName !== 'A') {
e.preventDefault()
el.click()
}
}
}
}

onKeyDown(['ArrowDown', 'ArrowUp', 'Enter'], handleResultsKeydown)
onKeyDown('Enter', handleSearchInputEnter)

useSeoMeta({
title: () =>
Expand Down
Loading
Loading