Skip to content

fix: keep the modal overlay visible while a lazy chunk resolves - #92

Merged
lnieuwenhuis merged 1 commit into
stagingfrom
dev
Aug 13, 2026
Merged

fix: keep the modal overlay visible while a lazy chunk resolves#92
lnieuwenhuis merged 1 commit into
stagingfrom
dev

Conversation

@lnieuwenhuis

Copy link
Copy Markdown
Owner

A lazy modal always suspends for at least a frame on first mount, and the null Suspense fallback made the whole overlay disappear for that frame, so opening a template or recipe read as one modal closing and another opening.

Give each lazy modal a fallback that mirrors its own shell: the CompactModal based modals get a matching CompactModal with their real title, and the photo/barcode flows get a plain backdrop. The overlay is now continuous from click to content.

A lazy modal always suspends for at least a frame on first mount, and the
null Suspense fallback made the whole overlay disappear for that frame, so
opening a template or recipe read as one modal closing and another opening.

Give each lazy modal a fallback that mirrors its own shell: the CompactModal
based modals get a matching CompactModal with their real title, and the
photo/barcode flows get a plain backdrop. The overlay is now continuous from
click to content.
@lnieuwenhuis

lnieuwenhuis commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

Jeeves' Code Review

Round 1 · Relaxed · head 327485f9 · gpt-5.6-sol / high

Issues: 0 High · 1 Medium · 0 Low · 1 total

Summary

High Severity Issues

No high severity issues found.

Medium Severity Issues

  • Medium — Loading fallbacks cannot be dismissed. In dashboard-shell.tsx:64, ModalChunkFallback passes a no-op to CompactModal, so its close button, backdrop, and Escape handler do nothing. OverlayBackdropFallback at lines 73–79 has no dismissal path either. Chunk loading is unbounded on slow or stalled networks, so users can attempt to cancel yet still have the completed modal open later.

Low Severity Issues

No low severity issues found.

Recommended Changes

  • Give each fallback its real dismissal callback. Preserve the local Suspense boundary and continuous overlay, but ensure search/template/recipe fallbacks close their corresponding state, photo closes showPhotoModal, and barcode cancellation clears scanner/result/not-found state. Add a focused deferred-load test covering close, backdrop, and Escape behavior.
Codex prompt to fix the review
In C:\Users\lnieu\Documents\GitHub\macro-tracker, make a focused follow-up fix for lnieuwenhuis/macro-tracker PR #92, which has already merged into target branch staging. Work from the current dev branch and use reviewed head SHA 327485f9bfb6f0e415e74bcdba4e4a904ddcdf19 as the reference. Fix only the lazy-modal fallback cancellation bug: while a modal chunk is pending, its visible close button, backdrop dismissal, and Escape behavior must invoke the same state cleanup as the real modal. Cover search, templates, recipe picker, photo, and barcode flows while preserving code splitting, the local Suspense boundary, and continuous overlay. Add a focused test using a deferred loader to prove that dismissing during loading prevents the resolved modal from opening. Run the focused test, web unit tests, lint, typecheck, and build. Do not refactor unrelated code.
Round history

Issue trend: 1

Round Head Mode High Medium Low Total
1 327485f9 relaxed 0 1 0 1

@lnieuwenhuis
lnieuwenhuis merged commit b0afffb into staging Aug 13, 2026
12 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