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..a0e89fc 100644 --- a/hooks/use-studio-ui.ts +++ b/hooks/use-studio-ui.ts @@ -115,39 +115,119 @@ 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. + const pendingLightboxTimerRef = React.useRef | null>(null); + const pendingLightboxRef = React.useRef<{ pending: boolean; image: LightboxImage | null }>({ + pending: false, + image: null, + }); + + // 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 hasOpenDrawer = isMobile && (showGallery || showLeftSidebar); + + // 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], + [isMobile, showGallery, showLeftSidebar, 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); }, []); // ========================================