diff --git a/apps/web/src/components/ChatView.logic.browser.test.ts b/apps/web/src/components/ChatView.logic.browser.test.ts index e1c1f04a0c3a5..7e660fcfd5376 100644 --- a/apps/web/src/components/ChatView.logic.browser.test.ts +++ b/apps/web/src/components/ChatView.logic.browser.test.ts @@ -1,5 +1,6 @@ import { expect, it } from "vite-plus/test"; import { userEvent } from "vite-plus/test/browser"; +import { createWorkspaceFocusController } from "../lib/workspaceFocus"; import { shouldRefocusComposerOnWindowFocus } from "./ChatView.logic"; it("keeps keyboard focus on a sidebar row on window return, but not a clicked row", async () => { @@ -26,3 +27,35 @@ it("keeps keyboard focus on a sidebar row on window return, but not a clicked ro expect(document.activeElement).toBe(tabbed); expect(shouldRefocusComposerOnWindowFocus(document.activeElement)).toBe(false); }); + +it("refocuses the composer on window return from a clicked area after a bare key press", async () => { + const controller = createWorkspaceFocusController(document); + const root = document.createElement("div"); + root.innerHTML = + '' + + '
'; + document.body.append(root); + using _root = { + [Symbol.dispose]: () => { + controller.dispose(); + root.remove(); + }, + }; + const conversation = root.querySelector("section")!; + + await userEvent.click(conversation); + expect(document.activeElement).toBe(conversation); + // A window switch can deliver a lone modifier key without moving focus. + await userEvent.keyboard("{Shift}"); + expect( + shouldRefocusComposerOnWindowFocus(document.activeElement), + "a clicked area counts as reached from the keyboard after a bare key press", + ).toBe(true); + + await userEvent.keyboard("{F6}"); + controller.move(-1); + await userEvent.keyboard("{F6}"); + controller.move(1); + expect(document.activeElement).toBe(conversation); + expect(shouldRefocusComposerOnWindowFocus(document.activeElement)).toBe(false); +}); diff --git a/apps/web/src/components/ChatView.logic.ts b/apps/web/src/components/ChatView.logic.ts index 49fe3a441fbb8..dfd10f687a41c 100644 --- a/apps/web/src/components/ChatView.logic.ts +++ b/apps/web/src/components/ChatView.logic.ts @@ -1165,7 +1165,7 @@ export function shouldRefocusComposerOnWindowFocus( } return ( activeElement.closest( - '[data-workspace-area]:focus-visible, [role="treeitem"]:focus-visible, [role="dialog"], [role="alertdialog"], [role="application"], [role="menu"], [role="listbox"], [data-slot$="-popup"], [data-terminal-owner]', + '[data-workspace-keyboard-focus], [role="treeitem"]:focus-visible, [role="dialog"], [role="alertdialog"], [role="application"], [role="menu"], [role="listbox"], [data-slot$="-popup"], [data-terminal-owner]', ) === null ); } diff --git a/apps/web/src/index.css b/apps/web/src/index.css index e186516791252..0c162018d57ac 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -2563,8 +2563,12 @@ code { } } -/* A workspace area with no control of its own takes F6 focus itself; keep that visible. */ +/* A workspace area with no control of its own takes F6 focus itself; keep that visible. + workspaceFocus.ts marks those, since :focus-visible also matches a clicked area after a key press. */ [data-workspace-area]:focus-visible { + outline: none; +} +[data-workspace-keyboard-focus]:focus { outline: 2px solid var(--ring); outline-offset: -2px; } diff --git a/apps/web/src/lib/workspaceFocus.browser.test.ts b/apps/web/src/lib/workspaceFocus.browser.test.ts index f0064630f08c2..eec7c289a0039 100644 --- a/apps/web/src/lib/workspaceFocus.browser.test.ts +++ b/apps/web/src/lib/workspaceFocus.browser.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; +import appCss from "~/index.css?inline"; import { createWorkspaceFocusController } from "./workspaceFocus"; function setup() { @@ -257,4 +259,53 @@ describe("workspace area focus", () => { expect(fixture.controller.move(1)).toBe(true); expect(document.activeElement).toBe(terminal.button); }); + + it("rings an area reached with F6, but not a clicked one after a bare key press", async () => { + using fixture = setup(); + const style = document.createElement("style"); + style.textContent = appCss; + document.head.append(style); + using _style = { [Symbol.dispose]: () => style.remove() }; + const sidebar = fixture.area("sidebar", 0); + const home = fixture.append( + '
', + ); + const outline = () => getComputedStyle(home).outlineStyle; + + await userEvent.click(home); + expect(document.activeElement).toBe(home); + expect(outline()).toBe("none"); + // A window switch can deliver a lone modifier key without moving focus. + await userEvent.keyboard("{Shift}"); + expect(outline(), "a clicked area shows the focus ring after a bare key press").toBe("none"); + // Window activation refocuses the element that had focus, with nothing focused in between. + home.blur(); + home.focus(); + expect(outline(), "a clicked area rings when the window regains focus").toBe("none"); + + await userEvent.keyboard("{F6}"); + fixture.controller.move(-1); + expect(document.activeElement).toBe(sidebar.button); + await userEvent.keyboard("{F6}"); + fixture.controller.move(1); + expect(document.activeElement).toBe(home); + expect(outline()).toBe("solid"); + + // A dialog opened from the keyboard returns focus to the area when it closes. + await userEvent.keyboard("{Enter}"); + sidebar.button.focus(); + await userEvent.keyboard("{Escape}"); + home.focus(); + expect(outline(), "an area loses its ring when keyboard focus returns to it").toBe("solid"); + + await userEvent.click(home); + expect(outline(), "clicking the ringed area removes its ring").toBe("none"); + + // With no other area visible, F6 lands on the area that already has focus. + sidebar.element.hidden = true; + await userEvent.keyboard("{F6}"); + fixture.controller.move(1); + expect(document.activeElement).toBe(home); + expect(outline(), "F6 onto the only, already focused area shows no ring").toBe("solid"); + }); }); diff --git a/apps/web/src/lib/workspaceFocus.ts b/apps/web/src/lib/workspaceFocus.ts index e612a8ff54974..ceea4322f5406 100644 --- a/apps/web/src/lib/workspaceFocus.ts +++ b/apps/web/src/lib/workspaceFocus.ts @@ -90,10 +90,28 @@ function revealInScrollers(target: HTMLElement) { export function createWorkspaceFocusController(doc: Document) { const lastFocused = new Map(); let lastArea: string | undefined; + // An area that took focus itself from the keyboard. Chromium's :focus-visible also matches + // a clicked area after any later key press, so the ring and window return read this mark. + let keyboardArea: HTMLElement | undefined; + let previousTarget: EventTarget | null = null; + const unmarkKeyboardArea = () => { + keyboardArea?.removeAttribute("data-workspace-keyboard-focus"); + keyboardArea = undefined; + }; + const markKeyboardArea = (area: HTMLElement) => { + if (!area.matches(":focus-visible")) return; + keyboardArea = area; + area.setAttribute("data-workspace-keyboard-focus", ""); + }; const remember = (event: FocusEvent) => { + if (event.target !== keyboardArea) unmarkKeyboardArea(); + // Window activation refocuses the same element; only focus arriving from elsewhere counts. + const arrived = event.target !== previousTarget; + previousTarget = event.target; if (!(event.target instanceof HTMLElement)) return; const area = workspaceArea(event.target); if (!area?.dataset.workspaceArea) return; + if (arrived && event.target === area) markKeyboardArea(area); lastArea = area.dataset.workspaceArea; const key = area.dataset.workspaceFocusKey ?? lastArea; lastFocused.delete(key); @@ -162,6 +180,8 @@ export function createWorkspaceFocusController(doc: Document) { const active = root instanceof ShadowRoot ? root.activeElement : doc.activeElement; if (active === target || (active && target.contains(active))) { if (target !== area && !isConversation) revealInScrollers(target); + // Focusing the already focused area fires no focusin, as when F6 has nowhere else to go. + if (target === area) markKeyboardArea(area); return true; } } @@ -301,7 +321,11 @@ export function createWorkspaceFocusController(doc: Document) { const onUserInput = () => { if (cancelOnInput) cancelRequest(); }; - doc.addEventListener("pointerdown", onUserInput, true); + const onPointerDown = () => { + unmarkKeyboardArea(); + onUserInput(); + }; + doc.addEventListener("pointerdown", onPointerDown, true); doc.addEventListener("keydown", onUserInput, true); return { @@ -313,9 +337,10 @@ export function createWorkspaceFocusController(doc: Document) { request, dispose: () => { cancelRequest(); + unmarkKeyboardArea(); doc.removeEventListener("focusin", remember); doc.removeEventListener("focusout", recover); - doc.removeEventListener("pointerdown", onUserInput, true); + doc.removeEventListener("pointerdown", onPointerDown, true); doc.removeEventListener("keydown", onUserInput, true); }, };