diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx index 53a9ae48784..75836630a3b 100644 --- a/apps/web/src/components/AppSidebarLayout.tsx +++ b/apps/web/src/components/AppSidebarLayout.tsx @@ -21,7 +21,6 @@ import { useSidebarStageBackdropVariant } from "./SidebarStageBackdrop"; import { resolveInitialThreadSidebarWidth, resolveThreadSidebarMaximumWidth, - THREAD_MAIN_CONTENT_MIN_WIDTH, THREAD_SIDEBAR_MIN_WIDTH, THREAD_SIDEBAR_WIDTH_STORAGE_KEY, } from "./threadSidebarWidth"; @@ -190,9 +189,11 @@ export function AppSidebarLayout({ children }: { children: ReactNode }) { resizable={{ maxWidth: sidebarMaximumWidth, minWidth: THREAD_SIDEBAR_MIN_WIDTH, + // Same rule as the clamp above, measured against the live wrapper so + // the two never disagree and refuse a width the clamp just allowed. shouldAcceptWidth: ({ currentWidth, nextWidth, wrapper }) => nextWidth <= currentWidth || - wrapper.clientWidth - nextWidth >= THREAD_MAIN_CONTENT_MIN_WIDTH, + nextWidth <= resolveThreadSidebarMaximumWidth(wrapper.clientWidth), storageKey: THREAD_SIDEBAR_WIDTH_STORAGE_KEY, onResize: setSidebarWidth, }} diff --git a/apps/web/src/components/threadSidebarWidth.test.ts b/apps/web/src/components/threadSidebarWidth.test.ts index 12aef63bd5e..0cc7857b6a2 100644 --- a/apps/web/src/components/threadSidebarWidth.test.ts +++ b/apps/web/src/components/threadSidebarWidth.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vite-plus/test"; import { resolveInitialThreadSidebarWidth, + resolveThreadSidebarMaximumWidth, THREAD_MAIN_CONTENT_MIN_WIDTH, THREAD_SIDEBAR_DEFAULT_WIDTH, THREAD_SIDEBAR_MIN_WIDTH, @@ -20,15 +21,36 @@ describe("thread sidebar width", () => { expect(resolveInitialThreadSidebarWidth(120, 1200)).toBe(THREAD_SIDEBAR_MIN_WIDTH); }); - it("leaves enough room for the main content on a smaller window", () => { - const viewportWidth = 1000; + it("leaves enough room for the main content on a wide window", () => { + const viewportWidth = 1600; - expect(resolveInitialThreadSidebarWidth(900, viewportWidth)).toBe( + expect(resolveInitialThreadSidebarWidth(1500, viewportWidth)).toBe( viewportWidth - THREAD_MAIN_CONTENT_MIN_WIDTH, ); }); + it("still gives the sidebar room to grow on a default-sized desktop window", () => { + // A flat 40rem reservation would cap this at 276px, which is where the + // sidebar already sits by default — the rail would have nowhere to go. + expect(resolveThreadSidebarMaximumWidth(916)).toBe(458); + }); + + it("never lets the sidebar take more than half of a narrow window", () => { + const viewportWidth = 1000; + + expect(resolveInitialThreadSidebarWidth(900, viewportWidth)).toBe(viewportWidth / 2); + }); + + it("gives the odd pixel of a narrow window to the main content", () => { + expect(resolveThreadSidebarMaximumWidth(917)).toBe(458); + }); + + it("switches over to the flat main content reservation at 80rem", () => { + expect(resolveThreadSidebarMaximumWidth(1280)).toBe(1280 - THREAD_MAIN_CONTENT_MIN_WIDTH); + expect(resolveThreadSidebarMaximumWidth(1279)).toBe(639); + }); + it("keeps the sidebar minimum when the whole layout is narrower than its minimums", () => { - expect(resolveInitialThreadSidebarWidth(900, 700)).toBe(THREAD_SIDEBAR_MIN_WIDTH); + expect(resolveInitialThreadSidebarWidth(900, 300)).toBe(THREAD_SIDEBAR_MIN_WIDTH); }); }); diff --git a/apps/web/src/components/threadSidebarWidth.ts b/apps/web/src/components/threadSidebarWidth.ts index 93dd196e84d..9ac92ba270e 100644 --- a/apps/web/src/components/threadSidebarWidth.ts +++ b/apps/web/src/components/threadSidebarWidth.ts @@ -4,10 +4,18 @@ export const THREAD_SIDEBAR_MIN_WIDTH = 13 * 16; export const THREAD_MAIN_CONTENT_MIN_WIDTH = 40 * 16; export function resolveThreadSidebarMaximumWidth(viewportWidth: number): number { - return Math.max( - THREAD_SIDEBAR_MIN_WIDTH, - Math.floor(viewportWidth) - THREAD_MAIN_CONTENT_MIN_WIDTH, + // Reserving a flat 40rem for the main content leaves almost no travel on a + // default-sized desktop window (~916px inner width caps the sidebar at 276px, + // where it usually already sits), so the rail feels dead. Below ~80rem the + // reservation scales down to half the window instead: the main content still + // keeps at least half, and the sidebar always has room to grow. + // Rounding up hands the odd pixel to the main content, so the sidebar stays + // at or below half the window rather than one pixel past it. + const reservedMainContentWidth = Math.min( + THREAD_MAIN_CONTENT_MIN_WIDTH, + Math.ceil(Math.floor(viewportWidth) / 2), ); + return Math.max(THREAD_SIDEBAR_MIN_WIDTH, Math.floor(viewportWidth) - reservedMainContentWidth); } export function resolveInitialThreadSidebarWidth( diff --git a/apps/web/src/components/ui/sidebar.test.tsx b/apps/web/src/components/ui/sidebar.test.tsx index 935c4d1562e..353c50ddca0 100644 --- a/apps/web/src/components/ui/sidebar.test.tsx +++ b/apps/web/src/components/ui/sidebar.test.tsx @@ -3,6 +3,7 @@ import { describe, expect, it } from "vite-plus/test"; import { applyPendingSidebarResize, + parseSidebarPixelWidth, Sidebar, SidebarMenuAction, SidebarMenuButton, @@ -21,6 +22,14 @@ function renderSidebarButton(className?: string) { ); } +// The options a drag captured at pointer-down. applyPendingSidebarResize takes +// the options to apply explicitly, so these only need to satisfy the type. +const resizeOptions = { + maxWidth: 600, + minWidth: 208, + storageKey: null, +} as const; + describe("sidebar interactive cursors", () => { it("commits the latest pending width before a queued animation frame can run", () => { const appliedWidths: string[] = []; @@ -35,6 +44,7 @@ describe("sidebar interactive cursors", () => { } as unknown as HTMLElement; const resizeState = { moved: true, + options: resizeOptions, pointerId: 1, pendingWidth: 320, rail: {} as HTMLButtonElement, @@ -69,6 +79,7 @@ describe("sidebar interactive cursors", () => { } as unknown as HTMLElement; const resizeState = { moved: true, + options: resizeOptions, pointerId: 1, pendingWidth: 720, rail: {} as HTMLButtonElement, @@ -171,3 +182,18 @@ describe("sidebar interactive cursors", () => { expect(html).toContain("cursor-pointer"); }); }); + +describe("sidebar applied width parsing", () => { + it("reads the pixel width the resize path writes", () => { + expect(parseSidebarPixelWidth("458px")).toBe(458); + expect(parseSidebarPixelWidth(" 947.5703125px ")).toBe(947.5703125); + }); + + it("refuses units the resize path never writes, rather than misreading them", () => { + // "16rem" is the provider default; Number.parseFloat would read it as 16px + // and collapse the sidebar to its minimum on the next reconcile. + expect(parseSidebarPixelWidth("16rem")).toBeNull(); + expect(parseSidebarPixelWidth("calc(100vw - 12px)")).toBeNull(); + expect(parseSidebarPixelWidth("")).toBeNull(); + }); +}); diff --git a/apps/web/src/components/ui/sidebar.tsx b/apps/web/src/components/ui/sidebar.tsx index 33c28404443..254e39a9796 100644 --- a/apps/web/src/components/ui/sidebar.tsx +++ b/apps/web/src/components/ui/sidebar.tsx @@ -39,6 +39,14 @@ type SidebarContextProps = { toggleSidebar: () => void; }; +/** + * `minWidth`/`maxWidth` are enforced while dragging, when a persisted width is + * restored, and whenever the limits themselves change. That last reconciliation + * only applies to a pixel-valued `--sidebar-width`, which is what the resize + * path writes; a sidebar left on the provider's `rem` default stays there until + * its first drag. Pass an initial pixel width via `onResize` if the limits must + * hold before then. + */ type SidebarResizableOptions = { maxWidth?: number; minWidth?: number; @@ -71,6 +79,10 @@ type SidebarResolvedResizableOptions = { type SidebarResizeState = { moved: boolean; + // Captured at pointer-down: a drag settles against the configuration that + // started it, so options changing mid-drag cannot redirect where the result + // is persisted or which callback hears about it. + options: SidebarResolvedResizableOptions; pointerId: number; pendingWidth: number; rail: HTMLButtonElement; @@ -362,6 +374,19 @@ function clampSidebarWidth(width: number, options: SidebarResolvedResizableOptio return Math.max(options.minWidth, Math.min(width, options.maxWidth)); } +// The resize path only ever writes pixels. Anything else (the "16rem" default, a +// calc(), a caller managing the variable itself) is not ours to reconcile, and +// parsing it as a number would read "16rem" as 16 and collapse the sidebar. +function parseSidebarPixelWidth(value: string): number | null { + const match = /^\s*(\d+(?:\.\d+)?)px\s*$/.exec(value); + if (!match?.[1]) { + return null; + } + + const width = Number.parseFloat(match[1]); + return Number.isFinite(width) ? width : null; +} + function applyPendingSidebarResize( resizeState: SidebarResizeState, options: SidebarResolvedResizableOptions, @@ -399,40 +424,48 @@ function SidebarRail({ const railRef = React.useRef(null); const suppressClickRef = React.useRef(false); const resizeStateRef = React.useRef(null); + const hydratedStorageKeyRef = React.useRef(null); + const observedStorageKeyRef = React.useRef(null); + // The width the user actually asked for, kept unclamped so a window that + // grows again can restore it rather than staying at a narrower window's cap. + // Tagged with the key that owns it: a sidebar that switches storage keys must + // not inherit the previous key's preference, least of all when the new key has + // nothing stored and would otherwise silently keep the old width. + const preferredWidthRef = React.useRef<{ storageKey: string | null; width: number } | null>(null); const resolvedResizable = sidebarInstance?.resizable ?? null; const canResize = resolvedResizable !== null && open; const railLabel = canResize ? "Resize Sidebar" : "Toggle Sidebar"; const railTitle = canResize ? "Drag to resize sidebar" : "Toggle Sidebar"; - const stopResize = React.useCallback( - (pointerId: number) => { - const resizeState = resizeStateRef.current; - if (!resizeState) { - return; - } - if (resizeState.rafId !== null) { - window.cancelAnimationFrame(resizeState.rafId); - resizeState.rafId = null; - } - if (resolvedResizable) { - applyPendingSidebarResize(resizeState, resolvedResizable); - } - resizeState.transitionTargets.forEach((element) => { - element.style.removeProperty("transition-duration"); - }); - if (resolvedResizable?.storageKey && typeof window !== "undefined") { - setLocalStorageItem(resolvedResizable.storageKey, resizeState.width, Schema.Finite); - } - resolvedResizable?.onResize?.(resizeState.width); - resizeStateRef.current = null; - if (resizeState.rail.hasPointerCapture(pointerId)) { - resizeState.rail.releasePointerCapture(pointerId); - } - document.body.style.removeProperty("cursor"); - document.body.style.removeProperty("user-select"); - }, - [resolvedResizable], - ); + const stopResize = React.useCallback((pointerId: number) => { + const resizeState = resizeStateRef.current; + if (!resizeState) { + return; + } + if (resizeState.rafId !== null) { + window.cancelAnimationFrame(resizeState.rafId); + resizeState.rafId = null; + } + const options = resizeState.options; + applyPendingSidebarResize(resizeState, options); + resizeState.transitionTargets.forEach((element) => { + element.style.removeProperty("transition-duration"); + }); + if (options.storageKey && typeof window !== "undefined") { + setLocalStorageItem(options.storageKey, resizeState.width, Schema.Finite); + } + preferredWidthRef.current = { + storageKey: options.storageKey, + width: resizeState.width, + }; + options.onResize?.(resizeState.width); + resizeStateRef.current = null; + if (resizeState.rail.hasPointerCapture(pointerId)) { + resizeState.rail.releasePointerCapture(pointerId); + } + document.body.style.removeProperty("cursor"); + document.body.style.removeProperty("user-select"); + }, []); const handlePointerDown = React.useCallback( (event: React.PointerEvent) => { @@ -467,6 +500,7 @@ function SidebarRail({ event.stopPropagation(); resizeStateRef.current = { moved: false, + options: resolvedResizable, pointerId: event.pointerId, pendingWidth: initialWidth, rail: event.currentTarget, @@ -515,7 +549,7 @@ function SidebarRail({ if (!activeResizeState || !resolvedResizable) return; activeResizeState.rafId = null; - applyPendingSidebarResize(activeResizeState, resolvedResizable); + applyPendingSidebarResize(activeResizeState, activeResizeState.options); }); }, [onPointerMove, resolvedResizable], @@ -570,7 +604,23 @@ function SidebarRail({ ); React.useLayoutEffect(() => { - if (!resolvedResizable?.storageKey || typeof window === "undefined") return; + const storageKey = resolvedResizable?.storageKey ?? null; + // Tracked separately from the hydrated key so that leaving a key and coming + // back to it counts as a fresh restore. Recording only successful hydrations + // would let a key that was never re-observed look permanently settled. + if (observedStorageKeyRef.current !== storageKey) { + observedStorageKeyRef.current = storageKey; + hydratedStorageKeyRef.current = null; + } + if (!resolvedResizable || !storageKey || typeof window === "undefined") return; + // Callers pass their resizable options as an object literal, so this effect + // re-runs on every parent render. Restoring is a mount concern: repeating it + // would overwrite a live drag (which only writes storage once it settles) + // with the pre-drag width every time an unrelated render lands. + if (hydratedStorageKeyRef.current === storageKey) return; + // A drag owns the width until it settles, and it settles against the options + // it began with — so restoring waits rather than racing it. + if (resizeStateRef.current) return; const rail = railRef.current; if (!rail) return; const wrapper = rail.closest("[data-slot='sidebar-wrapper']"); @@ -578,12 +628,14 @@ function SidebarRail({ let storedWidth: number | null; try { - storedWidth = getLocalStorageItem(resolvedResizable.storageKey, Schema.Finite); + storedWidth = getLocalStorageItem(storageKey, Schema.Finite); } catch (error) { console.error("Could not restore persisted sidebar width.", error); return; } + hydratedStorageKeyRef.current = storageKey; if (storedWidth === null) return; + preferredWidthRef.current = { storageKey, width: storedWidth }; const clampedWidth = clampSidebarWidth(storedWidth, resolvedResizable); // Hydrate the CSS variable before the browser paints so a restored sidebar // never flashes at the default width first. @@ -591,6 +643,63 @@ function SidebarRail({ resolvedResizable.onResize?.(clampedWidth); }, [resolvedResizable]); + // Hydration above runs once, so the applied width would otherwise ignore later + // changes to the limits: shrinking the window used to leave an oversized + // sidebar squeezing the main content until the next drag. Reconciling is + // separate from restoring, and idempotent, so an unrelated render is a no-op. + React.useLayoutEffect(() => { + if (!resolvedResizable || typeof window === "undefined") return; + // Never fight a drag in progress — the pointer owns the width until it settles. + if (resizeStateRef.current) return; + const rail = railRef.current; + if (!rail) return; + const wrapper = rail.closest("[data-slot='sidebar-wrapper']"); + if (!wrapper) return; + + // Only widths this component owns are reconcilable: the resize path writes + // px, so anything else (the provider's "16rem" default, a caller managing + // the variable itself) is left alone rather than reinterpreted. + const appliedWidth = parseSidebarPixelWidth( + window.getComputedStyle(wrapper).getPropertyValue("--sidebar-width"), + ); + if (appliedWidth === null) return; + + const preference = preferredWidthRef.current; + const preferredWidth = + preference && preference.storageKey === resolvedResizable.storageKey + ? preference.width + : null; + const clampedWidth = clampSidebarWidth(preferredWidth ?? appliedWidth, resolvedResizable); + if (clampedWidth === appliedWidth) return; + + // Bringing an out-of-bounds width back inside the limits is mandatory, so it + // skips the predicate: a caller that rejects shrinking would otherwise pin + // the sidebar above a maxWidth that just dropped. The predicate only governs + // optional movement between two valid widths, such as growing back toward + // the stored preference, where the caller may know a constraint we don't. + const appliedWithinLimits = + appliedWidth >= resolvedResizable.minWidth && appliedWidth <= resolvedResizable.maxWidth; + if (appliedWithinLimits) { + const sidebarRoot = rail.closest("[data-slot='sidebar']"); + if (!sidebarRoot) return; + const accepted = + resolvedResizable.shouldAcceptWidth?.({ + currentWidth: appliedWidth, + nextWidth: clampedWidth, + rail, + side: sidebarInstance?.side ?? "left", + sidebarRoot, + wrapper, + }) ?? true; + if (!accepted) return; + } + + wrapper.style.setProperty("--sidebar-width", `${clampedWidth}px`); + // Deliberately not persisted: storage holds the width the user asked for, so + // a window that grows again restores it instead of keeping the smaller cap. + resolvedResizable.onResize?.(clampedWidth); + }); + React.useEffect(() => { return () => { const resizeState = resizeStateRef.current; @@ -1016,6 +1125,7 @@ function SidebarMenuSubButton({ export { Sidebar, applyPendingSidebarResize, + parseSidebarPixelWidth, SidebarContent, SidebarFooter, SidebarGroup,