fix(desktop): stop search shortcut from hijacking the sidebar#1447
Merged
Conversation
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two related bugs reported with global search (⌘K):
Root cause
Search open is signaled by
searchFocusRequest, a monotonically incrementing counter inAppShell. Two effects watched it — one inTopbarSearch(opens the dialog) and one inAppSidebar(setOpen(true)to reveal the pinned-header trigger). Both guarded only oncounter === 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. (TheisMobile/setOpenMobilebranch was unreachable in the desktop app: TauriminWidthis 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) ✅channels.spec.ts/scroll-history.spec.tsare pre-existing onmain(reproduced on a cleanmaincheckout) and unrelated to this change.