mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-27 17:58:07 +00:00
6d427e82df
PR #53891 (cherry-picked in the three preceding commits) landed the Electron bridge, the store, the find bar overlay, and Cmd/Ctrl+F to open. It stopped short of the rest of the accelerator set and had two lifecycle leaks. This completes the surface to match the platform convention that Chrome, Safari, VS Code, and Claude Desktop's own findInPage bundle all ship. Accelerators now wired: - Cmd/Ctrl+F open the find bar (view.findInPage, from the PR) - Cmd/Ctrl+G find next (new) - Cmd/Ctrl+Shift+G find previous (new) - Enter / Shift+Enter step from the input (from the PR) - Escape close + stopFindInPage('clearSelection') (from the PR) Keyboard ownership (the substantive fix) Cmd+G was already bound to `view.toggleReview` and Escape to `composer.cancel`. The find bar's own capture-phase window listener cannot win those keys by calling stopPropagation: the keybind dispatcher's listener sits on the SAME window target in the SAME phase, and propagation control does not suppress sibling listeners on one target. Left alone, Cmd+G would step a match AND toggle the review pane, and Escape would dismiss the bar AND abort a running turn. So ownership is decided by the dispatcher, which AGENTS.md already names the single owner of combo dispatch: `findBarClaimsCombo` is consulted in use-keybinds before the registry lookup, and yields mod+g / mod+shift+g / escape to the bar only while it is open. Closing the bar hands every one of them straight back. That is the "keyboard ownership follows focus / one cancel gesture does exactly one thing" invariant. `view.findNext` / `view.findPrevious` are registered with EMPTY defaults on purpose — shipping mod+g as a second default would flag a permanent conflict in the keybinds panel against view.toggleReview. The entries document the pair and let a user bind a dedicated chord; stepping is a no-op unless the bar is open with a query, so a bound key can never search invisibly. Listener-leak fixes - The found-in-page bridge subscription is now refcounted in the store, so a remount (the connection re-home path remounts the global overlays) cannot stack duplicate subscribers that each re-dispatch the same result and outlive their component. The subscription is deliberately mount-scoped, not active-scoped: results for an in-flight search must still land if the bar just closed. - `setFindQuery` now refuses to search a closed bar. The component clears its debounce on close, but a 200ms timer that already fired would re-issue a find and re-highlight the page after the user pressed Escape. Caught by the test, fixed in the store rather than papered over in the component. - `closeFindBar` is idempotent — Escape is a shared gesture, so a second close must not reach into Electron again. Pure logic extracted for testing (no source regexing) - `src/lib/find-in-page.ts`: `formatMatchLabel` (three distinct counter states: hidden with no query, explicit 0/0, ordinal/count; clamps the ordinal and never emits NaN — Electron legitimately reports ordinal 0 on a non-final update), `findBarKeyAction` (the keybinding matcher, DOM-free), and `findBarClaimsCombo` (the ownership predicate above). Also: match counter and buttons get accessible names and the counter is aria-live, the hardcoded English "Previous"/"Next"/"Close" tooltips move to i18n (en + zh) alongside the new keybind labels, and the input gets an aria-label so the bar is reachable by role. Tests: apps/desktop/src/components/find-bar.test.tsx — 42 cases over the pure helpers, the store (open/close, next/prev dispatch shape, escape clears, refcount, double-release), and the component (focus on open, debounce coalescing, Cmd+G from outside the input, unmount releases both the bridge subscription and the window listener). cd apps/desktop && npx vitest run src/components/find-bar.test.tsx \ electron/find-in-page.test.ts -> 62 passed (42 new + 20 from the PR) Adjacent suites (src/lib/keybinds, src/i18n, src/store): 487 passed. `tsc -p tsconfig.electron.json --noEmit` clean; `tsc -p .` has 114 pre-existing errors vs 120 on the merge base (all @assistant-ui / bippy / composer), none in the touched files. eslint clean on every touched file. Co-authored-by: David Metcalfe <DavidMetcalfe@users.noreply.github.com> |
||
|---|---|---|
| .. | ||
| bootstrap-installer | ||
| desktop | ||
| shared | ||