From 35a5a968a3c9b3cc55e2d8177c85a599629e98e1 Mon Sep 17 00:00:00 2001 From: Jenni C <97056108+dihydroJenoxide@users.noreply.github.com> Date: Wed, 14 Oct 2026 19:56:46 +0000 Subject: [PATCH 1/2] Create docs-internal PR for Early Access preview (#63818) From 8fdd34063233a8fa1fb83a9496507617f1cc68c9 Mon Sep 17 00:00:00 2001 From: Evan Bonsignori Date: Wed, 14 Oct 2026 20:04:08 +0000 Subject: [PATCH 2/2] Docs 2026: brand-migrate the search modal (position, scrim, typography) (#62940) Co-authored-by: Kevin Heis Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe098c4c-e0c5-4568-95ce-04beade3d23d --- data/ui.yml | 3 + src/fixtures/fixtures/data/ui.yml | 3 + src/fixtures/tests/playwright-a11y.spec.ts | 45 ++ .../tests/playwright-rendering.spec.ts | 590 +++++++++++++- src/frame/components/hooks/useQueryParam.ts | 18 +- src/frame/components/page-header/Header.tsx | 3 +- .../context/SearchOverlayContext.tsx | 3 +- .../helpers/execute-search-actions.ts | 45 ++ .../components/hooks/useMultiQueryParams.ts | 10 +- .../components/input/AskAIResults.module.scss | 94 ++- src/search/components/input/AskAIResults.tsx | 105 +-- src/search/components/input/SearchContext.tsx | 20 +- src/search/components/input/SearchDialog.tsx | 127 +++ src/search/components/input/SearchGroups.tsx | 402 +++++----- .../input/SearchOverlay.module.scss | 419 ++++++++-- src/search/components/input/SearchOverlay.tsx | 737 ++++++++---------- .../input/SearchOverlayContainer.tsx | 14 +- src/search/components/input/search-options.ts | 166 ++++ src/search/components/input/variables.scss | 10 +- src/search/lib/ai-search-constants.ts | 3 + src/search/tests/search-options.ts | 211 +++++ 21 files changed, 2214 insertions(+), 814 deletions(-) create mode 100644 src/search/components/input/SearchDialog.tsx create mode 100644 src/search/components/input/search-options.ts create mode 100644 src/search/tests/search-options.ts diff --git a/data/ui.yml b/data/ui.yml index 5ceaa8643ce5..f71b85944b51 100644 --- a/data/ui.yml +++ b/data/ui.yml @@ -54,9 +54,12 @@ search: beta_tag: Beta return_to_search: Return to search clear_search_query: Clear + close_search: Close search view_all_search_results: View more results + view_all_search_results_with_count: View all {{n}} results no_results_found: No results found no_results_found_announcement: 'Search Docs: No results found' + loading_results_announcement: Loading search results search_docs_with_query: Search docs for "{{query}}" privacy_disclaimer: For product and service improvement purposes, GitHub will retain questions and answers generated in the Docs search function. Please see the GitHub Privacy Statement to review how GitHub collects and uses your data. ai: diff --git a/src/fixtures/fixtures/data/ui.yml b/src/fixtures/fixtures/data/ui.yml index 5ceaa8643ce5..f71b85944b51 100644 --- a/src/fixtures/fixtures/data/ui.yml +++ b/src/fixtures/fixtures/data/ui.yml @@ -54,9 +54,12 @@ search: beta_tag: Beta return_to_search: Return to search clear_search_query: Clear + close_search: Close search view_all_search_results: View more results + view_all_search_results_with_count: View all {{n}} results no_results_found: No results found no_results_found_announcement: 'Search Docs: No results found' + loading_results_announcement: Loading search results search_docs_with_query: Search docs for "{{query}}" privacy_disclaimer: For product and service improvement purposes, GitHub will retain questions and answers generated in the Docs search function. Please see the GitHub Privacy Statement to review how GitHub collects and uses your data. ai: diff --git a/src/fixtures/tests/playwright-a11y.spec.ts b/src/fixtures/tests/playwright-a11y.spec.ts index 8cb28892669d..9a1b801d2b79 100644 --- a/src/fixtures/tests/playwright-a11y.spec.ts +++ b/src/fixtures/tests/playwright-a11y.spec.ts @@ -79,3 +79,48 @@ test.describe('search filters (narrow viewport)', () => { expect(accessibilityScanResults.violations).toEqual([]) }) }) + +// The search overlay has never been axe-scanned: the `search` entry above covers the +// results page, and nothing opened the dialog. It is now a hand-rolled listbox with +// virtual focus, so its ARIA is worth asserting rather than assuming. +test.describe('search overlay', () => { + test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') + + test('open suggestions listbox passes axe', async ({ page }) => { + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + await page.getByTestId('overlay-search-input').fill('serve playwright') + await expect(page.getByTestId('view-all-results')).toBeVisible() + + // Scoped to the dialog: the page behind the scrim has its own entry, and scanning + // both would attribute unrelated violations to this component. + const accessibilityScanResults = await new AxeBuilder({ page }) + .include('[role="dialog"]') + .analyze() + + expect(accessibilityScanResults.violations).toEqual([]) + }) + + test('listbox with an option selected passes axe', async ({ page }) => { + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('serve playwright') + await expect(page.getByTestId('view-all-results')).toBeVisible() + + // aria-activedescendant only points at something once a row is selected, so the + // unselected scan above cannot catch a broken reference. + await page.keyboard.press('ArrowDown') + await expect(input).toHaveAttribute('aria-activedescendant', /search-option-/) + + const accessibilityScanResults = await new AxeBuilder({ page }) + .include('[role="dialog"]') + .analyze() + + expect(accessibilityScanResults.violations).toEqual([]) + }) +}) diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index ec3ffd1405d0..dbbc831cd3ff 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -1,5 +1,5 @@ import dotenv from 'dotenv' -import { test, expect } from '@playwright/test' +import { test, expect, type Page } from '@playwright/test' import { turnOffExperimentsInPage } from '../helpers/turn-off-experiments' import { contrastRatio } from '@/fixtures/helpers/color-contrast' import { @@ -298,7 +298,7 @@ test('do a search from home page and click on "Foo" page', async ({ page }) => { await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('serve playwright') await page.waitForTimeout(1000) - await page.getByText('View more results').click() + await page.getByTestId('view-all-results').click() await expect(page).toHaveURL( /\/search\?search-overlay-input=serve\+playwright&query=serve\+playwright/, @@ -321,7 +321,7 @@ test('open search, and perform a general search', async ({ page }) => { await page.getByTestId('overlay-search-input').fill('serve playwright') // Wait 1 second for results to load, because the UI blocks submitting the query until then. await page.waitForTimeout(1000) - await page.getByText('View more results').click() + await page.getByTestId('view-all-results').click() await expect(page).toHaveURL( /\/search\?search-overlay-input=serve\+playwright&query=serve\+playwright/, @@ -344,13 +344,569 @@ test('open search, and select a general search article', async ({ page }) => { await page.getByTestId('overlay-search-input').fill('serve playwright') const searchOverlay = page.getByTestId('general-autocomplete-suggestions') await expect(searchOverlay.getByText('For Playwright')).toBeVisible() - await page.keyboard.press('ArrowDown') + + // Walk down to "For Playwright" rather than assuming a fixed number of presses — AI rows + // render above docs rows, so this keeps working if the groups are reordered again. + const input = page.getByTestId('overlay-search-input') + const targetId = await searchOverlay + .locator('[role="option"][id^="search-option-general-"]', { hasText: 'For Playwright' }) + .first() + .getAttribute('id') + expect(targetId).toBeTruthy() + + for (let i = 0; i < 15; i++) { + if ((await input.getAttribute('aria-activedescendant')) === targetId) break + await page.keyboard.press('ArrowDown') + } + await expect(input).toHaveAttribute('aria-activedescendant', targetId as string) + + // Select the general search item, "For Playwright" await page.keyboard.press('Enter') await expect(page).toHaveURL(/\/get-started\/foo\/for-playwright$/) await expect(page).toHaveTitle(/For Playwright/) }) +test('open search, and arrow down to the pinned "View all results" option', async ({ page }) => { + test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') + + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('serve playwright') + await expect(page.getByTestId('view-all-results')).toBeVisible() + + // Count across the dialog: the pinned view-all joins the listbox through aria-owns, so it + // is not a DOM descendant of #search-listbox. + const optionCount = await page.locator('[role="dialog"] [role="option"]').count() + + // The pinned control is the final stop, not an unreachable footer link. Stepping once + // more past it returns to "nothing selected" rather than wrapping. + for (let i = 0; i < optionCount; i++) { + await page.keyboard.press('ArrowDown') + } + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-view-all') + await expect(page.getByTestId('view-all-results')).toHaveAttribute('aria-selected', 'true') + // Pinned outside the scroller, so it stays on screen even when selected last. + await expect(page.getByTestId('view-all-results')).toBeInViewport() + + await page.keyboard.press('ArrowDown') + await expect(input).not.toHaveAttribute('aria-activedescendant', /.+/) + + // ArrowUp from nothing selected lands on the last option, which is this one. + await page.keyboard.press('ArrowUp') + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-view-all') + + await page.keyboard.press('Enter') + await expect(page).toHaveURL(/\/search\?.*query=serve\+playwright/) +}) + +test('search overlay keeps aria-activedescendant resolvable across the whole cycle', async ({ + page, +}) => { + test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') + + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('serve playwright') + await expect(page.getByTestId('view-all-results')).toBeVisible() + + const optionCount = await page.locator('[role="dialog"] [role="option"]').count() + expect(optionCount).toBeGreaterThan(0) + + // Walk past both ends in both directions. At every step the active descendant must + // either be absent or name a live element inside the controlled region — the class of + // bug that phantom indices used to produce silently. + for (const key of ['ArrowDown', 'ArrowUp']) { + for (let i = 0; i < optionCount + 2; i++) { + await page.keyboard.press(key) + const check = await page.evaluate(() => { + const el = document.querySelector('[data-testid="overlay-search-input"]')! + const activeId = el.getAttribute('aria-activedescendant') + if (!activeId) return { ok: true, activeId: null } + const controls = document.getElementById(el.getAttribute('aria-controls')!) + const target = document.getElementById(activeId) + // Inside the listbox, or owned by it: the pinned view-all joins through aria-owns. + const owned = (controls?.getAttribute('aria-owns') || '').split(/\s+/).includes(activeId) + return { + ok: !!target && !!controls && (controls.contains(target) || owned), + activeId, + selectedCount: document.querySelectorAll('[role="option"][aria-selected="true"]').length, + } + }) + expect( + check.ok, + `${key} step ${i}: ${check.activeId} did not resolve inside the listbox`, + ).toBe(true) + if (check.activeId) { + expect(check.selectedCount).toBe(1) + } + } + } +}) + +// The dialog's own contract. Search is mocked, so these run without Elasticsearch: they +// test the modal and its keyboard model, not search relevance. +test.describe('search dialog', () => { + const HITS = [ + { title: 'Bar Usually Comes After Foo', url: '/en/get-started/foo/bar' }, + { title: 'For Playwright', url: '/en/get-started/foo/for-playwright' }, + ] + + async function mockSearch(page: Page, { delayMs = 0, fail = false } = {}) { + await page.route('**/api/search/combined-search/v1**', async (route) => { + if (fail) return route.fulfill({ status: 500, body: '{"error":"mocked"}' }) + const query = new URL(route.request().url()).searchParams.get('query') || '' + if (delayMs) await new Promise((resolve) => setTimeout(resolve, delayMs)) + try { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + aiAutocompleteSuggestions: { + meta: {}, + hits: [{ term: `how do I ${query.trim()}?`, highlights: [] }], + }, + generalSearchResults: { meta: { found: { value: 2 } }, hits: HITS }, + }), + }) + } catch { + // The page may have navigated away while a delayed response was pending. + } + }) + } + + // Every id aria-activedescendant names must exist in the DOM. + async function expectActiveDescendantResolves(page: Page) { + const id = await page.getByTestId('overlay-search-input').getAttribute('aria-activedescendant') + if (id) await expect(page.locator(`[id="${id}"]`)).toHaveCount(1) + return id + } + + test('traps focus, locks page scroll, and returns focus to the opener', async ({ page }) => { + await page.goto('/get-started/foo/bar') + await turnOffExperimentsInPage(page) + + const opener = page.getByTestId('toggle-search') + await opener.click() + const input = page.getByTestId('overlay-search-input') + await expect(input).toBeFocused() + expect(await page.evaluate(() => document.body.style.overflow)).toBe('hidden') + + // Tab cycles inside the dialog rather than escaping to the page behind the scrim. + const dialog = page.locator('[role="dialog"][aria-modal="true"]') + for (let i = 0; i < 6; i++) { + await page.keyboard.press('Tab') + expect(await dialog.evaluate((el) => el.contains(document.activeElement))).toBe(true) + } + + // A press on the scrim closes it. (5, 5) is outside the centred card at this width. + await page.mouse.click(5, 5) + await expect(input).toHaveCount(0) + await expect(opener).toBeFocused() + expect(await page.evaluate(() => document.body.style.overflow)).toBe('') + }) + + test('the close button dismisses the dialog at a phone viewport', async ({ page }) => { + // Below `md` the dialog covers the viewport, so there is no scrim to press. + await page.setViewportSize({ width: 375, height: 812 }) + await page.goto('/get-started/foo/bar') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + await expect(page.getByTestId('overlay-search-input')).toBeVisible() + await page.getByRole('button', { name: 'Close search' }).click() + + await expect(page.getByTestId('overlay-search-input')).toHaveCount(0) + await expect(page).not.toHaveURL(/search-overlay-open/) + }) + + test('selecting a result straight after typing still navigates', async ({ page }) => { + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-1')).toBeVisible() + + // One more keystroke queues the input's debounced URL write. Selecting before it fires + // used to let that write — a replace of the current page — cancel the navigation. + await input.press('End') + await input.pressSequentially(' x', { delay: 0 }) + // The query row and one suggestion, then general-0 and general-1. + for (let i = 0; i < 4; i++) await input.press('ArrowDown', { delay: 0 }) + await input.press('Enter', { delay: 0 }) + + await expect(page).toHaveURL(/\/get-started\/foo\/for-playwright/) + await expect(page.locator('h1')).toHaveText('For Playwright') + }) + + test('closing straight after typing leaves the overlay closed in the URL', async ({ page }) => { + await mockSearch(page) + await page.goto('/get-started/foo/bar') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.pressSequentially('foo', { delay: 0 }) + await input.press('Escape', { delay: 0 }) + + await expect(input).toHaveCount(0) + // Past the 100ms debounce: a stale queued write used to put the param back. + await page.waitForTimeout(300) + await expect(page).not.toHaveURL(/search-overlay-open/) + }) + + test('clicking a row keeps focus, and the arrow keys, on the input', async ({ page }) => { + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + const row = page.locator('#search-option-general-0') + await expect(row).toBeVisible() + + // Press without releasing on the row, so no click (and no navigation) fires. + const box = await row.boundingBox() + await page.mouse.move(box!.x + 40, box!.y + box!.height / 2) + await page.mouse.down() + await expect(input).toBeFocused() + await page.mouse.move(5, 5) + await page.mouse.up() + }) + + test('rows hidden behind the autocomplete spinner are not arrow-reachable', async ({ page }) => { + test.setTimeout(30_000) + await page.goto('/') + await turnOffExperimentsInPage(page) + await mockSearch(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + + // Slow the next response past the 1s spinner threshold. + await page.unroute('**/api/search/combined-search/v1**') + await mockSearch(page, { delayMs: 4000 }) + await input.pressSequentially('b') + await expect(page.locator('#search-option-general-0')).toHaveCount(0, { timeout: 3000 }) + + for (let i = 0; i < 3; i++) { + await input.press('ArrowDown') + const id = await expectActiveDescendantResolves(page) + expect(id ?? '').not.toMatch(/^search-option-(ai|general)-/) + } + }) + + test('the docs fallback row shows the query without template braces', async ({ page }) => { + await mockSearch(page, { fail: true }) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + await page.getByTestId('overlay-search-input').fill('repository') + await expect(page.locator('#search-option-search-docs')).toHaveText( + 'Search docs for "repository"', + ) + }) + + test('Copilot sources are not arrow-reachable until the answer finishes', async ({ page }) => { + test.setTimeout(30_000) + // A stream that sends its sources, then stalls before the answer. route.fulfill sends + // the whole body at once, so the pause needs a real ReadableStream. + await page.addInitScript(() => { + const realFetch = window.fetch + window.fetch = async (input, init) => { + const url = + typeof input === 'string' ? input : input instanceof URL ? input.href : input.url + if (!url.includes('/api/ai-search/v1')) return realFetch(input, init) + const encoder = new TextEncoder() + const body = new ReadableStream({ + async start(controller) { + controller.enqueue( + encoder.encode( + '{"chunkType":"SOURCES","sources":[{"title":"For Playwright","index":"/en/get-started","url":"http://localhost:4000/en/get-started/foo/for-playwright"}]}\n', + ), + ) + await new Promise((resolve) => setTimeout(resolve, 2500)) + controller.enqueue( + encoder.encode('{"chunkType":"MESSAGE_CHUNK","text":"An answer."}\n'), + ) + controller.close() + }, + }) + return new Response(body, { + status: 200, + headers: { 'Content-Type': 'application/x-ndjson' }, + }) + } + }) + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + await input.press('Enter') // nothing selected: ask Copilot + + // Mid-stream: the sources have arrived but are not rendered, so they must not be reachable. + await page.waitForTimeout(800) + for (let i = 0; i < 2; i++) { + await input.press('ArrowDown') + const id = await expectActiveDescendantResolves(page) + expect(id ?? '').not.toMatch(/^search-option-reference-/) + } + + // Once the answer finishes, the source row renders and becomes reachable. + await expect(page.locator('#search-option-reference-0')).toBeVisible({ timeout: 10_000 }) + }) + + test('the selected row keeps a visible outline in forced-colors mode', async ({ page }) => { + await page.emulateMedia({ forcedColors: 'active' }) + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + + const outlineOf = (selector: string) => + page.locator(selector).evaluate((element) => getComputedStyle(element).outlineStyle) + + await input.press('ArrowDown') + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-ai-0') + expect(await outlineOf('#search-option-ai-0')).toBe('solid') + + // ArrowUp past the first row clears the selection, and the next one lands on view-all. + await input.press('ArrowUp') + await expect(input).not.toHaveAttribute('aria-activedescendant', /.+/) + await input.press('ArrowUp') + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-view-all') + expect(await outlineOf('#search-option-view-all')).toBe('solid') + }) + + test('announces loading while the spinner replaces the rows', async ({ page }) => { + test.setTimeout(30_000) + await page.goto('/') + await turnOffExperimentsInPage(page) + await mockSearch(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + const status = page.getByRole('dialog').getByRole('status') + await expect(status).toHaveText('') + + // Slow the next response past the 1s spinner threshold. + await page.unroute('**/api/search/combined-search/v1**') + await mockSearch(page, { delayMs: 4000 }) + await input.pressSequentially('b') + await expect(status).toHaveText('Loading search results', { timeout: 3000 }) + await expect(status).toHaveText('', { timeout: 10_000 }) + }) + + // Holds the Copilot answer, one source and one sentence, until the test releases it. + async function holdCopilotStream(page: Page) { + await page.addInitScript(() => { + const realFetch = window.fetch + let release = () => {} + const released = new Promise((resolve) => { + release = resolve + }) + ;(window as unknown as { releaseCopilot: () => void }).releaseCopilot = () => release() + window.fetch = async (input, init) => { + const url = + typeof input === 'string' ? input : input instanceof URL ? input.href : input.url + if (!url.includes('/api/ai-search/v1')) return realFetch(input, init) + await released + return new Response( + '{"chunkType":"SOURCES","sources":[{"title":"For Playwright","index":"/en/get-started","url":"http://localhost:4000/en/get-started/foo/for-playwright"}]}\n' + + '{"chunkType":"MESSAGE_CHUNK","text":"An answer."}\n', + { status: 200, headers: { 'Content-Type': 'application/x-ndjson' } }, + ) + } + }) + } + + async function releaseCopilot(page: Page) { + await page.evaluate(() => + (window as unknown as { releaseCopilot: () => void }).releaseCopilot(), + ) + } + + test('announces loading before the first Copilot chunk arrives', async ({ page }) => { + await holdCopilotStream(page) + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + await input.press('Enter') // nothing selected: ask Copilot + + const loading = page.locator('[aria-live="assertive"]', { + hasText: 'Loading Copilot response...', + }) + await expect(loading).toBeAttached() + + await releaseCopilot(page) + await expect(page.getByText('An answer.')).toBeVisible() + await expect(loading).toHaveCount(0) + }) + + test('the selected source row keeps a visible outline in forced-colors mode', async ({ + page, + }) => { + await page.emulateMedia({ forcedColors: 'active' }) + await holdCopilotStream(page) + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + await input.press('Enter') // nothing selected: ask Copilot + await releaseCopilot(page) + + const source = page.locator('#search-option-reference-0') + await expect(source).toBeVisible() + await expect(async () => { + await input.press('ArrowDown') + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-reference-0', { + timeout: 100, + }) + }).toPass({ timeout: 5000 }) + expect(await source.evaluate((element) => getComputedStyle(element).outlineStyle)).toBe('solid') + }) + + test('the Copilot answer sits outside the listbox, under real headings', async ({ page }) => { + await page.route('**/api/ai-search/v1', (route) => + route.fulfill({ + status: 200, + headers: { 'Content-Type': 'application/x-ndjson' }, + body: + '{"chunkType":"SOURCES","sources":[{"title":"For Playwright","index":"/en/get-started","url":"http://localhost:4000/en/get-started/foo/for-playwright"}]}\n' + + '{"chunkType":"MESSAGE_CHUNK","text":"An answer."}\n', + }), + ) + await mockSearch(page) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('foo') + await expect(page.locator('#search-option-general-0')).toBeVisible() + await input.press('Enter') // nothing selected: ask Copilot + await expect(page.locator('#search-option-reference-0')).toBeVisible() + + // Some screen readers expose only options inside a listbox, so the answer's prose and + // buttons live outside it, and get headings again. + const listbox = page.getByRole('listbox') + await expect(listbox.locator('#ask-ai-result-container')).toHaveCount(0) + await expect(listbox.getByRole('button')).toHaveCount(0) + const dialog = page.getByRole('dialog') + await expect(dialog.getByRole('heading', { name: 'Ask Copilot', level: 3 })).toBeVisible() + await expect(dialog.getByRole('heading', { name: 'Copilot Sources', level: 3 })).toBeVisible() + // The sources stay options, reachable from the input. + await input.press('ArrowDown') + await expect(input).toHaveAttribute('aria-activedescendant', 'search-option-reference-0') + }) + + test('Back after picking a result leaves the dialog closed', async ({ page }) => { + await mockSearch(page) + await page.goto('/get-started/foo/bar') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + await page.getByTestId('overlay-search-input').fill('foo') + await page.locator('#search-option-general-1').click() + await expect(page).toHaveURL(/\/get-started\/foo\/for-playwright/) + + // The entry we left must not still say the overlay is open. + await page.goBack() + await expect(page).toHaveURL(/\/get-started\/foo\/bar/) + await expect(page).not.toHaveURL(/search-overlay-open/) + await expect(page.getByTestId('overlay-search-input')).toHaveCount(0) + }) + + test('the rule under the field marks when the input has focus', async ({ page }) => { + await page.goto('/get-started/foo/bar') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await expect(input).toBeFocused() + const rule = page.locator('[role="dialog"] hr') + // Solid while the input has focus... + await expect(rule).toHaveCSS('background-image', 'none') + + // ...and back to the dashed rule once focus moves elsewhere in the dialog. + await page.getByRole('button', { name: 'Close search' }).focus() + await expect(rule).not.toHaveCSS('background-image', 'none') + }) + + test('the focus rule stays visible when search fails', async ({ page }) => { + // Error states used to remove the rule, and the input's focus marker with it. + await mockSearch(page, { fail: true }) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.fill('repository') + await expect(page.locator('#search-option-search-docs')).toBeVisible() + await expect(input).toBeFocused() + + const rule = page.locator('[role="dialog"] hr') + await expect(rule).toBeVisible() + await expect(rule).toHaveCSS('background-image', 'none') + }) + + test.describe('in a German-language browser', () => { + test.use({ locale: 'de-DE' }) + + test('the result count follows the page language, not the browser', async ({ page }) => { + await page.route('**/api/search/combined-search/v1**', (route) => + route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + aiAutocompleteSuggestions: { meta: {}, hits: [] }, + generalSearchResults: { meta: { found: { value: 1234567 } }, hits: HITS }, + }), + }), + ) + await page.goto('/') + await turnOffExperimentsInPage(page) + + await page.getByTestId('toggle-search').click() + await page.getByTestId('overlay-search-input').fill('foo') + // An English page: "1,234,567", not the browser's "1.234.567". + await expect(page.getByTestId('view-all-results')).toHaveText('View all 1,234,567 results') + }) + }) +}) + test('open search, and get auto-complete results', async ({ page }) => { test.skip(!SEARCH_TESTS, 'No local Elasticsearch, no tests involving search') @@ -361,7 +917,7 @@ test('open search, and get auto-complete results', async ({ page }) => { let listGroup = page.getByTestId('ai-autocomplete-suggestions') await expect(listGroup).toBeVisible() - let listItems = listGroup.locator('li') + let listItems = listGroup.locator('[role="option"]') await expect(listItems).toHaveCount(4) // The first list mirrors queries.json fixture topQueries. @@ -384,7 +940,7 @@ test('open search, and get auto-complete results', async ({ page }) => { await page.waitForTimeout(1000) listGroup = page.getByTestId('ai-autocomplete-suggestions') - listItems = listGroup.locator('li') + listItems = listGroup.locator('[role="option"]') await expect(listItems).toHaveCount(3) await expect(listGroup).toBeVisible() expectedTexts = [ @@ -406,7 +962,7 @@ test('search from enterprise-cloud and filter by top-level Fooing', async ({ pag await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('fixture') await page.waitForTimeout(1000) - await page.getByText('View more results').click() + await page.getByTestId('view-all-results').click() const matchingResult = page .getByTestId('search-result') @@ -910,7 +1466,10 @@ test.describe('test nav at different viewports', () => { await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('serve playwright') await page.waitForTimeout(1000) - await page.getByText('View more results').click() + // Pinned outside the scroller, so it must be on screen without scrolling. The click + // alone would not catch a regression — Playwright scrolls it into view first. + await expect(page.getByTestId('view-all-results')).toBeInViewport() + await page.getByTestId('view-all-results').click() await expect(page).toHaveURL( /\/search\?search-overlay-input=serve\+playwright&query=serve\+playwright/, @@ -931,7 +1490,10 @@ test.describe('test nav at different viewports', () => { await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('serve playwright') await page.waitForTimeout(1000) - await page.getByText('View more results').click() + // Pinned outside the scroller, so it must be on screen without scrolling. The click + // alone would not catch a regression — Playwright scrolls it into view first. + await expect(page.getByTestId('view-all-results')).toBeInViewport() + await page.getByTestId('view-all-results').click() await expect(page).toHaveURL( /\/search\?search-overlay-input=serve\+playwright&query=serve\+playwright/, @@ -1330,6 +1892,7 @@ test('open search, and ask Copilot (Ask AI) a question', async ({ page }) => { await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('How do I create a Repository?') + // Pressing enter with nothing selected asks Copilot await page.keyboard.press('Enter') await expect(page.getByText('Creating a repository on GitHub')).toBeVisible() @@ -1364,12 +1927,15 @@ test('open search, Ask AI returns 400 error and shows general search results', a await page.getByTestId('toggle-search').click() await page.getByTestId('overlay-search-input').fill('foo') + // Pressing enter with nothing selected asks Copilot await page.keyboard.press('Enter') - // Search suggestions render as ActionList buttons, so scope Foo and Bar to the suggestion group. + // Wait for the general search results to appear inside the overlay's suggestions + // group. Scope the lookup to the group rather than matching page-level links of the + // same name. const generalSuggestions = page.getByTestId('general-autocomplete-suggestions') - await expect(generalSuggestions.getByRole('button', { name: 'Foo' })).toBeVisible() - await expect(generalSuggestions.getByRole('button', { name: 'Bar' })).toBeVisible() + await expect(generalSuggestions.getByRole('option', { name: 'Foo' })).toBeVisible() + await expect(generalSuggestions.getByRole('option', { name: 'Bar' })).toBeVisible() // Wait for the canned 400 response before checking the paragraph. await page.waitForTimeout(1000) diff --git a/src/frame/components/hooks/useQueryParam.ts b/src/frame/components/hooks/useQueryParam.ts index 6a7fdac5d15f..b2e1d586578c 100644 --- a/src/frame/components/hooks/useQueryParam.ts +++ b/src/frame/components/hooks/useQueryParam.ts @@ -7,7 +7,9 @@ import { parseDebug } from '@/search/components/hooks/useQuery' type UseQueryParamReturn = { debug: boolean queryParam: T - setQueryParam: (value: T) => void + // Resolves once the URL write lands. A caller about to navigate must await it: the write is + // a `router.replace` of the current path, and overlapping a `router.push` gets one cancelled. + setQueryParam: (value: T) => Promise } export function useQueryParam(queryParamKey: string, isBoolean: true): UseQueryParamReturn @@ -33,8 +35,12 @@ export function useQueryParam( setDebug(parseDebug(router.query.debug || '') || false) }, [queryParamKey, router.pathname]) - const setQueryParam = (value: string | boolean) => { + const setQueryParam = async (value: string | boolean) => { const newValue = typeof value === 'boolean' ? (value ? 'true' : '') : value + + // Always set state: the effect above skips navigations within the same dynamic route. + setQueryParamState(newValue) + const [asPathWithoutHash] = router.asPath.split('#') const [asPathRoot, asPathQuery = ''] = asPathWithoutHash.split('?') const currentParams = new URLSearchParams(asPathQuery) @@ -52,21 +58,19 @@ export function useQueryParam( newUrl = `/${newUrl}` } - router.replace(newUrl, undefined, { shallow: true, locale: router.locale, scroll: false }) - - setQueryParamState(newValue) + await router.replace(newUrl, undefined, { shallow: true, locale: router.locale, scroll: false }) } if (isBoolean) { return { debug, queryParam: queryParam as boolean, - setQueryParam: setQueryParam as (value: boolean) => void, + setQueryParam: setQueryParam as (value: boolean) => Promise, } } return { debug, queryParam: queryParam as string, - setQueryParam: setQueryParam as (value: string) => void, + setQueryParam: setQueryParam as (value: string) => Promise, } } diff --git a/src/frame/components/page-header/Header.tsx b/src/frame/components/page-header/Header.tsx index c97c3979c009..3672fd95632a 100644 --- a/src/frame/components/page-header/Header.tsx +++ b/src/frame/components/page-header/Header.tsx @@ -41,7 +41,7 @@ export const Header = ({ isNarrowMenuOpen, onNarrowMenuToggle }: Props) => { const languagePickerVisible = Object.keys(languages).length > 1 const { currentVersion } = useVersion() const { t } = useTranslation(['header', 'search']) - const { params, updateParams } = useMultiQueryParams() + const { params, updateParams, cancelPendingUpdate } = useMultiQueryParams() const { hasAccount } = useHasAccount() const signupCTAVisible = hasAccount === false && @@ -118,6 +118,7 @@ export const Header = ({ isNarrowMenuOpen, onNarrowMenuToggle }: Props) => { setIsSearchOpen={setIsSearchOpen} params={params} updateParams={updateParams} + cancelPendingParamsUpdate={cancelPendingUpdate} searchButtonRef={searchButtonRef} /> diff --git a/src/search/components/context/SearchOverlayContext.tsx b/src/search/components/context/SearchOverlayContext.tsx index fb1378ecacf9..8f3838d45a19 100644 --- a/src/search/components/context/SearchOverlayContext.tsx +++ b/src/search/components/context/SearchOverlayContext.tsx @@ -3,7 +3,8 @@ import { useQueryParam } from '@/frame/components/hooks/useQueryParam' type SearchOverlayState = { isSearchOpen: boolean - setIsSearchOpen: (open: boolean) => void + // Resolves once the URL write lands; await it before navigating away. + setIsSearchOpen: (open: boolean) => Promise } const SearchOverlayContext = createContext(undefined) diff --git a/src/search/components/helpers/execute-search-actions.ts b/src/search/components/helpers/execute-search-actions.ts index 161ace92d5b1..5c299ecf29c1 100644 --- a/src/search/components/helpers/execute-search-actions.ts +++ b/src/search/components/helpers/execute-search-actions.ts @@ -9,6 +9,31 @@ import { sanitizeSearchQuery } from '@/search/lib/sanitize-search-query' export const GENERAL_SEARCH_CONTEXT = 'general-search' export const AI_SEARCH_CONTEXT = 'ai-search' +// The overlay's own params, which never travel to a destination page. +const OVERLAY_PARAMS = [ + 'search-overlay-open', + 'search-overlay-input', + 'search-overlay-ask-ai', + 'query', +] + +// Carries tool pickers, platform switches, and feature flags to the destination URL. +// Not built from `router.query`, which also holds dynamic route params like `restPage`. +// A `restPage` search param outranks the real path, so navigation would stay on this article. +// Reads `window`, so call it only from event handlers, never during render. +export function inheritableSearchParams(): URLSearchParams { + const params = new URLSearchParams(window.location.search) + for (const key of OVERLAY_PARAMS) { + params.delete(key) + } + return params +} + +export function withInheritedParams(url: string): string { + const qs = inheritableSearchParams().toString() + return qs ? `${url}?${qs}` : url +} + export function executeGeneralSearch( router: NextRouter, currentVersion: string, @@ -41,6 +66,26 @@ export function executeGeneralSearch( router.push(asPath, undefined, { shallow: false }) } +// A real href, so cmd-click and "copy link address" work on "View all N results". +// Skips the current query string, so a copied link opens the results page without the overlay. +// Reads no `window`: this renders during SSR, and a different client href breaks hydration. +export function buildGeneralSearchPath( + router: NextRouter, + currentVersion: string, + localQuery: string, + debug = false, +) { + let asPath = `/${router.locale}` + if (currentVersion !== DEFAULT_VERSION) { + asPath += `/${currentVersion}` + } + const params = new URLSearchParams({ query: localQuery }) + if (debug) { + params.set('debug', '1') + } + return `${asPath}/search?${params}` +} + export async function executeAISearch(version: string, query: string, debug = false) { const body = { query, diff --git a/src/search/components/hooks/useMultiQueryParams.ts b/src/search/components/hooks/useMultiQueryParams.ts index 71ef77b6804b..4f2050a77745 100644 --- a/src/search/components/hooks/useMultiQueryParams.ts +++ b/src/search/components/hooks/useMultiQueryParams.ts @@ -130,5 +130,13 @@ export function useMultiQueryParams(options?: { [router], ) - return { params, updateParams } + // Call before navigating away, or the queued `router.replace` cancels the `router.push`. + const cancelPendingUpdate = useCallback(() => { + if (pushTimeoutRef.current) { + clearTimeout(pushTimeoutRef.current) + pushTimeoutRef.current = null + } + }, []) + + return { params, updateParams, cancelPendingUpdate } } diff --git a/src/search/components/input/AskAIResults.module.scss b/src/search/components/input/AskAIResults.module.scss index a69e6eba9515..e3f77fa64e34 100644 --- a/src/search/components/input/AskAIResults.module.scss +++ b/src/search/components/input/AskAIResults.module.scss @@ -1,7 +1,7 @@ @import "@primer/css/support/variables/layout.scss"; -$bodyPadding: 0 16px 0px 16px; -$mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); +$bodyPadding: 0; +$mutedTextColor: var(--brand-color-text-muted, var(--fgColor-muted, #656d76)); .container { max-width: 100%; @@ -10,8 +10,10 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); .disclaimerText { display: block; - font-size: small !important; + font-size: 14px !important; + font-style: italic; font-weight: var(--base-text-weight-normal, 400) !important; + line-height: 1.5; color: $mutedTextColor; margin: 8px 0px 18px 0px; padding: $bodyPadding; @@ -22,8 +24,27 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); } } +.answerRow { + display: flex; + align-items: flex-start; + gap: 12px; + width: 100%; +} + +.answerGlyph { + flex-shrink: 0; + // Align with the first line's optical center. + margin-top: 2px; + color: var(--brand-color-text-muted); +} + .markdownBodyOverrides { - font-size: small; + // Let it shrink so long code spans wrap instead of overflowing. + flex: 1 1 auto; + min-width: 0; + font-family: var(--brand-body-fontFamily); + font-size: 14px; + line-height: 1.5; margin-top: 4px; margin-bottom: 16px; padding: $bodyPadding; @@ -39,13 +60,70 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); } } -.referencesTitle { +// Presentational: this sits inside the listbox, which can't own a heading element. +.answerHeading { margin: 0; - padding-left: 0 !important; + padding: 24px 0; + font-size: 14px; + font-weight: 700; + line-height: 1.2; + color: var(--brand-color-text-muted); +} + +// An h3 outside the listbox, directly above the sources group it names. Its top margin is +// the gap the old wrapper around title and rows used to carry. +.referencesTitle { + margin: 12px 0 0; + padding: 24px 0 8px; + font-size: 14px; + font-weight: 700; + line-height: 1.2; + color: var(--brand-color-text-muted); +} + +// Source rows are options too, so they match the docs rows' size and active state. +.referenceRow { + position: relative; + display: flex; + align-items: center; + gap: 12px; + min-height: 56px; + // Same 8px inset as docs rows, since these are docs links too. + padding: 16px 8px; + cursor: pointer; + font-size: 16px; + font-weight: 500; + line-height: 1.5; + color: var(--brand-color-text-default); +} + +.referenceRow:hover { + background-color: rgba(129, 139, 152, 0.1); + border-radius: 6px; +} + +.referenceRowActive { + background-color: rgba(129, 139, 152, 0.15); + border-radius: 6px; + font-weight: 600; + // Blue, not brand's green accent, which reads as "success". + box-shadow: inset 0 0 0 2px + var(--fgColor-accent, var(--color-accent-fg, #0969da)); + + .referenceGlyph { + color: var(--fgColor-accent, var(--color-accent-fg, #0969da)); + } + + // Forced colors drop the fill and the ring, so draw the selection as a system-color outline. + @media (forced-colors: active) { + outline: 2px solid Highlight; + outline-offset: -2px; + } } -.referencesList { - margin-top: 12px !important; +.referenceGlyph { + flex-shrink: 0; + color: var(--brand-color-text-muted); } .loadingContainer { diff --git a/src/search/components/input/AskAIResults.tsx b/src/search/components/input/AskAIResults.tsx index 8766475adf52..da16c31af321 100644 --- a/src/search/components/input/AskAIResults.tsx +++ b/src/search/components/input/AskAIResults.tsx @@ -3,11 +3,10 @@ import { uniqBy } from 'lodash-es' import { executeAISearch } from '../helpers/execute-search-actions' import { useRouter } from 'next/router' import { useTranslation } from '@/languages/components/useTranslation' -import { ActionList, IconButton, Spinner } from '@primer/react' +import { IconButton, Spinner } from '@primer/react' import { CheckIcon, CopilotIcon, - FileIcon, ShareIcon, ThumbsdownIcon, ThumbsupIcon, @@ -31,15 +30,10 @@ type AIQueryResultsProps = { version: string debug: boolean setAISearchError: (isError?: boolean) => void - references: AIReference[] setReferences: (references: AIReference[]) => void - referencesIndexOffset: number - referenceOnSelect: (url: string) => void - selectedIndex: number askAIEventGroupId: React.MutableRefObject aiCouldNotAnswer: boolean setAICouldNotAnswer: (aiCouldNotAnswer: boolean) => void - listElementsRef: React.RefObject> } type AISearchResultEventParams = { @@ -51,22 +45,15 @@ type AISearchResultEventParams = { connectedEventId?: string } -const MAX_REFERENCES_TO_SHOW = 5 - export function AskAIResults({ query, version, debug, setAISearchError, - references, setReferences, - referencesIndexOffset, - referenceOnSelect, - selectedIndex, askAIEventGroupId, aiCouldNotAnswer, setAICouldNotAnswer, - listElementsRef, }: AIQueryResultsProps) { const router = useRouter() const { t } = useTranslation('search') @@ -255,12 +242,12 @@ export function AskAIResults({ break case 'SOURCES': + // Buffer until streaming ends, so rows with no DOM never join the arrow order. if (!isCancelled) { sourcesBuffer = uniqBy( sourcesBuffer.concat((parsedLine.sources ?? []) as AIReference[]), 'url', ) - setReferences(sourcesBuffer) } break @@ -281,7 +268,7 @@ export function AskAIResults({ break } - if (!isCancelled) setAnnouncement('Copilot Response Loading...') + if (!isCancelled) setAnnouncement(t('search.ai.loading_status_message')) } while (!done && !isCancelled) { @@ -354,6 +341,9 @@ export function AskAIResults({ ) setInitialLoading(false) setResponseLoading(false) + // Clear rather than announce "done": the live article is still reading the answer. + setAnnouncement('') + setReferences(sourcesBuffer) sendAISearchResultEvent({ sources: sourcesBuffer, message: messageBuffer, @@ -366,6 +356,8 @@ export function AskAIResults({ } } + // The spinner is aria-hidden to keep the listbox valid, so loading is announced here. + setAnnouncement(t('search.ai.loading_status_message')) fetchData() return () => { @@ -374,34 +366,37 @@ export function AskAIResults({ }, [query]) return ( -
- - +
+ {/* A real heading: the answer sits outside the listbox, where headings are allowed. */} +

{t('search.overlay.ai_autocomplete_list_heading')} - +

{initialLoading ? ( -
+ ) : (
+
+ + + {responseLoading ? fixIncompleteMarkdown(message) : message} + +
{!aiCouldNotAnswer && message !== '' ? ( ) : null} - - {responseLoading ? fixIncompleteMarkdown(message) : message} -
)} {!responseLoading ? ( @@ -457,50 +452,6 @@ export function AskAIResults({ >
) : null} - {!aiCouldNotAnswer && !responseLoading && references && references.length > 0 ? ( - <> - - - - {t('search.ai.references')} - - {references - .map((source, index) => { - if (index >= MAX_REFERENCES_TO_SHOW) { - return null - } - const refIndex = index + referencesIndexOffset - return ( - { - referenceOnSelect(source.url) - }} - active={refIndex === selectedIndex} - ref={(element: HTMLLIElement | null) => { - if (listElementsRef.current) { - listElementsRef.current[refIndex] = element - } - }} - > - - {source.title} - - ) - }) - .filter(Boolean)} - - - - ) : null}
{announcement}
diff --git a/src/search/components/input/SearchContext.tsx b/src/search/components/input/SearchContext.tsx index 2e7e3d8cc87f..6cafbf857152 100644 --- a/src/search/components/input/SearchContext.tsx +++ b/src/search/components/input/SearchContext.tsx @@ -1,6 +1,7 @@ -import { createContext, useContext, RefObject, SetStateAction, MutableRefObject } from 'react' +import { createContext, useContext, SetStateAction, MutableRefObject } from 'react' import type { AIReference } from '../types' import type { AutocompleteSearchHit, GeneralSearchHit } from '@/search/types' +import type { SearchOption, SearchOptionModel } from './search-options' export type TranslationFunction = ( strings: TemplateStringsArray | string, @@ -12,7 +13,6 @@ export interface AutocompleteSearchHitWithUserQuery extends AutocompleteSearchHi } export interface GeneralSearchHitWithOptions extends GeneralSearchHit { - isViewAllResults?: boolean isNoResultsFound?: boolean isSearchDocsOption?: boolean } @@ -25,8 +25,6 @@ export interface AskAIState { setAISearchError: (isError?: boolean) => void references: AIReference[] setReferences: (value: SetStateAction) => void - referencesIndexOffset: number - referenceOnSelect: (url: string) => void askAIEventGroupId: MutableRefObject aiSearchError: boolean aiCouldNotAnswer: boolean @@ -35,13 +33,13 @@ export interface AskAIState { export interface SearchContextType { t: TranslationFunction - generalSearchOptions: GeneralSearchHitWithOptions[] - aiOptionsWithUserInput: AutocompleteSearchHitWithUserQuery[] - generalSearchResultOnSelect: (selectedOption: GeneralSearchHit) => void - aiAutocompleteOnSelect: (selectedOption: AutocompleteSearchHit) => void - performGeneralSearch: () => void - selectedIndex: number - listElementsRef: RefObject> + model: SearchOptionModel + // null means nothing is selected. Drives aria-activedescendant. + activeId: string | null + // Shared by mouse and keyboard, so both dispatch the same way. + activate: (option: SearchOption) => void + // One stable callback for every row, keyed by the element's own id. + registerOption: (element: HTMLElement | null) => (() => void) | void askAIState: AskAIState showSpinner: boolean searchLoading: boolean diff --git a/src/search/components/input/SearchDialog.tsx b/src/search/components/input/SearchDialog.tsx new file mode 100644 index 000000000000..48c8d6027036 --- /dev/null +++ b/src/search/components/input/SearchDialog.tsx @@ -0,0 +1,127 @@ +import React, { useEffect, useRef, useState, type RefObject } from 'react' +import { createPortal } from 'react-dom' +import { focusTrap } from '@primer/behaviors' +import { useTheme as useBrandTheme } from '@primer/react-brand' + +import styles from './SearchOverlay.module.scss' + +type Props = { + ariaLabel: string + returnFocusRef: RefObject + initialFocusRef?: RefObject + onClose: () => void + children: React.ReactNode +} + +// @primer/react-brand has no Dialog, Overlay, or Portal, so this hand-builds the portal, +// focus trap, Escape, click-outside, and return focus. +// Portals to `document.body`: dialog-overrides.scss forces `z-index: 3` inside +// `#__primerPortalRoot__` so the version picker clears the header, which would clamp this too. +export function SearchDialog({ + ariaLabel, + returnFocusRef, + initialFocusRef, + onClose, + children, +}: Props) { + const dialogRef = useRef(null) + + // The portal sits outside the provider's element, so stamp the resolved mode on the scrim. + const { colorMode } = useBrandTheme() + // The type allows "auto", but the provider has already resolved it. + const brandColorMode = colorMode === 'dark' ? 'dark' : 'light' + + // Open state comes from the URL, so this renders during SSR with no document to portal into. + const [isMounted, setIsMounted] = useState(false) + useEffect(() => { + setIsMounted(true) + }, []) + + // Abort the focusTrap controller on cleanup, or the trap outlives the dialog and steals focus. + useEffect(() => { + if (!isMounted || !dialogRef.current) { + return + } + const controller = focusTrap(dialogRef.current, initialFocusRef?.current || undefined) + return () => controller?.abort() + }, [isMounted, initialFocusRef]) + + // Lock body scroll behind the scrim; pad for the scrollbar's width so content doesn't shift. + useEffect(() => { + if (!isMounted) { + return + } + const { body, documentElement } = document + const previousOverflow = body.style.overflow + const previousPaddingRight = body.style.paddingRight + const scrollbarWidth = window.innerWidth - documentElement.clientWidth + + body.style.overflow = 'hidden' + if (scrollbarWidth > 0) { + const currentPaddingRight = parseFloat(window.getComputedStyle(body).paddingRight) || 0 + body.style.paddingRight = `${currentPaddingRight + scrollbarWidth}px` + } + + return () => { + body.style.overflow = previousOverflow + body.style.paddingRight = previousPaddingRight + } + }, [isMounted]) + + // Listen on document to keep the scrim handler-free. Use mousedown so drags onto it don't close. + useEffect(() => { + if (!isMounted) { + return + } + const handleKeyDown = (event: KeyboardEvent) => { + // The input closes on Escape and prevents default, so this catches Escape elsewhere. + if (event.key === 'Escape' && !event.defaultPrevented) { + onClose() + } + } + const handleMouseDown = (event: MouseEvent) => { + const target = event.target + if (target instanceof Node && !dialogRef.current?.contains(target)) { + // Prevent default, or focus moves to after the dialog returns it to the opener. + event.preventDefault() + onClose() + } + } + document.addEventListener('keydown', handleKeyDown) + document.addEventListener('mousedown', handleMouseDown) + return () => { + document.removeEventListener('keydown', handleKeyDown) + document.removeEventListener('mousedown', handleMouseDown) + } + }, [isMounted, onClose]) + + // Skip if the opener left the DOM, for example after navigating to a picked result. + useEffect(() => { + return () => { + // Read at teardown: Header swaps `parentRef` between desktop and mobile buttons on resize. + const returnFocusTo = returnFocusRef.current + if (returnFocusTo && document.contains(returnFocusTo)) { + returnFocusTo.focus({ preventScroll: true }) + } + } + }, [returnFocusRef]) + + if (!isMounted) { + return null + } + + return createPortal( +
+
+ {children} +
+
, + document.body, + ) +} diff --git a/src/search/components/input/SearchGroups.tsx b/src/search/components/input/SearchGroups.tsx index 9707e0bef149..1f65832db96f 100644 --- a/src/search/components/input/SearchGroups.tsx +++ b/src/search/components/input/SearchGroups.tsx @@ -1,246 +1,218 @@ -import { ActionList, Spinner } from '@primer/react' -import { - SearchIcon, - FileIcon, - ArrowRightIcon, - CopilotIcon, - CommentIcon, -} from '@primer/octicons-react' +import type { ReactNode } from 'react' +import { Spinner } from '@primer/react' +import { SearchIcon, ArrowUpRightIcon, TriangleDownIcon, FileIcon } from '@primer/octicons-react' import { AskAIResults } from './AskAIResults' -import { useSearchContext, AutocompleteSearchHitWithUserQuery } from './SearchContext' +import { useSearchContext } from './SearchContext' +import { VIEW_ALL_OPTION_ID, type ReferenceOption, type SearchOption } from './search-options' import styles from './SearchOverlay.module.scss' +import answerStyles from './AskAIResults.module.scss' +type OptionRowProps = { + option: SearchOption + label: string + // Figma gives AI rows a caret and docs rows an arrow-up-right. + trailingIcon: ReactNode +} + +function OptionRow({ option, label, trailingIcon }: OptionRowProps) { + const { activeId, activate, registerOption } = useSearchContext() + const isActive = activeId === option.id + // Figma insets docs-row separators 4px from each end. + const isDocs = option.kind === 'general' || option.kind === 'search-docs' + + return ( + // No key handler: selection is virtual, so the combobox input handles every key. + // eslint-disable-next-line jsx-a11y/click-events-have-key-events +
event.preventDefault()} + onClick={() => activate(option)} + > + {label} + +
+ ) +} + +// A Copilot source. Matches the docs rows' size and active state, with a leading file glyph. +function SourceRow({ option }: { option: ReferenceOption }) { + const { activeId, activate, registerOption } = useSearchContext() + const isActive = activeId === option.id + + return ( + // No key handler: selection is virtual, so the combobox input handles every key. + // eslint-disable-next-line jsx-a11y/click-events-have-key-events +
event.preventDefault()} + onClick={() => activate(option)} + > +
+ ) +} + +const SOURCES_HEADING_ID = 'search-sources-heading' + +// Renders the Copilot answer and the listbox. The answer, with its prose, links and feedback +// buttons, sits outside the listbox: a listbox may only hold options and groups, and some +// screen readers expose nothing else inside one. Headings are allowed out here too, so the +// answer and its sources get real ones. export function SearchGroups() { - const { - t, - generalSearchOptions, - aiOptionsWithUserInput, - generalSearchResultOnSelect, - aiAutocompleteOnSelect, - performGeneralSearch, - selectedIndex, - listElementsRef, - askAIState, - showSpinner, - searchLoading, - previousSuggestionsListHeight, - } = useSearchContext() + const { t, model, askAIState, showSpinner, searchLoading, previousSuggestionsListHeight } = + useSearchContext() const isInAskAIState = askAIState?.isAskAIState && !askAIState.aiSearchError - const isInAskAIStateButNoAnswer = isInAskAIState && askAIState.aiCouldNotAnswer + + // The pinned view-all renders outside this element, below the scroller; aria-owns makes it + // a child of the listbox, so aria-activedescendant can name it. + const listboxProps = { + id: 'search-listbox', + role: 'listbox', + 'aria-label': t('search.overlay.suggestions_list_aria_label'), + 'aria-owns': model.viewAll ? VIEW_ALL_OPTION_ID : undefined, + } as const // Reuse this spinner for autocomplete; Ask AI streaming shows its own spinner. if (showSpinner && !isInAskAIState) { return ( -
- -
+ <> + + {/* Kept mounted, empty and busy, so the input's aria-controls still resolves. */} +
+ ) } const groups = [] - // Show general search suggestions above the Ask AI section when Ask AI cannot answer. - if (generalSearchOptions.length || isInAskAIStateButNoAnswer) { - const items = [] - for (let index = 0; index < generalSearchOptions.length; index++) { - const option = generalSearchOptions[index] - if (option.isNoResultsFound) { - items.push( - - {option.title} - , - ) - // No-results ends the general list. - break - // When autocomplete fails, let the user's query fall back to docs search. - } else if (option.isSearchDocsOption) { - const isActive = selectedIndex === index - items.push( - performGeneralSearch()} - aria-label={t('search.overlay.search_docs_with_query').replace('{query}', option.title)} - ref={(element: HTMLLIElement | null) => { - if (listElementsRef.current) { - listElementsRef.current[index] = element - } - }} - > - - - - {option.title} - - - - , - ) - } else if (option.title) { - const isActive = selectedIndex === index - items.push( - - option.isViewAllResults ? performGeneralSearch() : generalSearchResultOnSelect(option) - } - className={option.isViewAllResults ? styles.viewAllSearchResults : ''} - active={isActive} - tabIndex={-1} - ref={(element: HTMLLIElement | null) => { - if (listElementsRef.current) { - listElementsRef.current[index] = element - } - }} - > - {!option.isNoResultsFound && ( - - - - )} - {option.title} - - - - , - ) - } - } + if (isInAskAIState && model.references.length) { + groups.push( +
+ {model.references.map((option) => ( + + ))} +
, + ) + } + + if (model.ai.length && !isInAskAIState) { + groups.push( +
+ + {model.ai.map((option) => ( + } + /> + ))} +
, + ) + } + if (model.general.length || model.noResults) { groups.push( - - +
+ {searchLoading && isInAskAIState ? ( + ) : model.noResults ? ( + // Not an option, so arrows need no skip rule. The overlay's live region announces it. +
{t('search.overlay.no_results_found')}
) : ( - items - )} - , - ) - - if (isInAskAIState || isInAskAIStateButNoAnswer) { - groups.push() - } - - if (isInAskAIState) { - groups.push( - -
  • - ( + : } /> -
    • -     , - ) - } - - // Hide the bottom divider for no-answer, AI-error, and empty-suggestions states. - if ( - !isInAskAIState && - !askAIState.aiSearchError && - generalSearchOptions.filter((option) => !option.isViewAllResults && !option.isNoResultsFound) - .length && - aiOptionsWithUserInput.length - ) { - groups.push() - } - } - - if (aiOptionsWithUserInput.length && !isInAskAIState) { - groups.push( - - - - {t('search.overlay.ai_autocomplete_list_heading')} - - {aiOptionsWithUserInput.map((option: AutocompleteSearchHitWithUserQuery, index: number) => { - // Since general search comes first, we need to add an offset for AI suggestions - const indexWithOffset = generalSearchOptions.length + index - const isActive = selectedIndex === indexWithOffset - const item = ( - aiAutocompleteOnSelect(option)} - active={isActive} - tabIndex={-1} - ref={(element: HTMLLIElement | null) => { - if (listElementsRef.current) { - listElementsRef.current[indexWithOffset] = element - } - }} - > - - - - {option.term} - - - - - ) - return item - })} - , + )) + )} +
    , ) } - return <>{groups} + return ( + <> + {/* Rendered even with no docs results, so the answer never depends on them. */} + {isInAskAIState && ( + + )} + {isInAskAIState && model.references.length > 0 && ( +

    + {t('search.ai.references')} +

    + )} +
    {groups}
    + + ) } diff --git a/src/search/components/input/SearchOverlay.module.scss b/src/search/components/input/SearchOverlay.module.scss index cec958b78bd5..4907f3269ff4 100644 --- a/src/search/components/input/SearchOverlay.module.scss +++ b/src/search/components/input/SearchOverlay.module.scss @@ -5,86 +5,276 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); -.overlayBackdrop { - position: fixed; - top: 0; - left: 0; - width: 100%; - height: 100%; - background-color: var( - --overlay-backdrop-bgColor, - var(--color-primer-fg-canvas-backdrop, rgba(31, 35, 40, 0.5)) +// Figma's dashed rule: 3px dashes, 3px gaps. A gradient fixes the dash length, which +// `border-style: dashed` leaves to the browser, and adds no height to rows. +@mixin dashedRuleGeometry($inset: 0px) { + background-size: calc(100% - #{$inset * 2}) 1px; + background-position: $inset 0; +} + +@mixin dashedRule($inset: 0px) { + background-image: repeating-linear-gradient( + to right, + var(--brand-color-border-subtle) 0 3px, + transparent 3px 6px ); - // Keep the backdrop above page content and below the overlay. - z-index: 1000; + background-repeat: no-repeat; + @include dashedRuleGeometry($inset); } -.overlayContainer { - // Place the overlay above the backdrop. - z-index: 1001; - top: 0; - left: 0; - width: searchVariables.$smSearchOverlayWidth !important; - max-width: 100%; - height: auto; - max-height: 95vh; - overflow-y: auto; +// The dialog is a child, not a sibling, so a mousedown in the gutter counts as click-outside. +.scrim { + position: fixed; + inset: 0; + z-index: 1000; display: flex; flex-direction: column; - justify-content: center; align-items: center; - @include breakpoint(sm) { - top: 0 !important; - left: 0 !important; - width: searchVariables.$smSearchOverlayWidth !important; + background-color: searchVariables.$searchScrimColor; + backdrop-filter: blur(25px); + + // `[data-color-mode]:not(html)` in frame/stylesheets/index.scss clears nested theme + // backgrounds at specificity (0,1,1), so a bare `.scrim` loses and the dim disappears. + &[data-color-mode] { + background-color: searchVariables.$searchScrimColor; + } + + overflow-y: auto; + overscroll-behavior: contain; + + @include breakpoint(md) { + justify-content: center; + padding: searchVariables.$searchDialogGutter; } +} + +// Full-bleed below `md`. From `md` up, the Figma's floating card. +.dialog { + width: 100%; + min-height: 100%; + // Without a cap, the scrim scrolls the overflow and pushes the pinned footer off-screen. + max-height: 100%; + + display: flex; + flex-direction: column; + + // Mona Sans, loaded by @primer/react-brand/fonts/fonts.css. + font-family: var(--brand-body-fontFamily); + background-color: var(--brand-color-canvas-default); + + // Match brand's antialiased type. Primer's subpixel default renders the same weight heavier. + -webkit-font-smoothing: antialiased; + -moz-osx-font-smoothing: grayscale; @include breakpoint(md) { - top: 0 !important; - left: 0 !important; - width: searchVariables.$mdSearchOverlayWidth !important; + max-width: searchVariables.$searchDialogMaxWidth; + min-height: 0; + max-height: calc(100vh - #{searchVariables.$searchDialogGutter} * 2); + // Unlike `justify-content: center` on the scrim, this scrolls an over-tall card from its top. + margin: auto; + padding: 16px 16px 32px; + gap: 24px; + + border: 1px solid var(--brand-color-border-default); + border-radius: 16px; + box-shadow: 0 0 34px rgba(0, 0, 0, 0.1); + overflow-y: auto; } - @include breakpoint(lg) { - // Offset by twice the header's 8px p-2 padding. - top: 16px !important; - left: calc(50vw - searchVariables.$lgSearchOverlayWidth / 2) !important; - width: searchVariables.$lgSearchOverlayWidth !important; + // Type from Figma node 446-105514. The field is @primer/react's TextInput, since brand's + // has no interactive trailing action for the clear button. The rules below restyle it. + + input { + font-family: inherit; + font-size: 18px; + font-weight: 480; + line-height: 1.5; + letter-spacing: 0.18px; } - @include breakpoint(xl) { - top: 16px !important; - left: calc(50vw - searchVariables.$xlSearchOverlayWidth / 2) !important; - width: searchVariables.$xlSearchOverlayWidth !important; + // Figma draws the field as a bare icon and text on the canvas, so strip Primer's input box. + [class*="TextInput-wrapper"] { + height: 40px; + padding-left: 0; + background-color: transparent; + border: 0; + border-radius: 0; + box-shadow: none; + + // Focus stays in the input while arrow keys move through results, so a :focus-within + // ring would always show and convey nothing. The caret marks focus. + outline: none; + + &::after { + display: none; + } + } + + // Figma puts an 18px glyph 24px from the query text. Primer ships 16px and 8px. + :global(.TextInput-icon) { + margin-right: 24px; + + svg { + width: 18px; + height: 18px; + } + } + + .dividerTopMargin { + margin: 0; } } +// Wraps the scroller and the pinned view-all link, so both are valid aria-activedescendant targets. +.listbox { + display: flex; + flex-direction: column; + min-height: 0; + flex: 1 1 auto; +} + +// Presentational only: aria-label names each group, and a listbox can't own a heading element. +.groupHeading { + padding: 24px 0; + font-family: inherit; + font-size: 14px; + font-weight: 700; + line-height: 1.2; + color: var(--brand-color-text-muted); +} + +// The first heading already has the dialog's 24px flex gap above it. +[role="group"]:first-child > .groupHeading { + padding-top: 0; +} + +.option { + position: relative; + display: flex; + align-items: center; + gap: 16px; + min-height: 56px; + // Figma insets AI rows 4px, but that puts text 2px off the active ring, so all rows use 8px. + padding: 16px 8px; + cursor: pointer; + + font-family: inherit; + font-size: 16px; + // Figma says 550, but in "Mona Sans WT3 VF". Brand's "Mona Sans" is heavier at the same + // weight, so 500 matches the design visually. + font-weight: 500; + line-height: 1.5; + letter-spacing: 0.16px; + color: var(--brand-color-text-default); + + & + & { + @include dashedRule; + } +} + +// Figma insets docs-row rules 4px at each end. Set geometry only, never `background-image`, +// so the active row's `background-image: none` still wins. +.optionDocs + .optionDocs { + @include dashedRuleGeometry(4px); +} + +$activeIndicatorColor: var(--fgColor-accent, var(--color-accent-fg, #0969da)); +// Translucent, so they work in both color modes. Same values as the rest of docs.github.com. +$optionHoverFill: rgba(129, 139, 152, 0.1); +$optionActiveFill: rgba(129, 139, 152, 0.15); + +// Figma has no selected state. The ring carries the contrast because the fill alone is too +// faint to track. Hover gets a lighter fill and no ring, so it never looks selected. +.option:hover { + background-color: $optionHoverFill; + border-radius: 6px; +} + +.optionActive { + background-color: $optionActiveFill; + border-radius: 6px; + font-weight: 600; + // Inset, so the scroller can't clip it. Not brand's accent: that green reads as "success". + box-shadow: inset 0 0 0 2px $activeIndicatorColor; + + .optionTrailing { + color: $activeIndicatorColor; + } + + // Forced colors drop the fill and the ring, so draw the selection as a system-color outline. + @media (forced-colors: active) { + outline: 2px solid Highlight; + outline-offset: -2px; + } +} + +// Drop the rules around the active row, since a rounded fill against a rule looks clipped. +// Two classes, to beat the specificity of `.option + .option`. +.option.optionActive, +.optionActive + .option { + background-image: none; +} + +.optionLabel { + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.optionTrailing { + display: flex; + flex-shrink: 0; + align-items: center; + // Center the 16px octicon in Figma's 20px glyph box. + justify-content: center; + width: 20px; + height: 20px; + color: var(--brand-color-text-muted); +} + .header { display: flex; - padding: 12px 14px 0px 14px !important; + align-items: center; width: 100%; - background-color: var(--overlay-bgColor) !important; + min-height: 40px; + // Below `md` the dialog is full-bleed, so the header keeps its own inset. + padding: 12px 14px 0 14px; + + @include breakpoint(md) { + padding: 0; + } } +// Figma spaces this 12px from the field's clear button. +.closeButton { + flex-shrink: 0; + margin-left: 12px; +} + +// Outside the listbox, so the disclaimer stays put while results scroll. .footer { - padding: 5px 16px 13px 16px; + display: flex; + flex-direction: column; + align-items: flex-start; + gap: 12px; + flex-shrink: 0; + width: 100%; - background-color: var(--overlay-bgColor); - overflow-y: auto; - min-height: 15vh; - border-top: 1px solid - var(--borderColor-default, var(--color-border-default, #d0d7de)); + // Below `md` the dialog has no flex `gap`, so this adds 12px under the view-all link. + padding: 12px 16px 13px 16px; + // No border-top: Figma closes the card with space, not a rule. - @include breakpoint(sm) { - min-height: fit-content !important; + @include breakpoint(md) { + padding: 0; } } .betaToken { color: var(--fgColor-success, var(--fgColor-open, green)) !important; - background-color: var(--overlay-bgColor); margin-right: 1em; height: 22px !important; font-weight: 700 !important; @@ -101,11 +291,56 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); .suggestionsList { width: 100%; max-width: 100%; - // In the rare viewport case where the results are taller than the viewport, scroll + // This list takes the overflow, so the view-all link and footer below it stay put. max-height: 95vh; - overflow-y: auto; overflow-x: hidden; - padding: 4px 0 4px 0 !important; + padding: 0 !important; + + // Force a visible scrollbar, since macOS overlay scrollbars hide when idle. + // Safari ignores `scrollbar-gutter: stable` when content fits, so rows would shift while typing. + overflow-y: scroll; + + // Put the 12px scrollbar lane in the card's right padding, so rows line up with the header + // and footer. Only WebKit/Blink reserve the lane, and only `md` and up has padding for it. + @supports selector(::-webkit-scrollbar) { + @include breakpoint(md) { + // Widen instead of a negative margin, which can't grow a box with an explicit width. + width: calc(100% + 12px); + max-width: calc(100% + 12px); + } + } + + &::-webkit-scrollbar { + width: 12px; + } + + &::-webkit-scrollbar-track { + background: transparent; + } + + &::-webkit-scrollbar-thumb { + // Text-muted, not border-default: border-default is about 1.9:1 on the dark canvas, + // under the WCAG 1.4.11 3:1 minimum. The transparent border insets the pill. + background-color: color-mix( + in srgb, + var(--brand-color-text-muted) 75%, + transparent + ); + background-clip: padding-box; + border: 3px solid transparent; + border-radius: 6px; + } + + // Firefox only: Chrome drops all ::-webkit-scrollbar rules if scrollbar-width/color is set. + @supports not selector(::-webkit-scrollbar) { + scrollbar-width: thin; + scrollbar-color: color-mix( + in srgb, + var(--brand-color-text-muted) 75%, + transparent + ) + transparent; + } } .errorBanner { @@ -114,11 +349,42 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); margin-bottom: 16px; } -.viewAllSearchResults { - button { - span { - color: var(--color-accent-emphasis) !important; - font-weight: 500 !important; +.viewAllResults { + // Brand's Link centers its label, so a stretched one renders as a centered bar. + // Set here, not on the parent, because the sibling scroller needs full width. + align-self: flex-start; + flex-shrink: 0; + + // 14px plus the 10px padding puts the label 24px below the last row, per Figma. + margin-top: 14px; + // Pad the 24px target to 44px and frame the selected fill. Negative margins cancel the + // padding so layout holds. 10px, not 12px, keeps the overhang inside the 12px above the footer. + padding: 10px 8px; + margin-bottom: -10px; + margin-inline: -8px; + border-radius: 6px; + + // Brand positions its underline against the link box, which the padding enlarged, so move + // it back under the text. Keep brand's trailing-visual terms so `arrowDirection` still works. + &::after { + bottom: calc(10px - 0.15em); + left: 8px; + width: calc(100% - 16px - var(--gap) - var(--visual-width)); + } + + // Arrow-key selection: the fill hugs the label and the underline shows as on hover, so it + // still reads as a link, not a result row. + &[aria-selected="true"] { + background-color: $optionActiveFill; + + &::after { + transform: scaleX(1); + } + + // Forced colors drop the fill, so draw the selection as a system-color outline. + @media (forced-colors: active) { + outline: 2px solid Highlight; + outline-offset: -2px; } } } @@ -163,23 +429,34 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); display: block; margin-top: 16px; width: 100%; + // Replace the global `hr` solid border with the rows' dashed rule. + height: 1px; + border-bottom: 0; + @include dashedRule; } -.dividerTopMarginHidden { - display: none; - margin-top: 16px; - width: 100%; -} +// The input's focus indicator. Primer's ring is stripped because the input holds focus +// nearly all the time; instead this rule turns solid accent while the input is focused, +// and drops back to dashed when Tab moves to the close button or into an answer. The +// second pixel is a shadow, so the layout doesn't move. +.header:has(input:focus) + .dividerTopMargin { + background-image: none; + background-color: $activeIndicatorColor; + box-shadow: 0 1px 0 $activeIndicatorColor; -.dividerFullWidth { - width: 100%; + // Forced colors drop backgrounds and shadows; a border survives as a system color. + @media (forced-colors: active) { + border-bottom: 2px solid Highlight; + } } .privacyDisclaimer { - color: var(--color-fg-muted); - margin-top: 0.5rem; + color: var(--brand-color-text-muted); + // The footer's `gap` handles spacing, so clear the default paragraph margins. + margin-top: 0; margin-bottom: 0; - font-size: small; + font-size: 14px; + line-height: 1.5; } .trailingVisualActive { @@ -191,11 +468,3 @@ $mutedTextColor: var(--fgColor-muted, var(--color-fg-muted, #656d76)); visibility: hidden; width: 1rem; } - -.leadingVisualHidden { - visibility: hidden; -} - -.leadingVisualVisible { - visibility: visible; -} diff --git a/src/search/components/input/SearchOverlay.tsx b/src/search/components/input/SearchOverlay.tsx index ebecf690624a..f220de91bc63 100644 --- a/src/search/components/input/SearchOverlay.tsx +++ b/src/search/components/input/SearchOverlay.tsx @@ -1,23 +1,22 @@ -import React, { useState, useRef, RefObject, useEffect, useMemo } from 'react' -import cx from 'clsx' +import React, { useState, useRef, RefObject, useEffect, useMemo, useCallback } from 'react' import { useRouter } from 'next/router' -import { ActionList, IconButton, Overlay, TextInput, Banner } from '@primer/react' -import { Stack } from '@primer/react-brand' -import { SearchIcon, XCircleFillIcon, CopilotIcon, ArrowLeftIcon } from '@primer/octicons-react' -import { focusTrap } from '@primer/behaviors' +import { IconButton, TextInput, Banner } from '@primer/react' +import { Link as BrandLink, Stack } from '@primer/react-brand' +import { SearchIcon, XCircleFillIcon, XIcon, ArrowLeftIcon } from '@primer/octicons-react' import { useTranslation } from '@/languages/components/useTranslation' import { useVersion } from '@/versions/components/useVersion' import { AI_SEARCH_CONTEXT, + buildGeneralSearchPath, executeGeneralSearch, GENERAL_SEARCH_CONTEXT, + withInheritedParams, } from '../helpers/execute-search-actions' import { useCombinedSearchResults } from '@/search/components/hooks/useAISearchAutocomplete' import { sendEvent, uuidv4 } from '@/events/components/events' import { EventType } from '@/events/types' import { ASK_AI_EVENT_GROUP, SEARCH_OVERLAY_EVENT_GROUP } from '@/events/components/event-groups' -import { useSharedUIContext } from '@/frame/components/context/SharedUIContext' import type { AIReference } from '../types' import type { AutocompleteSearchHit, GeneralSearchHit } from '@/search/types' @@ -25,12 +24,9 @@ import type { AutocompleteSearchHit, GeneralSearchHit } from '@/search/types' import { sanitizeSearchQuery } from '@/search/lib/sanitize-search-query' import { MAX_QUERY_LENGTH } from '@/search/lib/ai-search-constants' -import { - SearchContext, - SearchContextType, - AutocompleteSearchHitWithUserQuery, - GeneralSearchHitWithOptions, -} from './SearchContext' +import { SearchContext, SearchContextType } from './SearchContext' +import { buildSearchOptions, VIEW_ALL_OPTION_ID, type SearchOption } from './search-options' +import { SearchDialog } from './SearchDialog' import { SearchGroups } from './SearchGroups' import styles from './SearchOverlay.module.scss' import { RenderedHTML } from '@/frame/components/ui/RenderedHTML/RenderedHTML' @@ -39,7 +35,11 @@ type Props = { searchOverlayOpen: boolean parentRef: RefObject debug: boolean + // Escape, scrim click, or the close button. Writes the URL. onClose: () => void + // Closes with a URL write and resolves once it lands. Await it, then navigate: the two + // must not overlap, or Next cancels one of them. + onNavigateAway: () => Promise params: { 'search-overlay-input': string 'search-overlay-ask-ai': string @@ -57,6 +57,7 @@ export function SearchOverlay({ parentRef, debug, onClose, + onNavigateAway, params, updateParams, }: Props) { @@ -68,30 +69,32 @@ export function SearchOverlay({ const isAskAIState = params['search-overlay-ask-ai'] === 'true' const inputRef = useRef(null) - const suggestionsListHeightRef = useRef(null) - // Keep list item refs so keyboard navigation can scroll the selected option into view. - const listElementsRef = React.useRef>([]) + const suggestionsListHeightRef = useRef(null) + // Keyed by option id, not index, so it can't drift out of sync with the option list. + const optionElements = useRef(new Map()) + + // One stable callback for every row, so React doesn't reattach refs and empty the map. + const registerOption = useCallback((element: HTMLElement | null) => { + if (!element) { + return + } + const { id } = element + optionElements.current.set(id, element) + return () => { + optionElements.current.delete(id) + } + }, []) - const [selectedIndex, setSelectedIndex] = useState(-1) + const [activeId, setActiveId] = useState(null) const [aiQuery, setAIQuery] = useState(urlSearchInputQuery) const [aiSearchError, setAISearchError] = useState(false) const [aiReferences, setAIReferences] = useState([] as AIReference[]) const [aiCouldNotAnswer, setAICouldNotAnswer] = useState(false) const [showSpinner, setShowSpinner] = useState(false) - const [scrollPos, setScrollPos] = useState(0) const [announcement, setAnnouncement] = useState('') - const { hasOpenHeaderNotifications } = useSharedUIContext() - // Group overlay selection and keyboard events that pass this session ID. const searchEventGroupId = useRef('') - const overlayRef = useRef(null) - - useEffect(() => { - if (searchOverlayOpen && overlayRef.current) { - focusTrap(overlayRef.current, inputRef.current || undefined) - } - }, [searchOverlayOpen]) useEffect(() => { searchEventGroupId.current = uuidv4() @@ -99,19 +102,6 @@ export function SearchOverlay({ // Each Ask AI session gets its own event group. const askAIEventGroupId = useRef('') - // Header notifications push the fixed overlay down until the page scrolls past them. - useEffect(() => { - if (hasOpenHeaderNotifications) { - const handleScroll = () => { - setScrollPos(window.scrollY) - } - - window.addEventListener('scroll', handleScroll) - return () => window.removeEventListener('scroll', handleScroll) - } - }, [hasOpenHeaderNotifications]) - const overlayTopValue = scrollPos > 72 ? '0px' : `${88 - scrollPos}px !important` - const { autoCompleteOptions, searchLoading, @@ -157,84 +147,75 @@ export function SearchOverlay({ autoCompleteSearchError, ]) - // Drop the typed-query duplicate; userInputOptions adds it back with isUserQuery. - const filteredAIOptions = aiAutocompleteOptions.filter( - (option) => option.term !== urlSearchInputQuery, + // Results can outlive an emptied input, so check the query or the link offers `?query=`. + const showViewAllResults = generalSearchResults.length > 0 && urlSearchInputQuery.trim() !== '' + + const viewAllResultsHref = buildGeneralSearchPath( + router, + currentVersion, + urlSearchInputQuery, + debug, ) - const userInputOptions = - urlSearchInputQuery.trim() !== '' - ? [ - { - term: urlSearchInputQuery, - title: urlSearchInputQuery, - highlights: [], - isUserQuery: true, - }, - ] - : [] - - const [combinedOptions, generalOptionsWithViewStatus, aiOptionsWithUserInput] = useMemo(() => { - setAnnouncement('') - let generalWithView = [...generalSearchResults] - const aiWithUser = [...userInputOptions, ...filteredAIOptions] - const combined = [] as Array<{ - group: 'general' | 'ai' | string - url?: string - option: AutocompleteSearchHitWithUserQuery | GeneralSearchHitWithOptions - }> - - if (generalSearchResults.length > 0) { - generalWithView.push({ - title: t('search.overlay.view_all_search_results'), - isViewAllResults: true, - } as unknown as GeneralSearchHitWithOptions) - } else if (autoCompleteSearchError) { - if (urlSearchInputQuery.trim() !== '') { - generalWithView.push({ - ...(userInputOptions[0] || {}), - isSearchDocsOption: true, - } as unknown as GeneralSearchHit) - } - } else if (urlSearchInputQuery.trim() !== '' && !searchLoading) { - setAnnouncement(t('search.overlay.no_results_found_announcement')) - generalWithView.push({ - title: t('search.overlay.no_results_found'), - isNoResultsFound: true, - } as unknown as GeneralSearchHitWithOptions) - } else { - generalWithView = [] - } - // Keep general options before AI options because selectedIndex indexes this combined array. - combined.push(...generalWithView.map((option) => ({ group: 'general', option }))) - // Add AI suggestions and user input only outside Ask AI and AI-error states. - if (!aiSearchError && !isAskAIState) { - combined.push(...aiWithUser.map((option) => ({ group: 'ai', option }))) - } else if (isAskAIState && !aiCouldNotAnswer) { - // Ask AI references become keyboard-navigable options after results replace suggestions. - combined.push( - ...aiReferences.map((option) => ({ - group: 'reference', - url: option.url, - option: { - term: option.title, - highlights: [], - isUserQuery: false, - }, - })), - ) - } + // Match SearchGroups: the spinner hides rows unless ask-AI is streaming with its own spinner. + const suggestionsHidden = showSpinner && !(isAskAIState && !aiSearchError) + + const model = useMemo( + () => + buildSearchOptions({ + query: urlSearchInputQuery, + isAskAIState, + searchLoading, + suggestionsHidden, + aiSearchError, + aiCouldNotAnswer, + autoCompleteSearchError, + aiAutocompleteOptions, + aiReferences, + generalSearchResults, + showViewAllResults, + viewAllHref: viewAllResultsHref, + viewAllLabel: totalGeneralSearchResults + ? t('search.overlay.view_all_search_results_with_count').replace( + '{{n}}', + // The page's language, not the browser's: a German page shows "1.234" even + // in an English browser. + totalGeneralSearchResults.toLocaleString(router.locale), + ) + : t('search.overlay.view_all_search_results'), + }), + [ + urlSearchInputQuery, + isAskAIState, + searchLoading, + suggestionsHidden, + aiSearchError, + aiCouldNotAnswer, + autoCompleteSearchError, + aiAutocompleteOptions, + aiReferences, + generalSearchResults, + showViewAllResults, + viewAllResultsHref, + totalGeneralSearchResults, + router.locale, + t, + ], + ) + const { options } = model + const activeIndex = activeId ? options.findIndex((option) => option.id === activeId) : -1 - return [combined, generalWithView, aiWithUser] - }, [ - generalSearchResults, - totalGeneralSearchResults, - urlSearchInputQuery, - aiSearchError, - aiReferences, - isAskAIState, - autoCompleteSearchError, - ]) + // Announce from an effect, not the memo, so the live region doesn't re-fire on every recompute. + useEffect(() => { + setAnnouncement(model.noResults ? t('search.overlay.no_results_found_announcement') : '') + }, [model.noResults, t]) + + // Clear a selection whose row new results unmounted, so aria-activedescendant names a real row. + useEffect(() => { + if (activeId && activeIndex === -1) { + setActiveId(null) + } + }, [activeId, activeIndex]) // Focus manually with preventScroll because Primer Overlay initialFocusRef scrolls to the top. useEffect(() => { @@ -268,13 +249,17 @@ export function SearchOverlay({ aiCouldNotAnswer, ]) - // Keyboard control refs must track the current option count. + // Skip the view-all option: it sits outside the scroller, so scrolling to it moves the scrim. useEffect(() => { - listElementsRef.current = listElementsRef.current.slice( - 0, - generalOptionsWithViewStatus.length + aiOptionsWithUserInput.length, - ) - }, [generalOptionsWithViewStatus, aiOptionsWithUserInput]) + if (!activeId) { + return + } + const element = optionElements.current.get(activeId) + if (!element || !suggestionsListHeightRef.current?.contains(element)) { + return + } + element.scrollIntoView({ behavior: 'smooth', block: 'center' }) + }, [activeId]) // Estimate loading space from result counts, or reserve 150px for two suggestions. const previousSuggestionsListHeight = useMemo(() => { @@ -288,7 +273,7 @@ export function SearchOverlay({ const handleSearchQueryChange = (event: React.ChangeEvent) => { event.preventDefault() const newQuery = event.target.value - setSelectedIndex(-1) + setActiveId(null) setSearchLoading(true) updateAutocompleteResults(newQuery) // Changing the query leaves the Ask AI state. @@ -304,7 +289,11 @@ export function SearchOverlay({ } } - const generalSearchResultOnSelect = (selectedOption: GeneralSearchHit) => { + // Take `generalRank` from the option, not the cursor, because mouse clicks have no cursor. + const generalSearchResultOnSelect = async ( + selectedOption: GeneralSearchHit, + generalRank: number, + ) => { sendEvent({ type: EventType.search, search_query: sanitizeSearchQuery(urlSearchInputQuery), @@ -315,28 +304,16 @@ export function SearchOverlay({ sendEvent({ type: EventType.searchResult, search_result_query: sanitizeSearchQuery(urlSearchInputQuery), - search_result_index: selectedIndex, + search_result_index: generalRank, search_result_total: totalGeneralSearchResults, search_result_url: selectedOption.url || '', - search_result_rank: (totalGeneralSearchResults - selectedIndex) / totalGeneralSearchResults, + search_result_rank: (totalGeneralSearchResults - generalRank) / totalGeneralSearchResults, eventGroupKey: SEARCH_OVERLAY_EVENT_GROUP, eventGroupId: searchEventGroupId.current, }) - const searchParams = new URLSearchParams((router.query as Record) || {}) - if (searchParams.has('search-overlay-open')) { - searchParams.delete('search-overlay-open') - } - if (searchParams.has('search-overlay-input')) { - searchParams.delete('search-overlay-input') - } - if (searchParams.has('search-overlay-ask-ai')) { - searchParams.delete('search-overlay-ask-ai') - } - if (searchParams.has('query')) { - searchParams.delete('query') - } - router.push(`${selectedOption.url}?${searchParams.toString()}`) - onClose() + const destination = withInheritedParams(selectedOption.url || '') + await onNavigateAway() + router.push(destination) } // AI results replace suggestions, so keep focus in the input after selection. @@ -351,7 +328,7 @@ export function SearchOverlay({ eventGroupKey: ASK_AI_EVENT_GROUP, eventGroupId: askAIEventGroupId.current, }) - setSelectedIndex(-1) + setActiveId(null) setSearchLoading(true) updateParams({ 'search-overlay-ask-ai': 'true', @@ -362,9 +339,9 @@ export function SearchOverlay({ } } - const performGeneralSearch = () => { + const performGeneralSearch = async () => { + await onNavigateAway() executeGeneralSearch(router, currentVersion, urlSearchInputQuery, debug) - onClose() } const referenceOnSelect = (url: string) => { @@ -374,128 +351,73 @@ export function SearchOverlay({ eventGroupKey: ASK_AI_EVENT_GROUP, eventGroupId: askAIEventGroupId.current, }) - setSelectedIndex(-1) - const searchParams = new URLSearchParams((router.query as Record) || {}) - if (searchParams.has('search-overlay-open')) { - searchParams.delete('search-overlay-open') - } - if (searchParams.has('search-overlay-input')) { - searchParams.delete('search-overlay-input') + setActiveId(null) + window.open(withInheritedParams(url), '_blank') + } + + // Shared by keyboard and row clicks, so both paths report the same analytics rank. + const activate = (option: SearchOption) => { + if (option.kind === 'general') { + generalSearchResultOnSelect(option.payload, option.groupRank) + } else if (option.kind === 'ai') { + aiSearchOptionOnSelect(option.payload) + } else if (option.kind === 'reference') { + referenceOnSelect(option.payload.url) + } else { + // 'search-docs' and 'view-all' both run the full docs search. + performGeneralSearch() } - if (searchParams.has('search-overlay-ask-ai')) { - searchParams.delete('search-overlay-ask-ai') + } + + // Keep these `pressed_on` values stable so they stay comparable with historical analytics. + const keyboardDispatch: Record = + { + ai: { pressedOn: 'ai-option', askAIGroup: false }, + general: { pressedOn: 'general-option', askAIGroup: false }, + reference: { pressedOn: 'reference-option', askAIGroup: true }, + 'search-docs': { pressedOn: 'view-all', askAIGroup: false }, + 'view-all': { pressedOn: 'view-all', askAIGroup: false }, } - if (searchParams.has('query')) { - searchParams.delete('query') + + // Stepping off either end clears the selection instead of wrapping. + const stepActiveOption = (delta: 1 | -1) => { + if (options.length === 0) { + return } - window.open(`${url}?${searchParams.toString()}`, '_blank') + const next = activeIndex === -1 ? (delta === 1 ? 0 : options.length - 1) : activeIndex + delta + setActiveId(next < 0 || next >= options.length ? null : options[next].id) } const handleKeyDown = (event: React.KeyboardEvent) => { - const optionsLength = listElementsRef.current?.length ?? 0 if (event.key === 'ArrowDown') { event.preventDefault() - if (optionsLength > 0) { - let newIndex = 0 - if (selectedIndex === -1) { - newIndex = 0 - } else { - newIndex = (selectedIndex + 1) % optionsLength - // Wraparound after the last option clears the selection. - if (newIndex < selectedIndex) { - newIndex = -1 - } - } - // If it's the "no results found" option, skip it - if ( - newIndex >= selectedIndex && - (combinedOptions[newIndex]?.option as GeneralSearchHitWithOptions)?.isNoResultsFound - ) { - newIndex += 1 - } - setSelectedIndex(newIndex) - if (newIndex !== -1 && listElementsRef.current[newIndex]) { - listElementsRef.current[newIndex]?.scrollIntoView({ - behavior: 'smooth', - block: 'center', - }) - } - } + stepActiveOption(1) } else if (event.key === 'ArrowUp') { event.preventDefault() - if (optionsLength > 0) { - let newIndex = 0 - if (selectedIndex === -1) { - newIndex = optionsLength - 1 - } else { - newIndex = (selectedIndex - 1 + optionsLength) % optionsLength - // Wraparound before the first option clears the selection. - if (newIndex > selectedIndex) { - newIndex = -1 - } - } - // If it's the "no results found" option, skip it - if ( - newIndex <= selectedIndex && - (combinedOptions[newIndex]?.option as GeneralSearchHitWithOptions)?.isNoResultsFound - ) { - newIndex -= 1 - } - setSelectedIndex(newIndex) - if (newIndex !== -1 && listElementsRef.current[newIndex]) { - listElementsRef.current[newIndex]?.scrollIntoView({ - behavior: 'smooth', - block: 'center', - }) - } - } + stepActiveOption(-1) } else if (event.key === 'Enter') { event.preventDefault() - let pressedGroupKey = SEARCH_OVERLAY_EVENT_GROUP - let pressedGroupId = searchEventGroupId - let pressedOnContext = '' - - // Enter with no selected option asks AI with the typed query. - if (selectedIndex === -1) { - pressedOnContext = AI_SEARCH_CONTEXT - pressedGroupKey = ASK_AI_EVENT_GROUP - pressedGroupId = askAIEventGroupId - sendKeyboardEvent(event.key, pressedOnContext, pressedGroupId, pressedGroupKey) - aiSearchOptionOnSelect({ term: urlSearchInputQuery } as AutocompleteSearchHit) - } else if ( - combinedOptions.length > 0 && - selectedIndex >= 0 && - selectedIndex < combinedOptions.length - ) { - const selectedItem = combinedOptions[selectedIndex] - if (!selectedItem) { + + // With nothing selected, Enter asks Copilot. Full results are the last arrow stop. + if (activeIndex === -1) { + if (!urlSearchInputQuery.trim()) { return } - // Send the event before running the action. - let action = () => {} - if (selectedItem?.group === 'general') { - if ( - (selectedItem.option as GeneralSearchHitWithOptions).isViewAllResults || - (selectedItem.option as GeneralSearchHitWithOptions).isSearchDocsOption - ) { - pressedOnContext = 'view-all' - action = performGeneralSearch - } else { - pressedOnContext = 'general-option' - action = () => generalSearchResultOnSelect(selectedItem.option as GeneralSearchHit) - } - } else if (selectedItem?.group === 'ai') { - pressedOnContext = 'ai-option' - action = () => aiSearchOptionOnSelect(selectedItem.option as AutocompleteSearchHit) - } else if (selectedItem?.group === 'reference') { - pressedGroupKey = ASK_AI_EVENT_GROUP - pressedGroupId = askAIEventGroupId - pressedOnContext = 'reference-option' - action = () => referenceOnSelect(selectedItem.url || '') - } - sendKeyboardEvent(event.key, pressedOnContext, pressedGroupId, pressedGroupKey) - return action() + sendKeyboardEvent(event.key, AI_SEARCH_CONTEXT, askAIEventGroupId, ASK_AI_EVENT_GROUP) + aiSearchOptionOnSelect({ term: urlSearchInputQuery } as AutocompleteSearchHit) + return } + + const option = options[activeIndex] + const { pressedOn, askAIGroup } = keyboardDispatch[option.kind] + // Send before navigating: router.push unmounts this component. + sendKeyboardEvent( + event.key, + pressedOn, + askAIGroup ? askAIEventGroupId : searchEventGroupId, + askAIGroup ? ASK_AI_EVENT_GROUP : SEARCH_OVERLAY_EVENT_GROUP, + ) + activate(option) } else if (event.key === 'Escape') { event.preventDefault() onClose() @@ -504,7 +426,7 @@ export function SearchOverlay({ const onBackButton = () => { // Leave the Ask AI state when the user clicks the back button - setSelectedIndex(-1) + setActiveId(null) updateParams({ 'search-overlay-ask-ai': '', 'search-overlay-input': urlSearchInputQuery, @@ -527,8 +449,6 @@ export function SearchOverlay({ }, references: aiReferences, setReferences: setAIReferences, - referencesIndexOffset: generalOptionsWithViewStatus.length, - referenceOnSelect, askAIEventGroupId, aiSearchError, aiCouldNotAnswer, @@ -537,13 +457,10 @@ export function SearchOverlay({ const searchContextValue: SearchContextType = { t, - generalSearchOptions: generalOptionsWithViewStatus, - aiOptionsWithUserInput, - generalSearchResultOnSelect, - aiAutocompleteOnSelect: aiSearchOptionOnSelect, - performGeneralSearch, - selectedIndex, - listElementsRef: listElementsRef as RefObject>, + model, + activeId, + activate, + registerOption, askAIState, showSpinner, searchLoading, @@ -556,179 +473,197 @@ export function SearchOverlay({ const inErrorState = aiSearchError || (autoCompleteSearchError && !isAskAIState) if (inErrorState) { OverlayContents = ( - - +
    {/* Show the AI Search UI error message whenever AI search fails. */} {aiSearchError && ( - <> - -
  • - - - {t('search.overlay.ai_autocomplete_list_heading')} - -
    • -
  • -
    - -
    -
    • - {/* Show the bottom divider when general results follow the AI error. */} - {generalOptionsWithViewStatus.length > 0 && ( - - )} - +
    + +
    + +
    +
    )} - +
    ) } else { OverlayContents = ( - - +
    ) } return ( - <> -
    - -
    -
    - -
    - } - role="combobox" - // In Ask AI the input controls the results region instead of the suggestions list. - aria-controls={isAskAIState ? 'ask-ai-result-container' : 'search-suggestions-list'} - aria-expanded={combinedOptions.length > 0} - aria-label={t('search.overlay.input_aria_label')} - aria-activedescendant={ - selectedIndex >= 0 - ? `search-option-${combinedOptions[selectedIndex]?.group}-${selectedIndex}` - : undefined - } - onKeyDown={handleKeyDown} - placeholder={t('search.input.placeholder_no_icon')} - trailingAction={ - - { - setSelectedIndex(-1) - if (isAskAIState) { - updateParams({ - 'search-overlay-ask-ai': '', - 'search-overlay-input': '', - }) - } else { - updateParams({ - 'search-overlay-input': '', - }) - } - if (!isAskAIState) { - updateAutocompleteResults('') - } - }} - icon={XCircleFillIcon} - aria-label={t('search.overlay.clear_search_query')} - /> - - } - /> + +
    +
    +
    -
    + + {/* + Layout only: the scroller above the pinned view-all. The listbox itself is inside the + scroller (SearchGroups), and takes the pinned view-all in through aria-owns. + */} +
    {OverlayContents} - -
    - -
    -
    - {announcement} -
    - - + {/* No divider above: Figma uses space here, set by the `.viewAllResults` margin. */} + {model.viewAll && ( + inside an option fails axe nested-interactive. + role="option" + aria-selected={activeId === VIEW_ALL_OPTION_ID} + // Skips registerOption: brand's Link drops refs, and this sits outside the scroller. + href={model.viewAll.payload.href} + size="medium" + variant="accent" + // Figma draws a plain text link, so drop brand's trailing arrow. + arrowDirection="none" + // Tests use this instead of the label, which includes a count. + data-testid="view-all-results" + // Clear the virtual selection on real focus, or two things look focused. + onFocus={() => setActiveId(null)} + onClick={(event: React.MouseEvent) => { + // Leave new-tab and new-window gestures to the browser and the href. + if ( + event.metaKey || + event.ctrlKey || + event.shiftKey || + event.altKey || + event.button !== 0 + ) { + return + } + event.preventDefault() + performGeneralSearch() + }} + > + {model.viewAll.payload.label} + + )} +
    +
    + +
    +
    + {announcement} +
    + {/* The spinner is aria-hidden to keep the listbox valid, so loading is announced here. */} +
    + {suggestionsHidden ? t('search.overlay.loading_results_announcement') : ''} +
    +     ) } diff --git a/src/search/components/input/SearchOverlayContainer.tsx b/src/search/components/input/SearchOverlayContainer.tsx index 84141391658c..b2500ba99808 100644 --- a/src/search/components/input/SearchOverlayContainer.tsx +++ b/src/search/components/input/SearchOverlayContainer.tsx @@ -4,9 +4,11 @@ import { QueryParams } from '../hooks/useMultiQueryParams' type Props = { isSearchOpen: boolean - setIsSearchOpen: (value: boolean) => void + setIsSearchOpen: (value: boolean) => Promise params: QueryParams updateParams: (updates: Partial) => void + // Drops a debounced search-input URL write that has not fired yet. + cancelPendingParamsUpdate: () => void searchButtonRef: React.RefObject } @@ -15,6 +17,7 @@ export function SearchOverlayContainer({ setIsSearchOpen, params, updateParams, + cancelPendingParamsUpdate, searchButtonRef, }: Props) { const debug = params.debug === 'true' @@ -28,8 +31,17 @@ export function SearchOverlayContainer({ params={params} updateParams={updateParams} onClose={() => { + // A pending input write would restore `search-overlay-open` after this close. + cancelPendingParamsUpdate() setIsSearchOpen(false) }} + // Writes the closed state to the URL and resolves once it lands, so the caller can + // navigate afterwards. Writing it at all keeps Back from reopening the dialog; waiting + // for it keeps the write from overlapping the navigation, which Next would cancel. + onNavigateAway={async () => { + cancelPendingParamsUpdate() + await setIsSearchOpen(false) + }} /> ) } diff --git a/src/search/components/input/search-options.ts b/src/search/components/input/search-options.ts new file mode 100644 index 000000000000..b4015b554283 --- /dev/null +++ b/src/search/components/input/search-options.ts @@ -0,0 +1,166 @@ +import type { AIReference } from '../types' +import type { AutocompleteSearchHit, GeneralSearchHit } from '@/search/types' +import { MAX_REFERENCES_TO_SHOW } from '@/search/lib/ai-search-constants' + +import type { + AutocompleteSearchHitWithUserQuery, + GeneralSearchHitWithOptions, +} from './SearchContext' + +export type SearchOptionKind = 'ai' | 'reference' | 'general' | 'search-docs' | 'view-all' + +type OptionBase = { + // The DOM id and the only value aria-activedescendant takes. Set only here, so no renderer + // computes it from an offset. + id: string + // 0-based rank within this option's kind. + groupRank: number +} + +export type SearchOption = + | (OptionBase & { kind: 'ai'; payload: AutocompleteSearchHitWithUserQuery }) + | (OptionBase & { kind: 'reference'; payload: AIReference }) + | (OptionBase & { kind: 'general'; payload: GeneralSearchHitWithOptions }) + | (OptionBase & { kind: 'search-docs'; payload: GeneralSearchHitWithOptions }) + | (OptionBase & { kind: 'view-all'; payload: { href: string; label: string } }) + +export type AiOption = Extract +export type ReferenceOption = Extract +export type GeneralOption = Extract +export type ViewAllOption = Extract + +export type SearchOptionModel = { + // Every arrow-navigable row, in render order. Each renders exactly one role="option". + options: SearchOption[] + // Slices of `options` that share object identities, not copies. + ai: AiOption[] + references: ReferenceOption[] + general: GeneralOption[] + viewAll: ViewAllOption | null + // Render "No results found" as plain text, not an option, and announce it. + noResults: boolean +} + +export const VIEW_ALL_OPTION_ID = 'search-option-view-all' +export const SEARCH_DOCS_OPTION_ID = 'search-option-search-docs' + +type BuildInput = { + query: string + isAskAIState: boolean + searchLoading: boolean + // The spinner replaces the suggestion rows, so they leave the arrow order. + // The pinned view-all renders outside the spinner and stays. + suggestionsHidden: boolean + aiSearchError: boolean + aiCouldNotAnswer: boolean + autoCompleteSearchError: boolean + aiAutocompleteOptions: AutocompleteSearchHit[] + aiReferences: AIReference[] + generalSearchResults: GeneralSearchHit[] + showViewAllResults: boolean + viewAllHref: string + viewAllLabel: string +} + +// Defines everything the arrow keys can reach. Stays pure and React-free, so tests can cover +// every state and the calling memo has a complete dependency list. +// Never count options from the DOM, or aria-activedescendant can name rows that don't exist. +export function buildSearchOptions({ + query, + isAskAIState, + searchLoading, + suggestionsHidden, + aiSearchError, + aiCouldNotAnswer, + autoCompleteSearchError, + aiAutocompleteOptions, + aiReferences, + generalSearchResults, + showViewAllResults, + viewAllHref, + viewAllLabel, +}: BuildInput): SearchOptionModel { + const hasQuery = query.trim() !== '' + + const ai: AiOption[] = [] + const references: ReferenceOption[] = [] + const general: GeneralOption[] = [] + let viewAll: ViewAllOption | null = null + let noResults = false + + if (isAskAIState) { + // On an AI error, neither the answer nor its source rows render. + if (!aiCouldNotAnswer && !aiSearchError) { + // Truncate here, not in the renderer, so aria-activedescendant never names a missing row. + const shown = aiReferences.slice(0, MAX_REFERENCES_TO_SHOW) + for (const [index, reference] of shown.entries()) { + references.push({ + kind: 'reference', + id: `search-option-reference-${index}`, + groupRank: index, + payload: reference, + }) + } + } + } else if (!aiSearchError) { + // The user's own query leads the AI rows; drop the suggestion that duplicates it. + const aiPayloads: AutocompleteSearchHitWithUserQuery[] = [ + ...(hasQuery + ? [ + { + term: query, + title: query, + highlights: [], + isUserQuery: true, + } as unknown as AutocompleteSearchHitWithUserQuery, + ] + : []), + ...aiAutocompleteOptions.filter((option) => option.term !== query), + ] + for (const [index, payload] of aiPayloads.entries()) { + ai.push({ + kind: 'ai', + id: `search-option-ai-${index}`, + groupRank: index, + payload, + }) + } + } + + if (generalSearchResults.length > 0) { + for (const [index, payload] of generalSearchResults.entries()) { + general.push({ + kind: 'general', + id: `search-option-general-${index}`, + groupRank: index, + payload, + }) + } + if (showViewAllResults) { + viewAll = { + kind: 'view-all', + id: VIEW_ALL_OPTION_ID, + groupRank: 0, + payload: { href: viewAllHref, label: viewAllLabel }, + } + } + } else if (autoCompleteSearchError) { + // Results failed, so offer a full docs search for the query. Never coexists with view-all. + if (hasQuery) { + general.push({ + kind: 'search-docs', + id: SEARCH_DOCS_OPTION_ID, + groupRank: 0, + payload: { title: query } as GeneralSearchHitWithOptions, + }) + } + } else if (hasQuery && !searchLoading) { + noResults = true + } + + // Must match render order: AI rows or references, then docs rows, then view-all last. + const rows = suggestionsHidden ? [] : [...ai, ...references, ...general] + const options = [...rows, ...(viewAll ? [viewAll] : [])] + + return { options, ai, references, general, viewAll, noResults } +} diff --git a/src/search/components/input/variables.scss b/src/search/components/input/variables.scss index 06252a0a60d2..4883f0cce7c2 100644 --- a/src/search/components/input/variables.scss +++ b/src/search/components/input/variables.scss @@ -3,7 +3,9 @@ $mdHeaderSearchInputWidth: 100%; // Technically we don't show the search bar at $lgHeaderSearchInputWidth: 25rem; $xlHeaderSearchInputWidth: 40rem; -$smSearchOverlayWidth: 100vw; -$mdSearchOverlayWidth: 100vw; -$lgSearchOverlayWidth: 40rem; -$xlSearchOverlayWidth: 50rem; +// Figma nodes 446-109611 and 446-105514: a card up to 880px wide, centered in a 72px gutter. +// Full-bleed below `md`. +$searchDialogMaxWidth: 55rem; // 880px +$searchDialogGutter: 72px; +// Repeated at higher specificity in .scrim to survive the nested theme wrapper reset. +$searchScrimColor: rgba(0, 0, 0, 0.2); diff --git a/src/search/lib/ai-search-constants.ts b/src/search/lib/ai-search-constants.ts index 2f4f93c5f55e..85314a78c013 100644 --- a/src/search/lib/ai-search-constants.ts +++ b/src/search/lib/ai-search-constants.ts @@ -5,3 +5,6 @@ export const MAX_QUERY_LENGTH = 500 // Azure Responsible AI input filtering returns this detail.code for expected // user-triggered rejections, so the error-rate monitor excludes them. export const RAI_CONTENT_FILTER_CODE = 'RAI_INPUT_CONTENT_POLICY_BREACH_ERROR' + +// buildSearchOptions truncates to this, so aria-activedescendant never names a missing row. +export const MAX_REFERENCES_TO_SHOW = 5 diff --git a/src/search/tests/search-options.ts b/src/search/tests/search-options.ts new file mode 100644 index 000000000000..48a33c176f21 --- /dev/null +++ b/src/search/tests/search-options.ts @@ -0,0 +1,211 @@ +import { describe, expect, test } from 'vitest' + +import { + buildSearchOptions, + VIEW_ALL_OPTION_ID, + SEARCH_DOCS_OPTION_ID, +} from '@/search/components/input/search-options' +import { MAX_REFERENCES_TO_SHOW } from '@/search/lib/ai-search-constants' +import type { AIReference } from '@/search/components/types' +import type { AutocompleteSearchHit, GeneralSearchHit } from '@/search/types' + +function generalHits(n: number): GeneralSearchHit[] { + return Array.from({ length: n }, (_, i) => ({ + title: `Doc ${i}`, + url: `/doc-${i}`, + })) as unknown as GeneralSearchHit[] +} + +function aiHits(...terms: string[]): AutocompleteSearchHit[] { + return terms.map((term) => ({ term, highlights: [] })) as unknown as AutocompleteSearchHit[] +} + +function references(n: number): AIReference[] { + return Array.from({ length: n }, (_, i) => ({ + title: `Source ${i}`, + url: `/source-${i}`, + })) as unknown as AIReference[] +} + +const base = { + query: 'repository', + isAskAIState: false, + searchLoading: false, + suggestionsHidden: false, + aiSearchError: false, + aiCouldNotAnswer: false, + autoCompleteSearchError: false, + aiAutocompleteOptions: [] as AutocompleteSearchHit[], + aiReferences: [] as AIReference[], + generalSearchResults: [] as GeneralSearchHit[], + showViewAllResults: false, + viewAllHref: '/en/search?query=repository', + viewAllLabel: 'View all 12 results', +} + +describe('buildSearchOptions', () => { + test('ids are unique across every state permutation', () => { + // The whole model rests on id uniqueness: a duplicate makes aria-activedescendant and + // the element map ambiguous, silently. + for (const isAskAIState of [true, false]) { + for (const aiSearchError of [true, false]) { + for (const autoCompleteSearchError of [true, false]) { + for (const aiCouldNotAnswer of [true, false]) { + for (const generalCount of [0, 3]) { + for (const showViewAllResults of [true, false]) { + const { options } = buildSearchOptions({ + ...base, + isAskAIState, + aiSearchError, + autoCompleteSearchError, + aiCouldNotAnswer, + aiAutocompleteOptions: aiHits('a', 'b'), + aiReferences: references(3), + generalSearchResults: generalHits(generalCount), + showViewAllResults: showViewAllResults && generalCount > 0, + }) + const ids = options.map((option) => option.id) + expect(new Set(ids).size, `duplicate id in ${JSON.stringify(ids)}`).toBe(ids.length) + } + } + } + } + } + } + }) + + test('references are truncated to the render cap', () => { + // Regression: the old model pushed every reference into the navigable list while only + // MAX_REFERENCES_TO_SHOW rendered, so arrowing past the cap pointed + // aria-activedescendant at an element that did not exist. + const model = buildSearchOptions({ + ...base, + isAskAIState: true, + aiReferences: references(MAX_REFERENCES_TO_SHOW + 2), + }) + expect(model.references).toHaveLength(MAX_REFERENCES_TO_SHOW) + expect(model.options.filter((option) => option.kind === 'reference')).toHaveLength( + MAX_REFERENCES_TO_SHOW, + ) + }) + + test('view-all is the last option when present', () => { + const model = buildSearchOptions({ + ...base, + aiAutocompleteOptions: aiHits('a'), + generalSearchResults: generalHits(3), + showViewAllResults: true, + }) + expect(model.options.at(-1)?.id).toBe(VIEW_ALL_OPTION_ID) + expect(model.viewAll?.payload.href).toBe(base.viewAllHref) + }) + + test('no view-all option when it is not shown', () => { + const model = buildSearchOptions({ ...base, generalSearchResults: generalHits(3) }) + expect(model.viewAll).toBeNull() + expect(model.options.some((option) => option.id === VIEW_ALL_OPTION_ID)).toBe(false) + }) + + test('search-docs and view-all can never coexist', () => { + // They are mutually exclusive by construction: search-docs only appears when there are + // no general results, and view-all only when there are some. + const model = buildSearchOptions({ + ...base, + autoCompleteSearchError: true, + generalSearchResults: [], + showViewAllResults: true, + }) + const kinds = model.options.map((option) => option.kind) + expect(kinds).toContain('search-docs') + expect(kinds).not.toContain('view-all') + }) + + test('no-results is reported as a flag, not as an option', () => { + // Keeping it out of the model is what lets the arrow keys stay a plain step with no + // skip rule — the old implementation carried a skip arm in both directions for it. + const model = buildSearchOptions({ ...base, aiSearchError: true }) + expect(model.noResults).toBe(true) + expect(model.general).toHaveLength(0) + expect(model.options).toHaveLength(0) + }) + + test('groupRank is contiguous from zero within each kind', () => { + const model = buildSearchOptions({ + ...base, + aiAutocompleteOptions: aiHits('a', 'b'), + generalSearchResults: generalHits(4), + showViewAllResults: true, + }) + // The user's own query leads the AI rows, so there is one more than was supplied. + expect(model.ai.map((option) => option.groupRank)).toEqual([0, 1, 2]) + expect(model.general.map((option) => option.groupRank)).toEqual([0, 1, 2, 3]) + }) + + test('general ids do not shift when the AI group above them changes size', () => { + // Under the old offset scheme a docs row's id moved whenever the AI group grew, which + // is what made the ids unstable to assert against. + const withoutAI = buildSearchOptions({ ...base, generalSearchResults: generalHits(2) }) + const withAI = buildSearchOptions({ + ...base, + aiAutocompleteOptions: aiHits('a', 'b', 'c'), + generalSearchResults: generalHits(2), + }) + expect(withoutAI.general.map((o) => o.id)).toEqual(withAI.general.map((o) => o.id)) + }) + + test('the query row is dropped from AI suggestions when it duplicates the query', () => { + const model = buildSearchOptions({ + ...base, + query: 'repository', + aiAutocompleteOptions: aiHits('repository', 'other'), + }) + const terms = model.ai.map((option) => option.payload.term) + expect(terms).toEqual(['repository', 'other']) + }) + + test('an empty query produces no AI rows', () => { + const model = buildSearchOptions({ ...base, query: ' ', aiAutocompleteOptions: [] }) + expect(model.ai).toHaveLength(0) + expect(model.noResults).toBe(false) + }) + + test('ask-AI state with couldNotAnswer exposes no reference options', () => { + const model = buildSearchOptions({ + ...base, + isAskAIState: true, + aiCouldNotAnswer: true, + aiReferences: references(3), + }) + expect(model.references).toHaveLength(0) + }) + + test('the search-docs row carries the stable singleton id', () => { + const model = buildSearchOptions({ ...base, autoCompleteSearchError: true }) + expect(model.general[0].id).toBe(SEARCH_DOCS_OPTION_ID) + }) + + test('ask-AI state with an AI error exposes no reference options', () => { + // The answer, and with it the source rows, does not render on an AI error. + const model = buildSearchOptions({ + ...base, + isAskAIState: true, + aiSearchError: true, + aiReferences: references(3), + }) + expect(model.references).toHaveLength(0) + expect(model.options.some((option) => option.kind === 'reference')).toBe(false) + }) + + test('rows hidden behind the spinner leave the arrow-key order, view-all stays', () => { + const model = buildSearchOptions({ + ...base, + suggestionsHidden: true, + aiAutocompleteOptions: aiHits('how do I create a repository'), + generalSearchResults: generalHits(3), + showViewAllResults: true, + }) + expect(model.options.map((option) => option.id)).toEqual([VIEW_ALL_OPTION_ID]) + // The renderer's slices are untouched, so the rows come back unchanged afterwards. + expect(model.general).toHaveLength(3) + }) +})