From bb8df17ecb17401eee79bcc6bb9e7966592531e9 Mon Sep 17 00:00:00 2001 From: ChenRuoT Date: Mon, 17 Aug 2026 23:24:29 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E9=9D=A2=E6=9D=BF=E6=8B=96=E5=8A=A8?= =?UTF-8?q?=E5=B8=83=E5=B1=80=E9=97=AA=E7=83=81=20=E2=80=94=20=E5=8F=B3?= =?UTF-8?q?=E4=BE=A7=E6=A0=8F=E5=85=B3=E9=97=AD=E6=97=B6=E5=BA=95=E9=83=A8?= =?UTF-8?q?=E9=AB=98=E5=BA=A6=E6=8B=96=E5=8A=A8=E6=8C=A4=E5=8E=8B=20AppFra?= =?UTF-8?q?me=20+=20=E9=87=8A=E6=94=BE=E7=9E=AC=E9=97=B4=E6=95=B4=E4=BD=93?= =?UTF-8?q?=E9=93=BA=E6=BB=A1=E5=86=8D=E5=9B=9E=E5=BC=B9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - applyDrag 仅在右侧面板打开时写入 --dsh-sidebar-width（底部高度拖动传入的是右侧栏持久化宽度，关闭时会把整个 AppFrame 左挤，直到松手才恢复） - 布局 push effect 拆分为「仅设置」+「仅卸载移除」：React 同一提交内先执行全部 cleanup 再执行全部 setup，原 cleanup 删除 CSS 变量后，draggingRef effect 的 measureCenter 强制重算把 0px 缓存为 margin-right 过渡起点，释放时过渡重新启用 → 0→W 动画（先铺满页面再回弹） - 回归测试：size change 提交过程中不得 removeProperty 布局变量（仅卸载时移除，issue #31 语义保留） --- src/client/Sidebar.tsx | 38 +++++++++++++++++++++++++++--------- tests/sidebar-crash.spec.tsx | 32 +++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/src/client/Sidebar.tsx b/src/client/Sidebar.tsx index 8d4cafc..2cafd83 100644 --- a/src/client/Sidebar.tsx +++ b/src/client/Sidebar.tsx @@ -472,7 +472,13 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // `width + detailsWidth` — derived from the measured column, keeping the // drag write-only (no React re-render mid-drag). bottomRef.current?.style.setProperty('right', `${(window.innerWidth - centerRect.right) + (width - (state?.width ?? 0))}px`) - document.documentElement.style.setProperty('--dsh-sidebar-width', `${width}px`) + // The width push exists only while the right panel is OPEN (the layout + // push effect writes 0 while it is collapsed). The bottom-height drag + // reuses this function with the right panel's PERSISTED width as `width`, + // so writing it unconditionally would squeeze the whole app shell left + // mid-drag — as if the collapsed right panel were still occupying space — + // until the store commit on release snaps it back. + document.documentElement.style.setProperty('--dsh-sidebar-width', `${state?.panelOpen === true ? width : 0}px`) document.documentElement.style.setProperty('--dsh-sidebar-height', `${height}px`) if (cornerRef.current !== null) { cornerRef.current.style.left = `${window.innerWidth - width - 6}px` @@ -516,6 +522,18 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { // crush the app shell to zero. Dragging disables the layout transition. // On NARROW viewports the drawer FLOATS over the app shell — no push, the // conversation keeps the full width behind the drawer. + // + // This effect is deliberately SET-only (no cleanup): React runs every + // effect cleanup before every effect setup in the same commit, so a + // cleanup here would remove both variables before the draggingRef effect + // (declared above) runs its setup and forces a layout in measureCenter + // (getBoundingClientRect). That forced recalc would resolve #root's + // margin-right to the 0px fallback, cache it as the transition start + // value, and measure centerRect far too wide; once the body attribute + // effect re-enables transitions on pointer-up, the shell would animate + // margin-right 0 → width — the visible "expand to the full page then + // bounce back" flash. Unmount release lives in the effect below + // (issue #31). useEffect(() => { const width = !narrow && snapshot.state?.panelOpen === true ? Math.min(snapshot.state.width, window.innerWidth) @@ -525,18 +543,20 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) { : 0 document.documentElement.style.setProperty('--dsh-sidebar-width', `${width}px`) document.documentElement.style.setProperty('--dsh-sidebar-height', `${height}px`) - // Unmount must release the push (issue #31): when the boundary swaps the - // whole sidebar after a render crash (or the plugin fiber is disposed / - // HMR), the CSS variables would otherwise stay on and layout.css - // keeps squeezing #root with a stale margin — "the sidebar cannot be - // hidden" until a full reload. removeProperty restores the CSS fallback - // (var(--dsh-sidebar-width, 0px)); React re-runs cleanup+setup in the - // same commit on state changes, so there is no visible flicker. + }, [narrow, snapshot.state?.panelOpen, snapshot.state?.width, snapshot.state?.bottomOpen, snapshot.state?.bottomHeight]) + // Unmount must release the push (issue #31): when the boundary swaps the + // whole sidebar after a render crash (or the plugin fiber is disposed / + // HMR), the CSS variables would otherwise stay on and layout.css + // keeps squeezing #root with a stale margin — "the sidebar cannot be + // hidden" until a full reload. removeProperty restores the CSS fallback + // (var(--dsh-sidebar-width, 0px)). Kept separate from the set effect so a + // dep-change re-run never removes the variables mid-commit. + useEffect(() => { return () => { document.documentElement.style.removeProperty('--dsh-sidebar-width') document.documentElement.style.removeProperty('--dsh-sidebar-height') } - }, [narrow, snapshot.state?.panelOpen, snapshot.state?.width, snapshot.state?.bottomOpen, snapshot.state?.bottomHeight]) + }, []) useEffect(() => { if (anyDragging) document.body.setAttribute('data-dsh-sidebar-dragging', '') else document.body.removeAttribute('data-dsh-sidebar-dragging') diff --git a/tests/sidebar-crash.spec.tsx b/tests/sidebar-crash.spec.tsx index a6c93f4..7adb552 100644 --- a/tests/sidebar-crash.spec.tsx +++ b/tests/sidebar-crash.spec.tsx @@ -26,7 +26,7 @@ import { act } from 'react-dom/test-utils' ;(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true import { Sidebar } from '../src/client/Sidebar.tsx' -import { createSidebarStore, type SidebarStore } from '../src/client/state.ts' +import { createSidebarStore, setBottomHeight, toggleBottomPanel, type SidebarStore } from '../src/client/state.ts' import { createBetterSidebarService, type BetterSidebarService } from '../src/client/service.ts' import { t } from '../src/client/locales.ts' @@ -99,6 +99,36 @@ describe('layout-push variable cleanup', () => { expect(htmlStyle.getPropertyValue('--dsh-sidebar-width')).toBe('') expect(htmlStyle.getPropertyValue('--dsh-sidebar-height')).toBe('') }) + + it('a size change (release commit) re-applies the variables without removing them mid-commit', () => { + const { store, unmount } = mountSidebar() + const htmlStyle = document.documentElement.style + const width = store.getSnapshot().state!.width + const removeSpy = vi.spyOn(htmlStyle, 'removeProperty') + // Simulate a drag release: the bottom panel opens and its height commits. + act(() => { store.reduce(toggleBottomPanel) }) + act(() => { store.reduce(s => setBottomHeight(s, 300)) }) + expect(htmlStyle.getPropertyValue('--dsh-sidebar-width')).toBe(`${width}px`) + expect(htmlStyle.getPropertyValue('--dsh-sidebar-height')).toBe('300px') + // The commit must NOT have removed the variables at any point. React + // runs every effect cleanup before every effect setup in a commit, so a + // cleanup here would remove the variables before the draggingRef effect + // (declared above the layout push) forces a layout in measureCenter + // (getBoundingClientRect). That forced recalc resolves #root's + // margin-right to the 0px fallback, caches it as the transition start + // value, and measures centerRect too wide; once transitions re-enable on + // release, the shell animates margin-right 0 → width — the "expand to + // the full page then bounce back" flash. + const removalProps = removeSpy.mock.calls + .map(call => call[0] as string) + .filter(prop => prop === '--dsh-sidebar-width' || prop === '--dsh-sidebar-height') + expect(removalProps).toHaveLength(0) + // Only unmounting may remove them (issue #31). + unmount() + expect(removeSpy.mock.calls.some(call => + call[0] === '--dsh-sidebar-width' || call[0] === '--dsh-sidebar-height')).toBe(true) + removeSpy.mockRestore() + }) }) describe('tab crash containment', () => {