From ab3ef4cac002baae35ab7ea527656f31b95d6b31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Mar 2026 23:33:28 +0000 Subject: [PATCH 1/3] Initial plan From a00c4e39a4f0bda44dec1750018bb20e7aa08871 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Mar 2026 23:50:06 +0000 Subject: [PATCH 2/3] Fix mobile lightbox freeze after opening from history drawer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user opened the history drawer on mobile, tapped an image/video to open it in the lightbox, then closed the lightbox, the entire page became unresponsive (history icon, controls, sidebar – nothing worked). Root cause: openLightbox() batched setShowGallery(false) and setIsFullscreen(true) into the same React render, so the vaul drawer's 200ms close animation overlapped with the Radix Dialog opening. Two concurrent dismissable-layer / react-remove-scroll instances would race on cleanup and leave pointer-events:none or aria-hidden/inert stuck on body-level DOM nodes. Fix in use-studio-ui.ts: - openLightbox() now reads the current drawer state via a ref (no dep array churn) and, when a drawer is open on mobile, closes it then waits 300ms (200ms animation + 100ms margin) before opening the Dialog. This makes the sequence strictly sequential – only one modal is ever active at a time. - closeLightbox() replaces the single requestAnimationFrame cleanup with a 300ms setTimeout so it runs after Radix's full close-animation teardown (75ms animation + aria-hidden/remove-scroll cleanup). It also removes stuck aria-hidden and inert attributes from direct body children in a single combined querySelectorAll pass, guarded by a check that no other modal is open. - Both timer refs are properly cancelled on component unmount. Tests: added 5 regression tests using vi.useFakeTimers() that validate the new timing behaviour. Co-authored-by: CanyoufeeltheAGI <255605710+CanyoufeeltheAGI@users.noreply.github.com> --- hooks/use-studio-ui.test.ts | 151 +++++++++++++++++++++++++++++++++++- hooks/use-studio-ui.ts | 125 ++++++++++++++++++++++++----- package.json | 1 + 3 files changed, 256 insertions(+), 21 deletions(-) diff --git a/hooks/use-studio-ui.test.ts b/hooks/use-studio-ui.test.ts index 21601d2..126c21c 100644 --- a/hooks/use-studio-ui.test.ts +++ b/hooks/use-studio-ui.test.ts @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { renderHook, act, waitFor } from "@testing-library/react"; import { useStudioUI } from "./use-studio-ui"; import { createMockImage } from "@/lib/test-utils"; @@ -285,6 +285,155 @@ describe("useStudioUI", () => { }); }); + describe("Mobile lightbox – drawer close sequencing (bug fix)", () => { + beforeEach(() => { + mockUseIsMobile.mockReturnValue(true); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + /** + * Regression test: opening lightbox while the history drawer is open must + * close the drawer first and only open the lightbox after the drawer's + * close animation completes (300ms). Previously both state updates were + * batched in the same render, leaving two concurrent dismissable-layer + * instances that corrupted body pointer-events / aria-hidden state. + */ + it("delays lightbox open until after drawer close animation when gallery is open", () => { + const { result } = renderHook(() => useStudioUI()); + + // Simulate user opening the history drawer + act(() => { + result.current.setShowGallery(true); + }); + expect(result.current.showGallery).toBe(true); + + // User clicks an image inside the history drawer + act(() => { + result.current.openLightbox(mockImage); + }); + + // Drawer must close immediately + expect(result.current.showGallery).toBe(false); + + // Lightbox must NOT be open yet (waiting for animation to complete) + expect(result.current.isFullscreen).toBe(false); + expect(result.current.lightboxImage).toBeNull(); + + // Advance past the 300ms delay + act(() => { + vi.advanceTimersByTime(300); + }); + + // Now the lightbox should be open + expect(result.current.isFullscreen).toBe(true); + expect(result.current.lightboxImage).toEqual(mockImage); + }); + + it("delays lightbox open until after drawer close animation when sidebar is open", () => { + const { result } = renderHook(() => useStudioUI()); + + // Simulate user opening the sidebar + act(() => { + result.current.setShowLeftSidebar(true); + }); + expect(result.current.showLeftSidebar).toBe(true); + + act(() => { + result.current.openLightbox(mockImage); + }); + + // Sidebar must close immediately + expect(result.current.showLeftSidebar).toBe(false); + // Lightbox not yet open + expect(result.current.isFullscreen).toBe(false); + + act(() => { + vi.advanceTimersByTime(300); + }); + + expect(result.current.isFullscreen).toBe(true); + expect(result.current.lightboxImage).toEqual(mockImage); + }); + + it("opens lightbox immediately on mobile when no drawer is open", () => { + const { result } = renderHook(() => useStudioUI()); + + // Both drawers closed (default on mobile after init effect) + expect(result.current.showGallery).toBe(false); + expect(result.current.showLeftSidebar).toBe(false); + + act(() => { + result.current.openLightbox(mockImage); + }); + + // No delay needed – opens immediately + expect(result.current.isFullscreen).toBe(true); + expect(result.current.lightboxImage).toEqual(mockImage); + }); + + it("cancels pending lightbox open when closeLightbox is called before the timer fires", () => { + const { result } = renderHook(() => useStudioUI()); + + act(() => { + result.current.setShowGallery(true); + }); + + // Queue a delayed open + act(() => { + result.current.openLightbox(mockImage); + }); + expect(result.current.isFullscreen).toBe(false); + + // Close (e.g., user presses Escape) before the timer fires + act(() => { + result.current.closeLightbox(); + }); + + // Advance past the delay + act(() => { + vi.advanceTimersByTime(300); + }); + + // The lightbox should NOT have opened + expect(result.current.isFullscreen).toBe(false); + }); + + it("closeLightbox resets stuck body pointer-events after 300ms", () => { + const { result } = renderHook(() => useStudioUI()); + + // Simulate the body getting stuck with pointer-events:none + document.body.style.pointerEvents = "none"; + + act(() => { + result.current.openLightbox(mockImage); + vi.advanceTimersByTime(300); + }); + + act(() => { + result.current.closeLightbox(); + }); + + // Before 300ms the body is still stuck + act(() => { + vi.advanceTimersByTime(150); + }); + expect(document.body.style.pointerEvents).toBe("none"); + + // After 300ms the cleanup has run + act(() => { + vi.advanceTimersByTime(150); + }); + expect(document.body.style.pointerEvents).toBe(""); + + // Restore + document.body.style.pointerEvents = ""; + }); + }); + describe("Callback stability", () => { it("toggle functions have stable references", () => { const { result, rerender } = renderHook(() => useStudioUI()); diff --git a/hooks/use-studio-ui.ts b/hooks/use-studio-ui.ts index 1103b27..f3effa8 100644 --- a/hooks/use-studio-ui.ts +++ b/hooks/use-studio-ui.ts @@ -115,39 +115,124 @@ export function useStudioUI(): UseStudioUIReturn { // ======================================== // Lightbox Handlers // - // IMPORTANT: On mobile, vaul drawers and the Radix Dialog lightbox use - // separate instances of @radix-ui/react-dismissable-layer (v1.1.3 in vaul - // vs v1.1.11 at top-level). Each instance independently manages - // body.style.pointerEvents. When both are open simultaneously, the close - // order can leave body stuck with pointer-events:none, making the entire - // page unresponsive. The fix: close drawers before opening the lightbox - // and defensively reset pointer-events on lightbox close. + // IMPORTANT: On mobile, vaul drawers and the Radix Dialog lightbox share + // the same @radix-ui/react-dismissable-layer but their close sequences can + // race. When both are active simultaneously (drawer animating out while + // dialog opens), the cleanup order leaves body.style.pointerEvents:"none" + // and/or aria-hidden/inert stuck on elements, making the entire page + // unresponsive. The fix: + // 1. Close drawers first, THEN open the lightbox after the drawer's + // close animation completes (300ms > 200ms animation duration). + // 2. After the lightbox closes, defensively reset body attributes once + // Radix's own close-animation cleanup has fully run (setTimeout > rAF). // ======================================== + + // Pending-lightbox refs: used to delay the open on mobile until drawer + // animation completes, keeping openLightbox's dep array stable. + const pendingLightboxTimerRef = React.useRef | null>(null); + const pendingLightboxRef = React.useRef<{ pending: boolean; image: LightboxImage | null }>({ + pending: false, + image: null, + }); + // Latest drawer state, read inside openLightbox without adding to deps + const drawerOpenRef = React.useRef({ showGallery, showLeftSidebar }); + drawerOpenRef.current = { showGallery, showLeftSidebar }; + + // Cancel any pending timer when the hook unmounts + React.useEffect(() => { + return () => { + if (pendingLightboxTimerRef.current !== null) { + clearTimeout(pendingLightboxTimerRef.current); + } + }; + }, []); + const openLightbox = React.useCallback( (image: LightboxImage | null) => { - // Close mobile drawers first to avoid the pointer-events race condition - // between vaul's and Radix Dialog's dismissable-layer instances - if (isMobile) { + const { showGallery: isGalleryOpen, showLeftSidebar: isSidebarOpen } = + drawerOpenRef.current; + const hasOpenDrawer = isMobile && (isGalleryOpen || isSidebarOpen); + + // Cancel any previous pending open + if (pendingLightboxTimerRef.current !== null) { + clearTimeout(pendingLightboxTimerRef.current); + pendingLightboxTimerRef.current = null; + } + + if (hasOpenDrawer) { + // Close drawers first, then wait for the drawer close animation to + // finish (200ms CSS transition + 100ms safety margin = 300ms) before + // opening the lightbox. This prevents concurrent dismissable-layer + // instances from corrupting body pointer-events / aria-hidden state. setShowLeftSidebar(false); setShowGallery(false); + + pendingLightboxRef.current = { pending: true, image }; + pendingLightboxTimerRef.current = setTimeout(() => { + pendingLightboxTimerRef.current = null; + if (pendingLightboxRef.current.pending) { + const { image: pendingImage } = pendingLightboxRef.current; + pendingLightboxRef.current = { pending: false, image: null }; + setLightboxImage(pendingImage); + setIsFullscreen(true); + } + }, 300); + } else { + setLightboxImage(image); + setIsFullscreen(true); } - setLightboxImage(image); - setIsFullscreen(true); }, [isMobile, setShowLeftSidebar, setShowGallery], ); + // Ref for the closeLightbox cleanup timer so it can be cancelled on unmount + const closeLightboxTimerRef = React.useRef | null>(null); + + React.useEffect(() => { + return () => { + if (closeLightboxTimerRef.current !== null) { + clearTimeout(closeLightboxTimerRef.current); + } + }; + }, []); + const closeLightbox = React.useCallback(() => { setIsFullscreen(false); - // Defensive cleanup: ensure body pointer-events are restored. - // The dual dismissable-layer instances can leave pointer-events:none - // on the body after the lightbox closes. We use rAF to run after - // Radix's own cleanup microtask. - requestAnimationFrame(() => { - if (document.body.style.pointerEvents === "none") { - document.body.style.pointerEvents = ""; + + // Cancel any pending lightbox open (in case the user closes while one is + // queued, e.g. via keyboard shortcut) + if (pendingLightboxTimerRef.current !== null) { + clearTimeout(pendingLightboxTimerRef.current); + pendingLightboxTimerRef.current = null; + pendingLightboxRef.current = { pending: false, image: null }; + } + + // Defensive cleanup: run AFTER Radix Dialog's close animation (75ms via + // !duration-75 class) and its aria-hidden / react-remove-scroll teardown. + // A single rAF (~16ms) is too early; use 300ms to be well clear of all + // async cleanup paths. Only resets attributes when no other modal is open. + if (closeLightboxTimerRef.current !== null) { + clearTimeout(closeLightboxTimerRef.current); + } + closeLightboxTimerRef.current = setTimeout(() => { + closeLightboxTimerRef.current = null; + const openModal = document.querySelector( + '[role="dialog"][data-state="open"], [role="alertdialog"][data-state="open"]', + ); + if (!openModal) { + if (document.body.style.pointerEvents === "none") { + document.body.style.pointerEvents = ""; + } + // Remove any stuck aria-hidden / inert from direct body children that + // Radix/vaul may have left behind when concurrent modals raced on close. + document + .querySelectorAll("body > [aria-hidden], body > [inert]") + .forEach((el) => { + el.removeAttribute("aria-hidden"); + el.removeAttribute("inert"); + }); } - }); + }, 300); }, []); // ======================================== diff --git a/package.json b/package.json index 2251ac5..6d29998 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "@stripe/stripe-js": "^8.8.0", "@tanstack/react-query": "^5.90.21", "@tanstack/react-virtual": "^3.13.19", + "@testing-library/dom": "^10.4.1", "@types/three": "^0.182.0", "@vercel/analytics": "1.6.1", "@vercel/speed-insights": "^1.3.1", From 37c648c5107ec3d3e7a904579b4e25e2dcf6e0f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 7 Mar 2026 00:33:57 +0000 Subject: [PATCH 3/3] Fix CI: remove @testing-library/dom from prod deps, fix ref-during-render lint error - Remove '@testing-library/dom' from production dependencies in package.json (was accidentally added by npm install during dev; it's a peer dep of @testing-library/react and already in bun.lock as a transitive dep). This restores bun install --frozen-lockfile compatibility. - Fix ESLint react-hooks/refs error: remove the 'drawerOpenRef.current = ...' assignment that was happening during render. Replace with direct inclusion of showGallery and showLeftSidebar in the openLightbox useCallback dep array. The ref was used to avoid adding these values to the dep array, but the linter correctly flags render-time ref mutations. The dep-array approach is simpler and equally correct. Co-authored-by: CanyoufeeltheAGI <255605710+CanyoufeeltheAGI@users.noreply.github.com> --- hooks/use-studio-ui.ts | 11 +++-------- package.json | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/hooks/use-studio-ui.ts b/hooks/use-studio-ui.ts index f3effa8..a0e89fc 100644 --- a/hooks/use-studio-ui.ts +++ b/hooks/use-studio-ui.ts @@ -128,15 +128,12 @@ export function useStudioUI(): UseStudioUIReturn { // ======================================== // Pending-lightbox refs: used to delay the open on mobile until drawer - // animation completes, keeping openLightbox's dep array stable. + // animation completes. const pendingLightboxTimerRef = React.useRef | null>(null); const pendingLightboxRef = React.useRef<{ pending: boolean; image: LightboxImage | null }>({ pending: false, image: null, }); - // Latest drawer state, read inside openLightbox without adding to deps - const drawerOpenRef = React.useRef({ showGallery, showLeftSidebar }); - drawerOpenRef.current = { showGallery, showLeftSidebar }; // Cancel any pending timer when the hook unmounts React.useEffect(() => { @@ -149,9 +146,7 @@ export function useStudioUI(): UseStudioUIReturn { const openLightbox = React.useCallback( (image: LightboxImage | null) => { - const { showGallery: isGalleryOpen, showLeftSidebar: isSidebarOpen } = - drawerOpenRef.current; - const hasOpenDrawer = isMobile && (isGalleryOpen || isSidebarOpen); + const hasOpenDrawer = isMobile && (showGallery || showLeftSidebar); // Cancel any previous pending open if (pendingLightboxTimerRef.current !== null) { @@ -182,7 +177,7 @@ export function useStudioUI(): UseStudioUIReturn { setIsFullscreen(true); } }, - [isMobile, setShowLeftSidebar, setShowGallery], + [isMobile, showGallery, showLeftSidebar, setShowLeftSidebar, setShowGallery], ); // Ref for the closeLightbox cleanup timer so it can be cancelled on unmount diff --git a/package.json b/package.json index 6d29998..2251ac5 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "@stripe/stripe-js": "^8.8.0", "@tanstack/react-query": "^5.90.21", "@tanstack/react-virtual": "^3.13.19", - "@testing-library/dom": "^10.4.1", "@types/three": "^0.182.0", "@vercel/analytics": "1.6.1", "@vercel/speed-insights": "^1.3.1",