Add main sidebar toggle#3497
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review This PR introduces a new sidebar toggle feature with keyboard shortcut handling and UI changes. There are 3 unresolved review comments identifying potential bugs (key repeat behavior, macOS titlebar alignment, event handling inconsistency) that should be addressed. You can customize Macroscope's approvability policy. Learn more. |
juliusmarminge
force-pushed
the
codex/toggle-main-sidebar
branch
from
5df58f6 to
57cb362
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: WCO inset overridden on Mac
- Added WCO visibility state tracking so the inline --workspace-controls-left: 90px style is only applied when Window Controls Overlay is not active, allowing the env(titlebar-area-x)-based CSS from .wco to take effect.
- ✅ Fixed: Key repeat toggles sidebar repeatedly
- Added event.repeat guard to the keydown handler, matching the pattern used by other keyboard shortcut handlers in the codebase.
Or push these changes by commenting:
@cursor push 371f8641cf
Preview (371f8641cf)
diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx
--- a/apps/web/src/components/AppSidebarLayout.tsx
+++ b/apps/web/src/components/AppSidebarLayout.tsx
@@ -1,5 +1,5 @@
import { useAtomValue } from "@effect/atom-react";
-import { useEffect, type CSSProperties, type ReactNode } from "react";
+import { useEffect, useState, type CSSProperties, type ReactNode } from "react";
import { useNavigate } from "@tanstack/react-router";
import { isElectron } from "../env";
@@ -22,7 +22,7 @@
useEffect(() => {
const onKeyDown = (event: KeyboardEvent) => {
- if (event.defaultPrevented) return;
+ if (event.defaultPrevented || event.repeat) return;
if (resolveShortcutCommand(event, keybindings) !== "sidebar.toggle") return;
event.preventDefault();
@@ -48,8 +48,25 @@
export function AppSidebarLayout({ children }: { children: ReactNode }) {
const navigate = useNavigate();
+
+ const [isWcoVisible, setIsWcoVisible] = useState(
+ () =>
+ (navigator as Navigator & { windowControlsOverlay?: { visible: boolean } })
+ .windowControlsOverlay?.visible ?? false,
+ );
+
+ useEffect(() => {
+ const overlay = (
+ navigator as Navigator & { windowControlsOverlay?: EventTarget & { visible: boolean } }
+ ).windowControlsOverlay;
+ if (!overlay) return;
+ const update = () => setIsWcoVisible(overlay.visible);
+ overlay.addEventListener("geometrychange", update);
+ return () => overlay.removeEventListener("geometrychange", update);
+ }, []);
+
const macosWindowControlsStyle =
- isElectron && isMacPlatform(navigator.platform)
+ isElectron && isMacPlatform(navigator.platform) && !isWcoVisible
? ({ "--workspace-controls-left": MACOS_TRAFFIC_LIGHTS_LEFT_INSET } as CSSProperties)
: undefined;You can send follow-ups to the cloud agent here.
|
|
||
| return ( | ||
| <SidebarProvider className="h-dvh! min-h-0!" defaultOpen> | ||
| <SidebarProvider className="h-dvh! min-h-0!" defaultOpen style={macosWindowControlsStyle}> |
Contributor
There was a problem hiding this comment.
WCO inset overridden on Mac
Medium Severity
On macOS Electron, SidebarProvider sets a fixed --workspace-controls-left to 90px. This overrides the env(titlebar-area-x) value used for Window Controls Overlay, causing the SidebarControl and collapsed sidebar header padding to misalign with native titlebar geometry.
Reviewed by Cursor Bugbot for commit 57cb362. Configure here.
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| toggleSidebar(); |
Contributor
There was a problem hiding this comment.
Key repeat toggles sidebar repeatedly
Medium Severity
The sidebar.toggle keydown handler in SidebarControl doesn't check event.repeat, causing the sidebar to toggle rapidly when the shortcut is held down.
Reviewed by Cursor Bugbot for commit 57cb362. Configure here.
juliusmarminge
force-pushed
the
codex/toggle-main-sidebar
branch
from
57cb362 to
41d4e81
Compare
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Mod+B blocked after preventDefault
- Changed the sidebar.toggle keydown listener from bubble phase to capture phase (addEventListener third arg: true) so it runs before focused elements like terminals can call preventDefault and block the shortcut, matching how ChatView handles other panel shortcuts.
Or push these changes by commenting:
@cursor push 028dc21f09
Preview (028dc21f09)
diff --git a/apps/server/src/keybindings.test.ts b/apps/server/src/keybindings.test.ts
--- a/apps/server/src/keybindings.test.ts
+++ b/apps/server/src/keybindings.test.ts
@@ -198,6 +198,7 @@
assert.equal(defaultsByCommand.get("thread.jump.1"), "mod+1");
assert.equal(defaultsByCommand.get("thread.jump.9"), "mod+9");
assert.equal(defaultsByCommand.get("modelPicker.toggle"), "mod+shift+m");
+ assert.equal(defaultsByCommand.get("sidebar.toggle"), "mod+b");
assert.equal(defaultsByCommand.get("rightPanel.toggle"), "mod+alt+b");
assert.equal(defaultsByCommand.get("terminal.splitVertical"), "mod+shift+d");
assert.equal(defaultsByCommand.get("modelPicker.jump.1"), "mod+1");
diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx
--- a/apps/web/src/components/AppSidebarLayout.tsx
+++ b/apps/web/src/components/AppSidebarLayout.tsx
@@ -1,14 +1,57 @@
-import { useEffect, type ReactNode } from "react";
+import { useAtomValue } from "@effect/atom-react";
+import { useEffect, type CSSProperties, type ReactNode } from "react";
import { useNavigate } from "@tanstack/react-router";
+import { isElectron } from "../env";
+import { resolveShortcutCommand, shortcutLabelForCommand } from "../keybindings";
+import { isMacPlatform } from "../lib/utils";
+import { primaryServerKeybindingsAtom } from "../state/server";
import ThreadSidebar from "./Sidebar";
-import { Sidebar, SidebarProvider, SidebarRail } from "./ui/sidebar";
+import { Sidebar, SidebarProvider, SidebarRail, SidebarTrigger, useSidebar } from "./ui/sidebar";
+import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip";
const THREAD_SIDEBAR_WIDTH_STORAGE_KEY = "chat_thread_sidebar_width";
const THREAD_SIDEBAR_MIN_WIDTH = 13 * 16;
const THREAD_MAIN_CONTENT_MIN_WIDTH = 40 * 16;
+const MACOS_TRAFFIC_LIGHTS_LEFT_INSET = "90px";
+
+function SidebarControl() {
+ const keybindings = useAtomValue(primaryServerKeybindingsAtom);
+ const { toggleSidebar } = useSidebar();
+ const shortcutLabel = shortcutLabelForCommand(keybindings, "sidebar.toggle");
+
+ useEffect(() => {
+ const onKeyDown = (event: KeyboardEvent) => {
+ if (event.defaultPrevented) return;
+ if (resolveShortcutCommand(event, keybindings) !== "sidebar.toggle") return;
+
+ event.preventDefault();
+ event.stopPropagation();
+ toggleSidebar();
+ };
+
+ window.addEventListener("keydown", onKeyDown, true);
+ return () => window.removeEventListener("keydown", onKeyDown, true);
+ }, [keybindings, toggleSidebar]);
+
+ return (
+ <div className="fixed left-[var(--workspace-controls-left)] top-[var(--workspace-controls-top)] z-50 flex h-[var(--workspace-topbar-height)] items-center [-webkit-app-region:no-drag]">
+ <Tooltip>
+ <TooltipTrigger render={<SidebarTrigger aria-label="Toggle main sidebar" />} />
+ <TooltipPopup side="bottom">
+ Toggle main sidebar{shortcutLabel ? ` (${shortcutLabel})` : ""}
+ </TooltipPopup>
+ </Tooltip>
+ </div>
+ );
+}
+
export function AppSidebarLayout({ children }: { children: ReactNode }) {
const navigate = useNavigate();
+ const macosWindowControlsStyle =
+ isElectron && isMacPlatform(navigator.platform)
+ ? ({ "--workspace-controls-left": MACOS_TRAFFIC_LIGHTS_LEFT_INSET } as CSSProperties)
+ : undefined;
useEffect(() => {
const onMenuAction = window.desktopBridge?.onMenuAction;
@@ -28,7 +71,8 @@
}, [navigate]);
return (
- <SidebarProvider className="h-dvh! min-h-0!" defaultOpen>
+ <SidebarProvider className="h-dvh! min-h-0!" defaultOpen style={macosWindowControlsStyle}>
+ <SidebarControl />
<Sidebar
side="left"
collapsible="offcanvas"
diff --git a/apps/web/src/components/NoActiveThreadState.tsx b/apps/web/src/components/NoActiveThreadState.tsx
--- a/apps/web/src/components/NoActiveThreadState.tsx
+++ b/apps/web/src/components/NoActiveThreadState.tsx
@@ -1,16 +1,18 @@
import { Empty, EmptyDescription, EmptyHeader, EmptyTitle } from "./ui/empty";
-import { SidebarInset, SidebarTrigger } from "./ui/sidebar";
+import { SidebarInset, useSidebarVisibility } from "./ui/sidebar";
import { isElectron } from "../env";
import { cn } from "~/lib/utils";
export function NoActiveThreadState() {
+ const sidebarOpen = useSidebarVisibility();
return (
<SidebarInset className="h-dvh min-h-0 overflow-hidden overscroll-y-none bg-background text-foreground">
<div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-x-hidden bg-background">
<header
className={cn(
- "border-b border-border px-3 sm:px-5",
+ "border-b border-border px-3 transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none sm:px-5",
isElectron ? "workspace-topbar drag-region" : "workspace-topbar",
+ !sidebarOpen && "pl-[calc(var(--workspace-controls-left)+2.5rem)]",
)}
>
{isElectron ? (
@@ -19,7 +21,6 @@
</span>
) : (
<div className="flex items-center gap-2">
- <SidebarTrigger className="size-7 shrink-0 md:hidden" />
<span className="text-sm font-medium text-foreground md:text-muted-foreground/60">
No active thread
</span>
diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx
--- a/apps/web/src/components/Sidebar.tsx
+++ b/apps/web/src/components/Sidebar.tsx
@@ -70,7 +70,7 @@
type SidebarThreadSortOrder,
} from "@t3tools/contracts/settings";
import { isElectron } from "../env";
-import { APP_STAGE_LABEL, APP_VERSION } from "../branding";
+import { APP_VERSION } from "../branding";
import { useOpenPrLink } from "../lib/openPullRequestLink";
import { isTerminalFocused } from "../lib/terminalFocus";
import { isMacPlatform } from "../lib/utils";
@@ -192,7 +192,6 @@
orderItemsByPreferredIds,
shouldClearThreadSelectionOnMouseDown,
sortProjectsForSidebar,
- resolveSidebarStageBadgeLabel,
useThreadJumpHintVisibility,
ThreadStatusPill,
} from "./Sidebar.logic";
@@ -202,7 +201,7 @@
import { useIsMobile } from "~/hooks/useMediaQuery";
import { CommandDialogTrigger } from "./ui/command";
import { useClientSettings, useUpdateClientSettings } from "~/hooks/useSettings";
-import { primaryServerConfigAtom, primaryServerKeybindingsAtom } from "../state/server";
+import { primaryServerKeybindingsAtom } from "../state/server";
import {
derivePhysicalProjectKey,
deriveProjectGroupingOverrideKey,
@@ -2452,22 +2451,6 @@
);
});
-function T3Wordmark() {
- return (
- <svg
- aria-label="T3"
- className="h-2.5 w-auto shrink-0 text-foreground"
- viewBox="15.5309 37 94.3941 56.96"
- xmlns="http://www.w3.org/2000/svg"
- >
- <path
- d="M33.4509 93V47.56H15.5309V37H64.3309V47.56H46.4109V93H33.4509ZM86.7253 93.96C82.832 93.96 78.9653 93.4533 75.1253 92.44C71.2853 91.3733 68.032 89.88 65.3653 87.96L70.4053 78.04C72.5386 79.5867 75.0186 80.8133 77.8453 81.72C80.672 82.6267 83.5253 83.08 86.4053 83.08C89.6586 83.08 92.2186 82.44 94.0853 81.16C95.952 79.88 96.8853 78.12 96.8853 75.88C96.8853 73.7467 96.0586 72.0667 94.4053 70.84C92.752 69.6133 90.0853 69 86.4053 69H80.4853V60.44L96.0853 42.76L97.5253 47.4H68.1653V37H107.365V45.4L91.8453 63.08L85.2853 59.32H89.0453C95.9253 59.32 101.125 60.8667 104.645 63.96C108.165 67.0533 109.925 71.0267 109.925 75.88C109.925 79.0267 109.099 81.9867 107.445 84.76C105.792 87.48 103.259 89.6933 99.8453 91.4C96.432 93.1067 92.0586 93.96 86.7253 93.96Z"
- fill="currentColor"
- />
- </svg>
- );
-}
-
type SortableProjectHandleProps = Pick<
ReturnType<typeof useSortable>,
"attributes" | "listeners" | "setActivatorNodeRef"
@@ -2664,49 +2647,50 @@
}: {
isElectron: boolean;
}) {
- const primaryServerVersion =
- useAtomValue(primaryServerConfigAtom)?.environment.serverVersion ?? null;
- const stageBadgeLabel = resolveSidebarStageBadgeLabel({
- primaryServerVersion,
- fallbackStageLabel: APP_STAGE_LABEL,
- });
- const wordmark = (
- <div className="flex items-center gap-2">
- <SidebarTrigger className="shrink-0 md:hidden" />
- <Tooltip>
- <TooltipTrigger
- render={
- <Link
- aria-label="Go to threads"
- className="ml-1 flex min-w-0 flex-1 cursor-pointer items-center gap-1 rounded-md outline-hidden ring-ring transition-colors hover:text-foreground focus-visible:ring-2"
- to="/"
- >
- <T3Wordmark />
- <span className="truncate text-sm font-medium tracking-tight text-muted-foreground">
- Code
- </span>
- <span className="rounded-full bg-muted/50 px-1.5 py-0.5 text-[8px] font-medium uppercase tracking-[0.18em] text-muted-foreground/60">
- {stageBadgeLabel}
- </span>
- </Link>
- }
- />
- <TooltipPopup side="bottom" sideOffset={2}>
- Version {APP_VERSION}
- </TooltipPopup>
- </Tooltip>
- </div>
- );
-
return isElectron ? (
- <SidebarHeader className="drag-region h-[52px] flex-row items-center gap-2 px-4 py-0 pl-[90px] wco:h-[env(titlebar-area-height)] wco:pl-[calc(env(titlebar-area-x)+1em)]">
- {wordmark}
+ <SidebarHeader className="@container/sidebar-header drag-region h-[var(--workspace-topbar-height)] shrink-0 flex-row items-center px-3 py-0">
+ <SidebarTrigger className="md:hidden" />
+ <SidebarBrand />
</SidebarHeader>
) : (
- <SidebarHeader className="gap-3 px-3 py-2 sm:gap-2.5 sm:px-4 sm:py-3">{wordmark}</SidebarHeader>
+ <SidebarHeader className="@container/sidebar-header h-[var(--workspace-topbar-height)] shrink-0 flex-row items-center px-3 py-0">
+ <SidebarTrigger className="md:hidden" />
+ <SidebarBrand />
+ </SidebarHeader>
);
});
+function SidebarBrand() {
+ return (
+ <Link
+ aria-label="Go to threads"
+ className="sidebar-brand h-full min-w-0 flex-1 items-center gap-1 overflow-hidden pl-[calc(var(--workspace-controls-left)+2.5rem)] text-foreground outline-hidden ring-ring focus-visible:ring-2"
+ to="/"
+ >
+ <T3Wordmark />
+ <span className="truncate text-sm font-medium tracking-tight text-muted-foreground">
+ Code
+ </span>
+ </Link>
+ );
+}
+
+function T3Wordmark() {
+ return (
+ <svg
+ aria-label="T3"
+ className="h-2.5 w-auto shrink-0 text-foreground"
+ viewBox="15.5309 37 94.3941 56.96"
+ xmlns="http://www.w3.org/2000/svg"
+ >
+ <path
+ d="M33.4509 93V47.56H15.5309V37H64.3309V47.56H46.4109V93H33.4509ZM86.7253 93.96C82.832 93.96 78.9653 93.4533 75.1253 92.44C71.2853 91.3733 68.032 89.88 65.3653 87.96L70.4053 78.04C72.5386 79.5867 75.0186 80.8133 77.8453 81.72C80.672 82.6267 83.5253 83.08 86.4053 83.08C89.6586 83.08 92.2186 82.44 94.0853 81.16C95.952 79.88 96.8853 78.12 96.8853 75.88C96.8853 73.7467 96.0586 72.0667 94.4053 70.84C92.752 69.6133 90.0853 69 86.4053 69H80.4853V60.44L96.0853 42.76L97.5253 47.4H68.1653V37H107.365V45.4L91.8453 63.08L85.2853 59.32H89.0453C95.9253 59.32 101.125 60.8667 104.645 63.96C108.165 67.0533 109.925 71.0267 109.925 75.88C109.925 79.0267 109.099 81.9867 107.445 84.76C105.792 87.48 103.259 89.6933 99.8453 91.4C96.432 93.1067 92.0586 93.96 86.7253 93.96Z"
+ fill="currentColor"
+ />
+ </svg>
+ );
+}
+
const SidebarChromeFooter = memo(function SidebarChromeFooter() {
const navigate = useNavigate();
const { isMobile, setOpenMobile } = useSidebar();
@@ -2730,6 +2714,9 @@
>
<SettingsIcon className="size-3.5" />
<span className="text-xs">Settings</span>
+ <span className="ml-auto font-mono text-[10px] text-muted-foreground/45">
+ v{APP_VERSION}
+ </span>
</SidebarMenuButton>
</SidebarMenuItem>
</SidebarMenu>
diff --git a/apps/web/src/components/chat/ChatHeader.tsx b/apps/web/src/components/chat/ChatHeader.tsx
--- a/apps/web/src/components/chat/ChatHeader.tsx
+++ b/apps/web/src/components/chat/ChatHeader.tsx
@@ -14,8 +14,8 @@
type NewProjectScriptInput,
type ProjectScriptActionResult,
} from "../ProjectScriptsControl";
-import { SidebarTrigger } from "../ui/sidebar";
import { OpenInPicker } from "./OpenInPicker";
+import { useSidebarVisibility } from "../ui/sidebar";
import { usePrimaryEnvironmentId } from "../../state/environments";
import { cn } from "~/lib/utils";
@@ -72,6 +72,7 @@
onDeleteProjectScript,
}: ChatHeaderProps) {
const primaryEnvironmentId = usePrimaryEnvironmentId();
+ const sidebarOpen = useSidebarVisibility();
const showOpenInPicker = shouldShowOpenInPicker({
activeProjectName,
activeThreadEnvironmentId,
@@ -79,8 +80,12 @@
});
return (
<div className="@container/header-actions flex min-w-0 flex-1 items-center gap-2 sm:gap-3">
- <div className="flex min-w-0 flex-1 items-center gap-2 overflow-hidden sm:gap-3">
- <SidebarTrigger className="size-7 shrink-0 md:hidden" />
+ <div
+ className={cn(
+ "flex min-w-0 flex-1 items-center gap-2 overflow-hidden transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none sm:gap-3",
+ !sidebarOpen && "pl-[calc(var(--workspace-controls-left)+2.5rem)]",
+ )}
+ >
<Tooltip>
<TooltipTrigger
render={
diff --git a/apps/web/src/components/ui/sidebar.tsx b/apps/web/src/components/ui/sidebar.tsx
--- a/apps/web/src/components/ui/sidebar.tsx
+++ b/apps/web/src/components/ui/sidebar.tsx
@@ -85,6 +85,11 @@
return context;
}
+function useSidebarVisibility() {
+ const { isMobile, open, openMobile } = useSidebar();
+ return isMobile ? openMobile : open;
+}
+
function SidebarProvider({
defaultOpen = true,
open: openProp,
@@ -310,13 +315,15 @@
}
function SidebarTrigger({ className, onClick, ...props }: React.ComponentProps<typeof Button>) {
- const { toggleSidebar, openMobile } = useSidebar();
+ const { toggleSidebar } = useSidebar();
+ const isOpen = useSidebarVisibility();
return (
<Button
- className={cn("size-7", className)}
+ className={cn("size-7 sm:size-7", className)}
data-sidebar="trigger"
data-slot="sidebar-trigger"
+ aria-pressed={isOpen}
onClick={(event) => {
onClick?.(event);
toggleSidebar();
@@ -325,7 +332,7 @@
variant="ghost"
{...props}
>
- {openMobile ? <PanelLeftCloseIcon /> : <PanelLeftIcon />}
+ {isOpen ? <PanelLeftCloseIcon /> : <PanelLeftIcon />}
<span className="sr-only">Toggle Sidebar</span>
</Button>
);
@@ -1004,4 +1011,5 @@
SidebarSeparator,
SidebarTrigger,
useSidebar,
+ useSidebarVisibility,
};
diff --git a/apps/web/src/index.css b/apps/web/src/index.css
--- a/apps/web/src/index.css
+++ b/apps/web/src/index.css
@@ -7,6 +7,7 @@
:root {
--workspace-topbar-height: 52px;
--workspace-controls-top: 0px;
+ --workspace-controls-left: calc(env(safe-area-inset-left) + 0.75rem);
--workspace-controls-right: calc(env(safe-area-inset-right) + 0.75rem);
--workspace-native-controls-inset: 0px;
}
@@ -14,6 +15,7 @@
.wco {
--workspace-topbar-height: env(titlebar-area-height, 52px);
--workspace-controls-top: env(titlebar-area-y, 0px);
+ --workspace-controls-left: calc(env(titlebar-area-x, 0px) + 0.75rem);
--workspace-controls-right: calc(
100vw - env(titlebar-area-width, 100vw) - env(titlebar-area-x, 0px) + 0.75rem
);
@@ -90,6 +92,18 @@
}
@layer components {
+ .sidebar-brand {
+ display: none;
+ }
+
+ @media (min-width: 48rem) {
+ @container sidebar-header (min-width: 14.5rem) {
+ .sidebar-brand {
+ display: flex;
+ }
+ }
+ }
+
.workspace-topbar {
display: flex;
height: var(--workspace-topbar-height);
diff --git a/apps/web/src/keybindings.test.ts b/apps/web/src/keybindings.test.ts
--- a/apps/web/src/keybindings.test.ts
+++ b/apps/web/src/keybindings.test.ts
@@ -85,6 +85,7 @@
}
const DEFAULT_BINDINGS = compile([
+ { shortcut: modShortcut("b"), command: "sidebar.toggle" },
{ shortcut: modShortcut("j"), command: "terminal.toggle" },
{ shortcut: modShortcut("b", { altKey: true }), command: "rightPanel.toggle" },
{
@@ -312,6 +313,10 @@
});
it("returns effective labels for non-terminal commands", () => {
+ assert.strictEqual(
+ shortcutLabelForCommand(DEFAULT_BINDINGS, "sidebar.toggle", "MacIntel"),
+ "⌘B",
+ );
assert.strictEqual(shortcutLabelForCommand(DEFAULT_BINDINGS, "chat.new", "MacIntel"), "⇧⌘O");
assert.strictEqual(shortcutLabelForCommand(DEFAULT_BINDINGS, "diff.toggle", "Linux"), "Ctrl+D");
assert.strictEqual(
diff --git a/apps/web/src/routes/_chat.index.tsx b/apps/web/src/routes/_chat.index.tsx
--- a/apps/web/src/routes/_chat.index.tsx
+++ b/apps/web/src/routes/_chat.index.tsx
@@ -4,10 +4,11 @@
import { NoActiveThreadState } from "../components/NoActiveThreadState";
import { Button } from "../components/ui/button";
import { Empty, EmptyDescription, EmptyHeader, EmptyTitle } from "../components/ui/empty";
-import { SidebarInset, SidebarTrigger } from "../components/ui/sidebar";
+import { SidebarInset, useSidebarVisibility } from "../components/ui/sidebar";
import { useEnvironments } from "../state/environments";
import { APP_DISPLAY_NAME } from "~/branding";
import { hasCloudPublicConfig } from "~/cloud/publicConfig";
+import { cn } from "~/lib/utils";
function ChatIndexRouteView() {
const { authGateState } = Route.useRouteContext();
@@ -26,13 +27,18 @@
function HostedStaticOnboardingState() {
const cloudEnabled = hasCloudPublicConfig();
+ const sidebarOpen = useSidebarVisibility();
return (
<SidebarInset className="h-dvh min-h-0 overflow-hidden overscroll-y-none bg-background text-foreground">
<div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-x-hidden bg-background">
- <header className="border-b border-border px-3 py-2 sm:px-5 sm:py-3">
+ <header
+ className={cn(
+ "border-b border-border px-3 py-2 transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none sm:px-5 sm:py-3",
+ !sidebarOpen && "pl-[calc(var(--workspace-controls-left)+2.5rem)]",
+ )}
+ >
<div className="flex items-center gap-2">
- <SidebarTrigger className="size-7 shrink-0 md:hidden" />
<span className="text-sm font-medium text-foreground md:text-muted-foreground/60">
{APP_DISPLAY_NAME}
</span>
diff --git a/apps/web/src/routes/settings.tsx b/apps/web/src/routes/settings.tsx
--- a/apps/web/src/routes/settings.tsx
+++ b/apps/web/src/routes/settings.tsx
@@ -11,8 +11,9 @@
import { useSettingsRestore } from "../components/settings/SettingsPanels";
import { Button } from "../components/ui/button";
-import { SidebarInset, SidebarTrigger } from "../components/ui/sidebar";
+import { SidebarInset, useSidebarVisibility } from "../components/ui/sidebar";
import { isElectron } from "../env";
+import { cn } from "~/lib/utils";
function RestoreDefaultsButton({ onRestored }: { onRestored: () => void }) {
const { changedSettingLabels, restoreDefaults } = useSettingsRestore(onRestored);
@@ -35,6 +36,7 @@
const navigate = useNavigate();
const canGoBack = useCanGoBack();
const [restoreSignal, setRestoreSignal] = useState(0);
+ const sidebarOpen = useSidebarVisibility();
const showRestoreDefaults = location.pathname === "/settings/general";
const handleRestored = () => setRestoreSignal((value) => value + 1);
const navigateBackWithinApp = useCallback(() => {
@@ -64,9 +66,13 @@
<SidebarInset className="h-dvh min-h-0 overflow-hidden overscroll-y-none bg-background text-foreground isolate">
<div className="flex min-h-0 min-w-0 flex-1 flex-col bg-background text-foreground">
{!isElectron && (
- <header className="border-b border-border px-3 py-2 sm:px-5">
+ <header
+ className={cn(
+ "border-b border-border px-3 py-2 transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none sm:px-5",
+ !sidebarOpen && "pl-[calc(var(--workspace-controls-left)+2.5rem)]",
+ )}
+ >
<div className="flex min-h-7 items-center gap-2 sm:min-h-6">
- <SidebarTrigger className="size-7 shrink-0 md:hidden" />
<span className="text-sm font-medium text-foreground">Settings</span>
{showRestoreDefaults ? (
<div className="ms-auto flex items-center gap-2">
@@ -78,7 +84,12 @@
)}
{isElectron && (
- <div className="drag-region flex h-[52px] shrink-0 items-center border-b border-border px-5 wco:h-[env(titlebar-area-height)] wco:pr-[calc(100vw-env(titlebar-area-width)-env(titlebar-area-x)+1em)]">
+ <div
+ className={cn(
+ "drag-region flex h-[52px] shrink-0 items-center border-b border-border px-5 transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none wco:h-[env(titlebar-area-height)] wco:pr-[calc(100vw-env(titlebar-area-width)-env(titlebar-area-x)+1em)]",
+ !sidebarOpen && "pl-[calc(var(--workspace-controls-left)+2.5rem)]",
+ )}
+ >
<span className="text-xs font-medium tracking-wide text-muted-foreground/70">
Settings
</span>
diff --git a/packages/contracts/src/keybindings.test.ts b/packages/contracts/src/keybindings.test.ts
--- a/packages/contracts/src/keybindings.test.ts
+++ b/packages/contracts/src/keybindings.test.ts
@@ -29,6 +29,12 @@
});
assert.strictEqual(parsed.command, "terminal.toggle");
+ const parsedSidebarToggle = yield* decode(KeybindingRule, {
+ key: "mod+b",
+ command: "sidebar.toggle",
+ });
+ assert.strictEqual(parsedSidebarToggle.command, "sidebar.toggle");
+
const parsedRightPanelToggle = yield* decode(KeybindingRule, {
key: "mod+alt+b",
command: "rightPanel.toggle",
diff --git a/packages/contracts/src/keybindings.ts b/packages/contracts/src/keybindings.ts
--- a/packages/contracts/src/keybindings.ts
+++ b/packages/contracts/src/keybindings.ts
@@ -48,6 +48,7 @@
export type ModelPickerKeybindingCommand = (typeof MODEL_PICKER_KEYBINDING_COMMANDS)[number];
const STATIC_KEYBINDING_COMMANDS = [
+ "sidebar.toggle",
"terminal.toggle",
"terminal.split",
"terminal.splitVertical",
diff --git a/packages/shared/src/keybindings.ts b/packages/shared/src/keybindings.ts
--- a/packages/shared/src/keybindings.ts
+++ b/packages/shared/src/keybindings.ts
@@ -19,6 +19,7 @@
| { type: "rparen" };
export const DEFAULT_KEYBINDINGS: ReadonlyArray<KeybindingRule> = [
+ { key: "mod+b", command: "sidebar.toggle" },
{ key: "mod+j", command: "terminal.toggle" },
{ key: "mod+alt+b", command: "rightPanel.toggle" },
{ key: "mod+d", command: "terminal.split", when: "terminalFocus" },You can send follow-ups to the cloud agent here.
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| toggleSidebar(); |
Contributor
There was a problem hiding this comment.
Mod+B blocked after preventDefault
Medium Severity
sidebar.toggle is handled on window bubble and returns early when event.defaultPrevented is set. Focused targets that call preventDefault on Mod+B before bubble can block toggling, unlike rightPanel.toggle, which is handled in ChatView’s capture listener and stops propagation earlier.
Reviewed by Cursor Bugbot for commit 41d4e81. Configure here.
juliusmarminge
force-pushed
the
codex/toggle-main-sidebar
branch
from
41d4e81 to
699be75
Compare
- Apply the closed-sidebar left padding at the `sm` breakpoint too - Keep the topbar/header spacing consistent on chat, onboarding, and settings screens
- Move the sidebar toggle and shared titlebar inset styling into reusable helpers - Keep header controls aligned when the sidebar is collapsed - Update sidebar tests for the new trigger sizing and collapsed state
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Mobile titlebar inset missing
- Changed the state derivation in SidebarProvider to treat mobile viewports as always 'collapsed', ensuring data-sidebar-state triggers the COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS padding when the Sheet overlay sidebar doesn't push content.
Or push these changes by commenting:
@cursor push 722d13527c
Preview (722d13527c)
diff --git a/apps/web/src/components/ui/sidebar.tsx b/apps/web/src/components/ui/sidebar.tsx
--- a/apps/web/src/components/ui/sidebar.tsx
+++ b/apps/web/src/components/ui/sidebar.tsx
@@ -137,7 +137,9 @@
// We add a state so that we can do data-state="expanded" or "collapsed".
// This makes it easier to style the sidebar with Tailwind classes.
- const state = open ? "expanded" : "collapsed";
+ // On mobile the sidebar is a Sheet overlay that never pushes content, so
+ // treat it as always collapsed for ancestor-based inset styling.
+ const state = isMobile || !open ? "collapsed" : "expanded";
const contextValue = React.useMemo<SidebarContextProps>(
() => ({You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 6cd4760. Configure here.
- Resolve shared sidebar state from the active responsive flag - Add coverage for mobile sheet visibility precedence
Co-authored-by: codex <codex@users.noreply.github.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.
Summary
Mod+Bas the default shortcutValidation
vp checkvp run typecheckNote
Low Risk
UI and keybinding defaults only;
mod+bis distinct frommod+alt+bfor the right panel, with targeted tests covering state and trigger behavior.Overview
Adds a main sidebar toggle with default shortcut
mod+b, registered assidebar.togglein contracts and default keybindings.A fixed
SidebarControlinAppSidebarLayouthosts the toggle (tooltip + global key handler) and applies macOS Electron control inset via CSS variables. InlineSidebarTriggerbuttons are removed from chat, settings, and empty-state headers; those bars instead useCOLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASSso content animates left when the sidebar is collapsed.Sidebar state for styling now follows mobile sheet vs desktop open flags (
resolveSidebarState,data-sidebar-stateon the provider).SidebarTriggerreflects open/closed viauseSidebarVisibility, uses titlebar sizing, and stays clickable in drag regions (no-drag).Reviewed by Cursor Bugbot for commit c0f9412. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add main sidebar toggle button and
mod+bkeybindingSidebarControlcomponent to the main layout that renders a persistent sidebar toggle button in the titlebar area, replacing per-pageSidebarTriggerbuttons removed from chat, settings, and onboarding headers.mod+b(⌘B on Mac) as the default keybinding forsidebar.toggleinpackages/shared/src/keybindings.tsand the contracts schema.resolveSidebarStateanduseSidebarVisibilityto compute a unified expanded/collapsed state across mobile and desktop, exposed via adata-sidebar-stateattribute onSidebarProvider.COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASSfor consistent left-padding transitions in content headers when the sidebar is collapsed.Macroscope summarized c0f9412.