From 62382d9828a9c159b7a23d1469b7bee062d67bc5 Mon Sep 17 00:00:00 2001
From: Jesse_Chen <chen.yue@mevixaga.com>
Date: Fri, 17 Jul 2026 17:32:42 +0800
Subject: [PATCH] fix: tighten composable sidebar primitives

---
 frontend/src/components/ui/sidebar.tsx     | 24 +++++++++++------
 frontend/src/hooks/use-sidebar-viewport.ts |  3 ++-
 frontend/tests/sidebar-contract.test.ts    | 31 ++++++++++++++++++++--
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/frontend/src/components/ui/sidebar.tsx b/frontend/src/components/ui/sidebar.tsx
index 0e0aa519..b276f8ec 100644
--- a/frontend/src/components/ui/sidebar.tsx
+++ b/frontend/src/components/ui/sidebar.tsx
@@ -81,6 +81,7 @@ export function SidebarProvider({
   const { viewport, ready } = useSidebarViewport();
   const [uncontrolledOpen, setUncontrolledOpen] = useState(defaultOpen);
   const [openMobile, setOpenMobileState] = useState(false);
+  const openMobileRef = useRef(false);
   const userChangedDesktopState = useRef(false);
   const insetTriggerRef = useRef<HTMLButtonElement | null>(null);
   const sidebarTriggerRef = useRef<HTMLButtonElement | null>(null);
@@ -96,6 +97,8 @@ export function SidebarProvider({
   }, [isControlled, onOpenChange]);
 
   const setOpenMobile = useCallback((nextOpen: boolean) => {
+    if (openMobileRef.current === nextOpen) return;
+    openMobileRef.current = nextOpen;
     setOpenMobileState(nextOpen);
     onMobileOpenChange?.(nextOpen);
   }, [onMobileOpenChange]);
@@ -107,10 +110,10 @@ export function SidebarProvider({
   }, [isControlled, ready, viewport]);
 
   useEffect(() => {
-    if (viewport === "mobile") return;
+    if (viewport === "mobile" || !openMobile) return;
     const closeMobileDrawer = window.requestAnimationFrame(() => setOpenMobile(false));
     return () => window.cancelAnimationFrame(closeMobileDrawer);
-  }, [setOpenMobile, viewport]);
+  }, [openMobile, setOpenMobile, viewport]);
 
   useEffect(() => {
     const handleKeydown = (event: KeyboardEvent) => {
@@ -168,9 +171,12 @@ export function SidebarProvider({
   );
 }
 
-export function Sidebar({ className, ...props }: ComponentProps<"aside">) {
-  const { open, openMobile } = useSidebar();
-  return <aside data-sidebar="sidebar" data-slot="sidebar" data-state={open ? "expanded" : "collapsed"} data-mobile-open={openMobile} className={cn("flex min-h-0 shrink-0 flex-col", className)} {...props} />;
+export function Sidebar({ id, className, ...props }: ComponentProps<"aside">) {
+  const { isMobile, open, openMobile, setOpenMobile } = useSidebar();
+  return <>
+    <button type="button" data-sidebar="scrim" data-slot="sidebar-scrim" data-mobile-open={isMobile && openMobile} aria-label="关闭聊天记录" aria-hidden={!isMobile || !openMobile} tabIndex={isMobile && openMobile ? 0 : -1} className="sidebar-scrim" onClick={() => setOpenMobile(false)} />
+    <aside id={id ?? "chat-sidebar"} data-sidebar="sidebar" data-slot="sidebar" data-state={open ? "expanded" : "collapsed"} data-mobile-open={openMobile} className={cn("flex min-h-0 shrink-0 flex-col", className)} {...props} />
+  </>;
 }
 
 export function SidebarHeader({ className, ...props }: ComponentProps<"div">) {
@@ -230,7 +236,9 @@ export const SidebarTrigger = forwardRef<HTMLButtonElement, SidebarTriggerProps>
 }, ref) {
   const { isMobile, open, openMobile, setInsetTrigger, setSidebarTrigger, toggleSidebar } = useSidebarContext();
   const expanded = isMobile ? openMobile : open;
-  const label = expanded ? "Close sidebar" : "Open sidebar";
+  const label = isMobile
+    ? openMobile ? "关闭聊天记录" : "打开聊天记录"
+    : open ? "收起侧边栏" : "展开侧边栏";
   const register = (element: HTMLButtonElement | null) => {
     setRef(ref, element);
     if (placement === "inset") setInsetTrigger(element);
@@ -239,9 +247,9 @@ export const SidebarTrigger = forwardRef<HTMLButtonElement, SidebarTriggerProps>
   return <button ref={register} type="button" data-sidebar="trigger" data-slot="sidebar-trigger" aria-label={label} aria-expanded={expanded} className={cn("min-h-11 min-w-11", className)} onClick={(event) => { onClick?.(event); if (!event.defaultPrevented) toggleSidebar(); }} {...props} />;
 });
 
-export function SidebarRail({ className, ...props }: ComponentProps<"button">) {
+export function SidebarRail({ className, onClick, ...props }: ComponentProps<"button">) {
   const { isMobile, open, setOpen } = useSidebar();
   if (isMobile) return null;
   const label = open ? "Collapse sidebar" : "Expand sidebar";
-  return <button type="button" data-sidebar="rail" data-slot="sidebar-rail" aria-label={label} className={cn("min-h-11 min-w-11", className)} onClick={() => setOpen(!open)} {...props} />;
+  return <button type="button" data-sidebar="rail" data-slot="sidebar-rail" aria-label={label} className={cn("min-h-11 min-w-11", className)} onClick={(event) => { onClick?.(event); if (!event.defaultPrevented) setOpen(!open); }} {...props} />;
 }
diff --git a/frontend/src/hooks/use-sidebar-viewport.ts b/frontend/src/hooks/use-sidebar-viewport.ts
index 572843e9..be57fc27 100644
--- a/frontend/src/hooks/use-sidebar-viewport.ts
+++ b/frontend/src/hooks/use-sidebar-viewport.ts
@@ -19,7 +19,8 @@ export function useSidebarViewport(): SidebarViewportState {
 
   useEffect(() => {
     const updateViewport = () => {
-      setState({ viewport: sidebarViewportForWidth(window.innerWidth), ready: true });
+      const viewport = sidebarViewportForWidth(window.innerWidth);
+      setState((previous) => previous.ready && previous.viewport === viewport ? previous : { viewport, ready: true });
     };
 
     updateViewport();
diff --git a/frontend/tests/sidebar-contract.test.ts b/frontend/tests/sidebar-contract.test.ts
index 2dfc565e..7264d5a1 100644
--- a/frontend/tests/sidebar-contract.test.ts
+++ b/frontend/tests/sidebar-contract.test.ts
@@ -32,14 +32,41 @@ test("keeps sidebar behavior in the generic primitive", () => {
   assert.match(sidebar, /data-mobile-open/);
   assert.match(sidebar, /addEventListener\("keydown"/);
   assert.match(sidebar, /preventDefault\(\)/);
-  assert.match(sidebar, /const label = expanded \? "Close sidebar" : "Open sidebar"/);
-  assert.match(sidebar, /viewport === "mobile"\) return;[\s\S]*setOpenMobile\(false\)/);
+  assert.match(sidebar, /viewport === "mobile" \|\| !openMobile\) return;[\s\S]*setOpenMobile\(false\)/);
   assert.match(sidebar, /@base-ui\/react\/tooltip/);
   assert.doesNotMatch(sidebar, /Sheet/);
   assert.match(sidebar, /cn\(/);
   assert.doesNotMatch(sidebar, /#[0-9a-fA-F]{3,8}|hsl\(/);
 });
 
+test("provides the mobile drawer closing surface", () => {
+  const sidebar = readProjectFile("src/components/ui/sidebar.tsx");
+  assert.match(sidebar, /data-sidebar="scrim"/);
+  assert.match(sidebar, /data-slot="sidebar-scrim"/);
+  assert.match(sidebar, /aria-label="关闭聊天记录"/);
+  assert.match(sidebar, /onClick=\{\(\) => setOpenMobile\(false\)\}/);
+});
+
+test("uses the approved localized trigger actions", () => {
+  const sidebar = readProjectFile("src/components/ui/sidebar.tsx");
+  assert.match(sidebar, /"收起侧边栏"/);
+  assert.match(sidebar, /"展开侧边栏"/);
+  assert.match(sidebar, /"打开聊天记录"/);
+  assert.match(sidebar, /"关闭聊天记录"/);
+});
+
+test("keeps provider primitive defaults and consumer handlers composable", () => {
+  const sidebar = readProjectFile("src/components/ui/sidebar.tsx");
+  assert.match(sidebar, /id=\{id \?\? "chat-sidebar"\}/);
+  assert.match(sidebar, /if \(viewport === "mobile" \|\| !openMobile\) return;/);
+  assert.match(sidebar, /onClick\?\.\(event\);\s*if \(!event\.defaultPrevented\) setOpen\(!open\);/);
+});
+
+test("retains viewport state within an unchanged breakpoint", () => {
+  const viewportHook = readProjectFile("src/hooks/use-sidebar-viewport.ts");
+  assert.match(viewportHook, /previous\.ready && previous\.viewport === viewport \? previous : \{ viewport, ready: true \}/);
+});
+
 test("documents the sidebar shell design contract", () => {
   const design = readProjectFile("DESIGN.md");
   assert.match(design, /### Sidebar shell/);