Skip to content

fix(desktop): stop search shortcut from hijacking the sidebar#1447

Merged
wesbillman merged 1 commit into
mainfrom
brain/search-sidebar-decouple
Jul 1, 2026
Merged

fix(desktop): stop search shortcut from hijacking the sidebar#1447
wesbillman merged 1 commit into
mainfrom
brain/search-sidebar-decouple

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

Problem

Two related bugs reported with global search (⌘K):

  1. ⌘K forces a collapsed sidebar open. The search input actually lives in a centered portal dialog, not the sidebar, so there is no reason for the sidebar to move.
  2. Closing Settings resurrects the search dialog (and the sidebar toggle appears "broken" because the sidebar keeps getting force-opened).

Root cause

Search open is signaled by searchFocusRequest, a monotonically incrementing counter in AppShell. Two effects watched it — one in TopbarSearch (opens the dialog) and one in AppSidebar (setOpen(true) to reveal the pinned-header trigger). Both guarded only on counter === 0, and the counter never resets. Any remount while it was non-zero — e.g. closing Settings, which remounts the sidebar subtree — replayed the last "open search + open sidebar."

Fix

  • AppSidebar.tsx — delete the effect that force-opened the sidebar on every focus request. (The isMobile/setOpenMobile branch was unreachable in the desktop app: Tauri minWidth is 800, mobile breakpoint is 768.)
  • TopbarSearch.tsx — make the open effect edge-triggered via a ref initialized to the current counter, so it fires only on a genuine change, never on remount. Also drop the now-pointless refocus of the (possibly off-screen) sidebar trigger button.

Tests

  • smoke.spec.ts — collapsed-sidebar test now asserts the sidebar stays collapsed on ⌘K (failed on old code with "expected collapsed, received expanded").
  • navigation.spec.ts — settings-shortcut test extended: open search via ⌘K → close → open/close Settings → search must stay hidden (failed on old code, reproducing the report exactly).

Verification

  • just desktop-check, just desktop-typecheck, just desktop-test (1475 unit tests) ✅
  • Both affected smoke E2E specs: 30 passed ✅
  • Full smoke E2E run: two failures in channels.spec.ts / scroll-history.spec.ts are pre-existing on main (reproduced on a clean main checkout) and unrelated to this change.

Global search (⌘K) is triggered by a monotonically incrementing
focus-request counter. Two effects watched it: TopbarSearch opened the
dialog, and AppSidebar force-opened the sidebar so the pinned-header
trigger would be visible. Both effects only skipped when the counter was
0, so any remount while it was non-zero replayed the last open.

That caused two bugs: ⌘K forced the collapsed sidebar open (the search
input actually lives in a portal dialog, not the sidebar), and closing
Settings — which remounts the sidebar subtree — resurrected search.

Delete the sidebar coupling entirely and make the search-open effect
edge-triggered so it fires only on an actual counter change, never on
mount. Update the collapsed-sidebar E2E to assert the sidebar stays put,
and extend the settings-shortcut E2E to guard the remount replay.

Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman wesbillman merged commit 15ad7ae into main Jul 1, 2026
25 checks passed
@wesbillman wesbillman deleted the brain/search-sidebar-decouple branch July 1, 2026 22:53
wpfleger96 added a commit that referenced this pull request Jul 2, 2026
…into HEAD

* origin/paul/nip-am-agent-turn-metrics:
  fix(profile): consolidate agent profile runtime metadata (#1451)
  fix(desktop): simplify workspace rail badges (#1462)
  perf(desktop): instant channel switching — non-blocking first paint, persisted snapshots (#1452)
  perf(relay): bounded-concurrency multi-filter query execution (S2) (#1457)
  fix(desktop): classify timeline prepends so history loads don't bump unread (#1416)
  fix(desktop): quiet gate for workspace switches instead of boot splash (#1449)
  fix(read-path): reach complete threads, dense-second timelines, and all people in the GUI (#1418)
  E1+E3: reduce relay ingest/fan-out DB round trips; ack p99 −7–16%, fd p99 −6–28%, p999 tails −29–53% vs PR #1453 tip (#1454)
  perf(relay): defer post-commit dispatch and avoid verify clone (#1453)
  fix(relay): include git hook tools in runtime image (#1326)
  feat(chart): per-pod emptyDir git scratch when persistence disabled (multi-replica HA) (#1450)
  fix(relay): remove media bearer-token auth (#1444)
  fix(desktop): stop search shortcut from hijacking the sidebar (#1447)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant