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

Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Wes
2026-07-01 16:53:25 -06:00
committed by GitHub
parent 89c4f76579
commit 15ad7ae87e
4 changed files with 40 additions and 25 deletions

View File

@@ -538,13 +538,15 @@ export function TopbarSearch({
],
);
// Edge-trigger: the counter never resets, so `!== 0` would replay on remount.
const lastFocusRequestRef = React.useRef(focusRequest);
React.useEffect(() => {
if (focusRequest === 0) {
if (focusRequest === lastFocusRequestRef.current) {
return;
}
lastFocusRequestRef.current = focusRequest;
openSearchDialog();
triggerRef.current?.focus();
}, [focusRequest, openSearchDialog]);
React.useEffect(() => {

View File

@@ -62,7 +62,6 @@ import {
SidebarMenu,
SidebarMenuItem,
SidebarRail,
useSidebar,
} from "@/shared/ui/sidebar";
type CollapsibleSidebarGroup =
@@ -240,24 +239,6 @@ export function AppSidebar({
const scrollRef = React.useRef<HTMLDivElement>(null);
useSidebarScrollLock(scrollRef);
// Search lives in the sidebar's pinned header, so a ⌘K focus request must
// first reveal the sidebar. When collapsed (offcanvas) on desktop the input
// is mounted but translated off-screen; on mobile it is unmounted entirely.
// Open the sidebar on each focus-request bump so the input the shortcut
// focuses is actually visible. Skips the initial mount (request === 0).
const { isMobile, setOpen, setOpenMobile } = useSidebar();
React.useEffect(() => {
if (searchFocusRequest === 0) {
return;
}
if (isMobile) {
setOpenMobile(true);
} else {
setOpen(true);
}
}, [searchFocusRequest, isMobile, setOpen, setOpenMobile]);
React.useEffect(() => {
const scrollElement = scrollRef.current;
if (!scrollElement) return;

View File

@@ -274,6 +274,26 @@ test("settings shortcut returns without opening search dialog", async ({
await expect(page.getByTestId("chat-title")).toHaveText("general");
const channelUrl = page.url();
// Open search via the ⌘K shortcut so the focus-request counter is non-zero,
// then close it. The Settings subtree remounts the sidebar + search on close,
// which must not replay the stale counter and resurrect search.
await page.evaluate(() => {
const isMac = /mac|iphone|ipad|ipod/i.test(navigator.platform);
window.dispatchEvent(
new KeyboardEvent("keydown", {
bubbles: true,
cancelable: true,
code: "KeyK",
ctrlKey: !isMac,
key: "k",
metaKey: isMac,
}),
);
});
await expect(page.getByTestId("search-dialog-input")).toBeFocused();
await page.keyboard.press("Escape");
await expect(page.getByTestId("search-results")).not.toBeVisible();
await page.keyboard.press(
process.platform === "darwin" ? "Meta+Comma" : "Control+Comma",
);

View File

@@ -318,7 +318,7 @@ test("closes sidebar search with Escape", async ({ page }) => {
await expect(page.getByTestId("open-search")).toBeFocused();
});
test("reopens the collapsed sidebar when the search shortcut fires", async ({
test("search shortcut opens search without disturbing the collapsed sidebar", async ({
page,
}) => {
await page.goto("/");
@@ -334,11 +334,23 @@ test("reopens the collapsed sidebar when the search shortcut fires", async ({
.click();
await expect(sidebarRoot).toHaveAttribute("data-state", "collapsed");
await focusSidebarSearchWithShortcut(page);
await page.evaluate(() => {
const isMac = /mac|iphone|ipad|ipod/i.test(navigator.platform);
window.dispatchEvent(
new KeyboardEvent("keydown", {
bubbles: true,
cancelable: true,
code: "KeyK",
ctrlKey: !isMac,
key: "k",
metaKey: isMac,
}),
);
});
// The shortcut reveals the sidebar and focuses the dialog search input.
await expect(sidebarRoot).toHaveAttribute("data-state", "expanded");
// Search opens in its portal dialog; the sidebar must not react.
await expect(page.getByTestId("search-dialog-input")).toBeFocused();
await expect(sidebarRoot).toHaveAttribute("data-state", "collapsed");
});
test("search results use your resolved profile label instead of You", async ({