Skip to content

Fix mobile page freeze after closing lightbox opened from history drawer - #34

Merged
CanyoufeeltheAGI merged 3 commits into
mainfrom
copilot/fix-history-drawer-lightbox-issue
Mar 7, 2026
Merged

CanyoufeeltheAGI merged 3 commits into
mainfrom
copilot/fix-history-drawer-lightbox-issue

Conversation

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor
  • Fix pointer-events/interaction freeze after closing lightbox from history drawer on mobile
    • Delay lightbox opening until after drawer close animation completes (300ms)
    • Improve closeLightbox cleanup with setTimeout(300) and comprehensive body attribute reset
    • Cancel pending timers on unmount
    • 5 new regression tests with vi.useFakeTimers()
  • Fix CI failures
    • Remove @testing-library/dom from production dependencies (was accidentally added by npm install; restores bun install --frozen-lockfile compatibility)
    • Fix ESLint react-hooks/refs error: replace render-time drawerOpenRef.current assignment with direct dep array inclusion of showGallery/showLeftSidebar
  • bun install --frozen-lockfile passes ✓
  • bun run lint passes with 0 errors ✓
  • All 23 use-studio-ui tests pass ✓

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Mar 6, 2026
@vercel

vercel Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
bloomstudio Ignored Ignored Preview Mar 7, 2026 0:34am

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>
Copilot AI changed the title [WIP] Fix bug in mobile view of history drawer lightbox Fix mobile page freeze after closing lightbox opened from history drawer Mar 6, 2026
…nder 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>

Copilot AI commented Mar 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix this PR and get it into mergeable state. Proof of mergeable must be provided. Do not stop until it’s fixed.

Fixed in commit 37c648c. Two issues were blocking CI:

  1. bun install --frozen-lockfile failure@testing-library/dom had been accidentally added to production dependencies in package.json by a stray npm install during the previous session. Since bun.lock was never updated to match, bun rejected it as a lockfile change. Removed the entry; it's already in the lockfile as a transitive dep of @testing-library/react.

  2. ESLint react-hooks/refs errordrawerOpenRef.current = { showGallery, showLeftSidebar } was written directly in the render body to avoid adding those values to openLightbox's dep array. The linter correctly rejects ref mutations during render. Simplified: removed the ref entirely and added showGallery/showLeftSidebar directly to the useCallback dep array — cleaner and equally correct.

Both bun install --frozen-lockfile and bun run lint now pass cleanly, and all 23 tests continue to pass.

@CanyoufeeltheAGI
CanyoufeeltheAGI merged commit 9ee44aa into main Mar 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant