Svelte: add support for navigating search results with the keyboard (#64257)

This adds support for navigating between search results with keyboard
shortcuts. Similar to the React app, `j` or `down` means "next result",
and `k` or `up` means previous results. To accompany this change, when a
search is submitted, the first result is focused by default to
facilitate iterating over results with the keyboard.
This commit is contained in:
Camden Cheek 2024-08-09 14:24:05 -06:00 committed by GitHub
parent 3df76cb173
commit f1060eccac
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 306 additions and 21 deletions

View File

@ -146,7 +146,7 @@
box-shadow: none;
> .label {
box-shadow: var(--focus-shadow-inner);
box-shadow: var(--focus-shadow-inset);
}
}

View File

@ -98,7 +98,7 @@
user-select: none;
&:focus-visible {
box-shadow: var(--focus-shadow-inner);
box-shadow: var(--focus-shadow-inset);
}
}

View File

@ -1,4 +1,5 @@
<script lang="ts">
import { registerHotkey } from '$lib/Hotkey'
// @sg EnableRollout
import { queryStateStore } from '$lib/search/state'
import { settings } from '$lib/stores'
@ -25,6 +26,25 @@
const queryState = queryStateStore(data.queryOptions ?? {}, $settings)
let searchResults: SearchResults | undefined
for (const key of ['j', 'down']) {
registerHotkey({
keys: { key },
handler: () => {
searchResults?.focusNextResult('down')
},
})
}
for (const key of ['k', 'up']) {
registerHotkey({
keys: { key },
handler: () => {
searchResults?.focusNextResult('up')
},
})
}
$: queryState.set(data.queryOptions ?? {})
$: queryState.setSettings($settings)
</script>

View File

@ -36,11 +36,11 @@
<script lang="ts">
import type { Action } from 'svelte/action'
import RepoStars from '$lib/repo/RepoStars.svelte'
import { type CommitMatch, getMatchUrl } from '$lib/shared'
import Timestamp from '$lib/Timestamp.svelte'
import RepoRev from './RepoRev.svelte'
import RepoStars from '$lib/repo/RepoStars.svelte'
import SearchResult from './SearchResult.svelte'
export let result: CommitMatch
@ -63,7 +63,7 @@
<div slot="title" data-sveltekit-preload-data="tap">
<RepoRev repoName={result.repository} rev={commitOid} />
<span aria-hidden={true} class="interpunct">·</span>
<a href={commitURL}>
<a href={commitURL} data-focusable-search-result>
{result.authorName}: {subject}
</a>
</div>
@ -100,4 +100,8 @@
font-family: var(--code-font-family);
font-size: var(--code-font-size);
}
[data-focusable-search-result]:focus {
box-shadow: var(--focus-shadow);
}
</style>

View File

@ -93,7 +93,7 @@
>
{#each matchesToShow as group, index}
<div class="code">
<a href={getMatchURL(group.startLine + 1, group.endLine)}>
<a href={getMatchURL(group.startLine + 1, group.endLine)} data-focusable-search-result>
<!--
We need to "post-slice" `highlightedHTMLRows` because we fetch highlighting for
the whole chunk.
@ -125,6 +125,7 @@
userInteracted = true
}}
class:expanded
data-focusable-search-result
>
<Icon icon={expanded ? ILucideChevronUp : ILucideChevronDown} inline aria-hidden="true" />
<span>{expandButtonText}</span>
@ -149,7 +150,7 @@
}
&:hover {
background-color: var(--color-bg-2);
background-color: var(--secondary-4);
color: var(--text-title);
}
}
@ -163,7 +164,7 @@
}
&:hover {
background-color: var(--color-bg-2);
background-color: var(--secondary-4);
}
a {
@ -173,4 +174,8 @@
padding: 0.125rem 0.375rem;
}
}
[data-focusable-search-result]:focus {
box-shadow: var(--focus-shadow-inset);
}
</style>

View File

@ -1,6 +1,8 @@
<svelte:options immutable />
<script lang="ts">
import { onMount } from 'svelte'
import RepoStars from '$lib/repo/RepoStars.svelte'
import type { PathMatch } from '$lib/shared'
@ -9,10 +11,20 @@
import SearchResult from './SearchResult.svelte'
export let result: PathMatch
let headerContainer: HTMLElement
onMount(() => {
const lastPathElement = headerContainer.querySelector<HTMLElement>('.last[data-path-item] > a')
if (lastPathElement) {
lastPathElement.dataset.focusableSearchResult = 'true'
}
})
</script>
<SearchResult>
<FileSearchResultHeader slot="title" {result} />
<div bind:this={headerContainer} class="header-container" slot="title">
<FileSearchResultHeader {result} />
</div>
<svelte:fragment slot="info">
{#if result.repoStars}
<RepoStars repoStars={result.repoStars} />
@ -20,3 +32,12 @@
<PreviewButton {result} />
</svelte:fragment>
</SearchResult>
<style lang="scss">
.header-container {
display: contents;
:global([data-focusable-search-result]:focus) {
box-shadow: var(--focus-shadow);
}
}
</style>

View File

@ -1,6 +1,8 @@
<svelte:options immutable />
<script lang="ts">
import { onMount } from 'svelte'
import { highlightRanges } from '$lib/dom'
import { featureFlag } from '$lib/featureflags'
import Icon from '$lib/Icon.svelte'
@ -23,10 +25,18 @@
$: repositoryMatches = result.repositoryMatches?.map(simplifyLineRange) ?? []
$: descriptionMatches = result.descriptionMatches?.map(simplifyLineRange) ?? []
$: rev = result.branches?.[0]
let title: HTMLElement
onMount(() => {
const repoLink = title.querySelector<HTMLElement>('a')
if (repoLink) {
repoLink.dataset.focusableSearchResult = 'true'
}
})
</script>
<SearchResult>
<div slot="title">
<div bind:this={title} slot="title" class="title">
<RepoRev repoName={result.repository} {rev} highlights={repositoryMatches} />
{#if result.fork}
<span class="info">
@ -102,4 +112,10 @@
margin-left: 0.5rem;
padding-left: 0.5rem;
}
.title {
:global([data-focusable-search-result]:focus) {
box-shadow: var(--focus-shadow-inset);
}
}
</style>

View File

@ -1,7 +1,12 @@
<svelte:options immutable />
<script context="module" lang="ts">
export type SearchResultsCapture = number
export interface SearchResultsCapture {
// The search results scroll offset
scroll: number
// The currently focused search result (if any)
focused: number | undefined
}
interface ResultStateCache {
count: number
expanded: Set<SearchMatch>
@ -15,7 +20,7 @@
<script lang="ts">
import type { Observable } from 'rxjs'
import { onMount, tick } from 'svelte'
import { afterUpdate, onMount, tick } from 'svelte'
import { writable } from 'svelte/store'
import { beforeNavigate, goto } from '$app/navigation'
@ -48,6 +53,7 @@
import type { SearchJob } from './searchJob'
import { getSearchResultComponent } from './searchResultFactory'
import { setSearchResultsContext } from './searchResultsContext'
import { focusedResultIndex, nextResult, nthFocusableResult } from './searchResultsFocus'
import StreamingProgress from './StreamingProgress.svelte'
export let stream: Observable<AggregateStreamingSearchResults>
@ -57,15 +63,34 @@
export let searchJob: SearchJob | undefined = undefined
export function capture(): SearchResultsCapture {
return $resultContainer?.scrollTop ?? 0
return {
scroll: $resultContainer?.scrollTop ?? 0,
focused: $resultContainer ? focusedResultIndex($resultContainer) : undefined,
}
}
export function restore(capture?: SearchResultsCapture): void {
if ($resultContainer) {
$resultContainer.scrollTop = capture ?? 0
if ($resultContainer && capture) {
$resultContainer.scrollTop = capture.scroll
if (capture.focused) {
nthFocusableResult($resultContainer, capture.focused)?.focus({ preventScroll: true })
}
}
}
export function focusNextResult(direction: 'up' | 'down'): boolean {
if ($resultContainer) {
const nextFocus = nextResult($resultContainer, direction)
if (!nextFocus) {
return false
}
nextFocus.focus({ preventScroll: true })
nextFocus.scrollIntoView({ behavior: 'smooth', block: 'nearest' })
return true
}
return false
}
const resultContainer = writable<HTMLElement | null>(null)
let searchResultsFiltersPanel: Panel
const recentSearches = createRecentSearchesStore()
@ -80,6 +105,18 @@
})
}
let haveSetFocus = false // gets reset on query resubmission or filter changes
afterUpdate(() => {
if (!$isViewportMobile && !haveSetFocus && results.length > 0) {
const firstFocusableResult = $resultContainer?.querySelector<HTMLElement>('[data-focusable-search-result]')
if (firstFocusableResult) {
firstFocusableResult.focus()
haveSetFocus = true
}
}
})
$: selectedFilters, (haveSetFocus = false) // reset focus on filter change
// Logic for maintaining list state (scroll position, rendered items, open
// items) for backwards navigation.
$: cacheEntry = cache.get(queryFromURL)
@ -148,6 +185,7 @@
}
function handleSubmit() {
haveSetFocus = false // reset focus when a new query is submitted
TELEMETRY_RECORDER.recordEvent('search', 'submit', {
metadata: { source: TELEMETRY_SEARCH_SOURCE_TYPE['nav'] },
})
@ -314,6 +352,14 @@
margin: 0;
list-style: none;
}
:global([data-focusable-search-result]) {
// Set a scroll margin on the focused search results
// so that it doesn't underlay the sticky headers and
// so that there is a little bit of space between the
// result and the scroll box.
scroll-margin: 4rem;
}
}
.message-container {

View File

@ -42,7 +42,7 @@
<svelte:fragment slot="body">
<div use:observeIntersection={$scrollContainer} on:intersecting={event => (visible = event.detail)}>
{#each result.symbols as symbol, index}
<a href={symbol.url}>
<a href={symbol.url} data-focusable-search-result>
<div class="result">
<SymbolKindIcon symbolKind={symbol.kind} />
{#await highlightedHTMLRows then result}
@ -69,13 +69,20 @@
gap: 0.5rem;
border-bottom: 1px solid var(--border-color);
background-color: var(--code-bg);
&:hover {
background-color: var(--subtle-bg-2);
}
}
a:hover {
text-decoration: none;
a {
display: block;
box-sizing: border-box;
&:hover {
text-decoration: none;
}
}
[data-focusable-search-result]:focus {
box-shadow: var(--focus-shadow-inset);
}
</style>

View File

@ -7,6 +7,7 @@ import {
createContentMatch,
createPathMatch,
createSymbolMatch,
createRepositoryMatch,
} from '$testing/search-testdata'
const chunkMatch: ContentMatch = {
@ -286,6 +287,96 @@ test.describe('search results', async () => {
await searchInput.press('Enter')
await expect(page).toHaveURL(/\/search\?q=test&patternType=standard&sm=0/)
})
test('focus shortcuts', async ({ page, sg }) => {
sg.mockOperations({
HighlightedFile: () => ({
repository: {
commit: {
blob: {
highlight: {
aborted: true,
lineRanges: [],
},
},
},
},
}),
})
const stream = await sg.mockSearchStream()
await page.goto('/search?q=test')
await page.getByRole('heading', { name: 'Filter results' }).waitFor()
const contentMatch = createContentMatch()
contentMatch.chunkMatches = contentMatch.chunkMatches?.slice(0, 2)
const pathMatch = createPathMatch()
const repoMatch = createRepositoryMatch()
const symbolMatch = createSymbolMatch()
const commitMatch = createCommitMatch('commit')
await stream.publish(
{
type: 'matches',
data: [contentMatch, pathMatch, repoMatch, symbolMatch, commitMatch],
},
createProgressEvent(),
createDoneEvent()
)
await stream.close()
function firstLine(s: string): string {
return s.split('\n')[0].trim()
}
// Iterate downwards through each result type
{
for (const chunkMatch of contentMatch.chunkMatches?.slice(0, 2) ?? []) {
await expect(page.locator('*:focus')).toContainText(firstLine(chunkMatch.content))
await page.keyboard.press('j')
}
await expect(page.locator('*:focus')).toContainText(pathMatch.path.split('/').at(-1) ?? '')
await page.keyboard.press('ArrowDown') // Check a down arrow too
await expect(page.locator('*:focus')).toContainText(repoMatch.repository.split('/').slice(1).join('/'))
await page.keyboard.press('j')
for (const symbol of symbolMatch.symbols) {
await expect(page.locator(`*:focus [data-line="${symbol.line + 1}"]`)).toBeVisible()
await page.keyboard.press('j')
}
await expect(page.locator('*:focus')).toContainText(firstLine(commitMatch.message))
await page.keyboard.press('j')
// Pressing down on the last result keeps us on the last result
await expect(page.locator('*:focus')).toContainText(firstLine(commitMatch.message))
}
// Go in reverse, iterating up through the results
{
for (const symbol of symbolMatch.symbols.reverse()) {
await page.keyboard.press('k')
await expect(page.locator(`*:focus [data-line="${symbol.line + 1}"]`)).toBeVisible()
}
await page.keyboard.press('k')
await expect(page.locator('*:focus')).toContainText(repoMatch.repository.split('/').slice(1).join('/'), {
useInnerText: true,
})
await page.keyboard.press('ArrowUp') // Check an up arrow too
await expect(page.locator('*:focus')).toContainText(pathMatch.path.split('/').at(-1) ?? '')
for (const chunkMatch of contentMatch.chunkMatches?.slice(0, 2)?.reverse() ?? []) {
await page.keyboard.press('k')
await expect(page.locator('*:focus')).toContainText(firstLine(chunkMatch.content))
}
// Pressing up on the top result stays on the top result
await page.keyboard.press('k')
await expect(page.locator('*:focus')).toContainText(firstLine(contentMatch.chunkMatches?.[0].content ?? ''))
}
})
})
test.describe('search filters', async () => {

View File

@ -0,0 +1,62 @@
export function nextResult(root: HTMLElement, direction: 'up' | 'down'): HTMLElement | undefined {
const focusedResult = getFocusedResult()
if (!focusedResult) {
return undefined
}
for (const nextFocus of focusableSearchResults(root, direction, focusedResult)) {
return nextFocus
}
return undefined
}
export function focusedResultIndex(root: HTMLElement): number | undefined {
const focusedResult = getFocusedResult()
if (focusedResult) {
for (const [i, focusable] of enumerate(focusableSearchResults(root, 'down'))) {
if (focusedResult.isEqualNode(focusable)) {
return i
}
}
}
return undefined
}
export function nthFocusableResult(root: HTMLElement, n: number): HTMLElement | undefined {
for (const [i, focusable] of enumerate(focusableSearchResults(root, 'down'))) {
if (i === n) {
return focusable
}
}
return undefined
}
// A generator that iterates over all focusable search result elements in the given direction.
// Also supports starting from a specific element by providing `from`.
function* focusableSearchResults(root: HTMLElement, direction: 'up' | 'down', from?: HTMLElement) {
const walker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT)
if (from) {
walker.currentNode = from
}
const next = () => (direction === 'up' ? walker.previousNode() : walker.nextNode()) as HTMLElement | null
for (let candidate = next(); candidate !== null; candidate = next()) {
if (candidate.hasAttribute('data-focusable-search-result')) {
yield candidate
}
}
}
// A helper that wraps a generator
function* enumerate<T>(iter: Iterable<T>): Generator<[number, T]> {
let i = 0
for (let value of iter) {
yield [i++, value]
}
}
function getFocusedResult(): HTMLElement | null {
return document.activeElement &&
document.activeElement instanceof HTMLElement &&
'focusableSearchResult' in document.activeElement.dataset
? document.activeElement
: null
}

View File

@ -23,6 +23,11 @@
// links, buttons, inputs, etc. We have to have some common "global"
// styles for them (like focus ring).
body {
--focus-shadow-inset: 0 0 0 2px var(--primary-2) inset;
--focus-shadow: 0 0 0 2px var(--primary-2);
}
:focus {
outline: none;
}
@ -32,13 +37,13 @@ a:focus-visible {
// that don't have any spacing between their border and
// overflowed parent visible border (so any outside shadow
// is visually cut/cropped)
box-shadow: 0 0 0 2px var(--primary-2) inset;
box-shadow: var(--focus-shadow-inset);
}
button:focus-visible,
input:focus-visible,
summary:focus-visible {
box-shadow: 0 0 0 2px var(--primary-2);
box-shadow: var(--focus-shadow);
}
// Taken from https://www.a11yproject.com/posts/how-to-hide-content/

View File

@ -21,7 +21,6 @@ body {
--font-size-xs: 0.8125rem;
--code-font-size: 13px;
--border-radius: 4px;
--focus-shadow-inner: 0 0 0 2px var(--primary-2) inset;
input::placeholder {
color: var(--text-muted);

View File

@ -9,6 +9,7 @@ import type {
SymbolMatch,
TeamMatch,
SearchEvent,
RepositoryMatch,
} from '$lib/shared'
import { SymbolKind } from '../lib/graphql-types'
@ -191,6 +192,14 @@ export function createSymbolMatch(): SymbolMatch {
}
}
export function createRepositoryMatch(): RepositoryMatch {
return {
type: 'repo',
repository: createRepoName(),
repoStars: createRepoStars(),
}
}
function createRange(
line: number,
maxLength: number = MAX_LINE_LENGTH