From aad85a6bffa76d7fef3db8ba6c25d5fbdb7c5376 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:57:07 -0700 Subject: [PATCH] fix(kanban-search): let the board search field own Escape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The board's Escape handler is a capture-phase listener on document, so it runs before React's handlers and the search field's stopPropagation could never reach it — pressing Escape to clear a query dismissed the whole board instead, and the reopen reset then discarded the query too. useWorkspaceBoardPanel now defers Escape to editable targets inside the board sheet, and the field handles both outcomes itself: clear when it has text, close the board when it does not. Also: keep focus in the field when the clear button unmounts itself, reserve counter width from the rendered text so three-digit counts cannot overlap typed text, and align the icon centering, X size, and placeholder with the sibling search fields. Co-authored-by: Orca --- .../sidebar/WorkspaceKanbanDrawerHeader.tsx | 1 + .../WorkspaceKanbanSearchField.test.tsx | 40 ++++++++--- .../sidebar/WorkspaceKanbanSearchField.tsx | 68 +++++++++++++------ .../sidebar/useWorkspaceBoardPanel.test.tsx | 39 ++++++++++- .../sidebar/useWorkspaceBoardPanel.ts | 18 +++++ src/renderer/src/i18n/locales/en.json | 1 - src/renderer/src/i18n/locales/es.json | 1 - src/renderer/src/i18n/locales/ja.json | 1 - src/renderer/src/i18n/locales/ko.json | 1 - src/renderer/src/i18n/locales/zh.json | 1 - 10 files changed, 135 insertions(+), 36 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawerHeader.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawerHeader.tsx index 732adb8750a..e55ed413fc9 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawerHeader.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawerHeader.tsx @@ -79,6 +79,7 @@ export default function WorkspaceKanbanDrawerHeader({ totalCount={totalCount} onQueryChange={onQueryChange} onClear={onClearQuery} + onClose={onClose} /> diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx index a842bb20675..67ab3ccdf94 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.test.tsx @@ -11,6 +11,7 @@ let container: HTMLDivElement let root: Root const onQueryChange = vi.fn() const onClear = vi.fn() +const onClose = vi.fn() function renderField(props: { query: string @@ -27,6 +28,7 @@ function renderField(props: { totalCount={props.totalCount ?? 0} onQueryChange={onQueryChange} onClear={onClear} + onClose={onClose} /> ) }) @@ -134,12 +136,12 @@ describe('WorkspaceKanbanSearchField', () => { expect(liveRegion().textContent).toBe('') }) - it('clears a non-empty query on Escape and leaves an empty one to the drawer', () => { + it('clears a non-empty query on Escape and closes the board on an empty one', () => { + // Why: useWorkspaceBoardPanel's Escape listener is capture-phase on + // document, so it runs before this handler and stopPropagation cannot + // reach it. The panel defers to board text fields instead, which makes + // this field solely responsible for both Escape outcomes. renderField({ query: 'orca', matchCount: 3, totalCount: 12 }) - // Why: React roots at `container`, so only a listener above it can observe - // whether the field let Escape through. - const bubbled = vi.fn() - document.body.addEventListener('keydown', bubbled) act(() => { input().dispatchEvent( @@ -147,7 +149,7 @@ describe('WorkspaceKanbanSearchField', () => { ) }) expect(onClear).toHaveBeenCalledOnce() - expect(bubbled).not.toHaveBeenCalled() + expect(onClose).not.toHaveBeenCalled() renderField({ query: '' }) act(() => { @@ -156,8 +158,30 @@ describe('WorkspaceKanbanSearchField', () => { ) }) expect(onClear).toHaveBeenCalledOnce() - expect(bubbled).toHaveBeenCalledOnce() + expect(onClose).toHaveBeenCalledOnce() + }) - document.body.removeEventListener('keydown', bubbled) + it('keeps focus in the field after the clear button unmounts itself', () => { + renderField({ query: 'orca', matchCount: 3, totalCount: 12 }) + act(() => { + input().focus() + clearButton()?.click() + }) + + expect(onClear).toHaveBeenCalledOnce() + expect(document.activeElement).toBe(input()) + }) + + it('reserves counter width so a large count cannot overlap typed text', () => { + renderField({ query: 'orca', matchCount: 298, totalCount: 1024 }) + const wide = Number.parseFloat(input().style.paddingRight) + + renderField({ query: 'orca', matchCount: 3, totalCount: 9 }) + const narrow = Number.parseFloat(input().style.paddingRight) + + expect(wide).toBeGreaterThan(narrow) + // '298 / 1024' is 10 characters, so the reserve must clear the clear button + // plus the full counter rather than a fixed 64px guess. + expect(wide).toBeGreaterThanOrEqual(32 + 10 * 6) }) }) diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx index a418d3fa288..c71054d1e14 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx @@ -1,11 +1,16 @@ -import React, { useEffect, useState } from 'react' +import React, { useEffect, useRef, useState } from 'react' import { Search, X } from 'lucide-react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' -import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' const ANNOUNCE_DEBOUNCE_MS = 400 +// Why: the counter overlays the input, so its width has to be reserved rather +// than guessed — a fixed reserve overlaps typed text once counts reach 3 digits. +// Safe over-estimates at text-[10px] tabular-nums, where '/' and ' ' are narrower. +const CLEAR_BUTTON_RESERVE_PX = 32 +const COUNTER_CHAR_PX = 6 +const COUNTER_GAP_PX = 4 type WorkspaceKanbanSearchFieldProps = { query: string @@ -15,6 +20,8 @@ type WorkspaceKanbanSearchFieldProps = { totalCount: number onQueryChange: (query: string) => void onClear: () => void + /** Escape in an empty field has nothing local to cancel, so it dismisses the board. */ + onClose: () => void } function formatAnnouncement(matchCount: number, totalCount: number): string { @@ -36,9 +43,12 @@ export default function WorkspaceKanbanSearchField({ matchCount, totalCount, onQueryChange, - onClear + onClear, + onClose }: WorkspaceKanbanSearchFieldProps): React.JSX.Element { const hasText = query !== '' + const counterText = isFiltering ? `${matchCount} / ${totalCount}` : null + const inputRef = useRef(null) const [announcement, setAnnouncement] = useState('') // Why: the filter itself is undebounced, but a polite live region that changes @@ -56,39 +66,49 @@ export default function WorkspaceKanbanSearchField({ }, [isFiltering, matchCount, totalCount]) return ( -
- +
+ onQueryChange(event.target.value)} onKeyDown={(event) => { - // Why: the board swallows Radix close requests, so an empty-field - // Escape has nothing local to do and is left to bubble. - if (event.key !== 'Escape' || !hasText) { + // Why: useWorkspaceBoardPanel defers Escape to board text fields, so + // this field is the only handler — it must cover both outcomes. + if (event.key !== 'Escape') { return } event.preventDefault() - event.stopPropagation() - onClear() + if (hasText) { + onClear() + return + } + onClose() }} /> {hasText ? (
- {isFiltering ? ( -
) : null} diff --git a/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.test.tsx b/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.test.tsx index 00018000745..575ec9ab076 100644 --- a/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.test.tsx +++ b/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.test.tsx @@ -52,12 +52,23 @@ async function updatePanel(update: (state: WorkspaceBoardPanelState) => void): P }) } -async function pressEscape(): Promise { +async function pressEscape(from: EventTarget = document): Promise { await act(async () => { - document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })) + from.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })) }) } +function appendInput(inside: 'board' | 'app'): HTMLInputElement { + const host = document.createElement('div') + if (inside === 'board') { + host.setAttribute('data-workspace-board-sheet', '') + } + const field = document.createElement('input') + host.appendChild(field) + document.body.appendChild(host) + return field +} + describe('useWorkspaceBoardPanel', () => { beforeEach(() => { latestState = null @@ -190,6 +201,30 @@ describe('useWorkspaceBoardPanel', () => { expect(panelState().workspaceBoardOpen).toBe(true) }) + it('defers Escape to a text field inside the board', async () => { + // Why: this listener is capture-phase on document, so it runs before React's + // handlers and a board field cannot stopPropagation its way out. The field + // owns Escape and calls closeWorkspaceBoard itself when it has nothing to + // cancel — without this guard, clearing a search query dismissed the board. + await renderHookProbe() + const field = appendInput('board') + + await updatePanel((state) => state.openWorkspaceBoard()) + await pressEscape(field) + + expect(panelState().workspaceBoardOpen).toBe(true) + }) + + it('still closes the board on Escape from a text field outside it', async () => { + await renderHookProbe() + const field = appendInput('app') + + await updatePanel((state) => state.openWorkspaceBoard()) + await pressEscape(field) + + expect(panelState().workspaceBoardOpen).toBe(false) + }) + it('keeps the board open on Escape while a nested dialog is open', async () => { await renderHookProbe() const dialog = document.createElement('div') diff --git a/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.ts b/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.ts index 8d99d225eb3..b77ddb234d8 100644 --- a/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.ts +++ b/src/renderer/src/components/sidebar/useWorkspaceBoardPanel.ts @@ -1,6 +1,21 @@ import { useCallback, useEffect, useRef, useState } from 'react' +import { isEditableTarget } from '@/lib/editable-target' import { useAppStore } from '@/store' +const WORKSPACE_BOARD_SHEET_SELECTOR = '[data-workspace-board-sheet]' + +// Why: the board's Escape listener is capture-phase on document, so it runs +// before React's handlers and a text field inside the board cannot stop it. +// Board fields own Escape and close the board themselves when they have +// nothing left to cancel. +function isWorkspaceBoardEditableTarget(target: EventTarget | null): boolean { + return ( + isEditableTarget(target) && + target instanceof HTMLElement && + target.closest(WORKSPACE_BOARD_SHEET_SELECTOR) !== null + ) +} + const WORKSPACE_BOARD_ESCAPE_BLOCKING_OVERLAY_SELECTOR = [ '[data-slot="dropdown-menu-content"][data-state="open"]', '[data-slot="context-menu-content"][data-state="open"]', @@ -124,6 +139,9 @@ export function useWorkspaceBoardPanel(): WorkspaceBoardPanelState { if (workspaceBoardMenuOpen) { return } + if (isWorkspaceBoardEditableTarget(event.target)) { + return + } // Why: Escape should dismiss interactive nested overlays before this // companion panel, but non-interactive tooltips should not trap it. if (document.querySelector(WORKSPACE_BOARD_ESCAPE_BLOCKING_OVERLAY_SELECTOR)) { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 0b2f99d768d..a8361ed43ee 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5030,7 +5030,6 @@ "bdb753c78d": "No workspaces match", "4d96c209d6": "{{value0}} of {{value1}} workspaces match", "c0cd6bdf6c": "Search workspaces", - "49c266baaa": "Search", "3b7ea51793": "Clear search" } }, diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 1ef31c9d5a8..432ab164ef8 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -5007,7 +5007,6 @@ "bdb753c78d": "Ningún espacio de trabajo coincide", "4d96c209d6": "{{value0}} de {{value1}} espacios de trabajo coinciden", "c0cd6bdf6c": "Buscar espacios de trabajo", - "49c266baaa": "Buscar", "3b7ea51793": "Borrar búsqueda" } }, diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 9ba43069893..e572a5c0f8b 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -5007,7 +5007,6 @@ "bdb753c78d": "一致するワークスペースはありません", "4d96c209d6": "{{value1}} 件中 {{value0}} 件のワークスペースが一致します", "c0cd6bdf6c": "ワークスペースの検索", - "49c266baaa": "検索", "3b7ea51793": "検索をクリア" } }, diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 1bfe9d0e3fa..07d72215a82 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -5007,7 +5007,6 @@ "bdb753c78d": "일치하는 워크스페이스가 없습니다", "4d96c209d6": "워크스페이스 {{value1}}개 중 {{value0}}개 일치", "c0cd6bdf6c": "워크스페이스 검색", - "49c266baaa": "검색", "3b7ea51793": "검색 지우기" } }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 4ddcd3749cc..2d999da42af 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -5007,7 +5007,6 @@ "bdb753c78d": "没有匹配的工作区", "4d96c209d6": "{{value1}} 个工作区中有 {{value0}} 个匹配", "c0cd6bdf6c": "搜索工作区", - "49c266baaa": "搜索", "3b7ea51793": "清除搜索" } },