fix(popups): portal overlay lightboxes to wp_footer to escape stacking contexts#1561
fix(popups): portal overlay lightboxes to wp_footer to escape stacking contexts#1561adekbadek wants to merge 4 commits into
Conversation
…r stacking contexts Scroll-triggered overlay popups were rendered as the first child of `the_content`, which makes them descend from `.entry-content`. Any ancestor wrapper that creates a stacking context (transform, position+z-index, isolation, will-change, contain) traps the popup's `z-index: 99999` local to that context, so siblings of the wrapper with even a low z-index can paint over the lightbox close button. Queue overlay lightboxes at the inserter level and flush them at `wp_footer` priority 100 so they render as direct children of `<body>`. Inline / above-header / archive placements stay where they were. Scroll-triggered overlays keep their `#page-position-marker` inline in `.entry-content` (its percentage `top` resolves against the article column and drives the IntersectionObserver that reveals the lightbox). Only the lightbox itself is portaled; `generate_popup()` gets a `\$include_position_marker` flag, new helper `generate_position_marker()` emits the marker alone. Tested in docker (195/195 pass) and verified end-to-end on a local repro that wraps `.entry-content` in a transformed/positioned container with a higher-z sibling rail. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The DOM API property is camelCase by spec; cannot be renamed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
🎉 This PR is included in version 3.12.1-hotfix-overlay-stacking-context.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Blockers: - Drop array_reverse on the singular-content queue site: most-specific overlay must be queued first so the JS reveal walks it first and a segment-targeted prompt isn't suppressed by a generic one. - Emit the scroll-trigger page-position marker inline on the classic-theme archive path (insert_popups_after_header) and the block-theme above-header path (get_before_header_markup), not only on the singular content path. Without it, scroll-triggered overlays on archive pages silently never reveal. - Un-scope the wide/full-alignment CSS rules in style.scss from `.entry-content .newspack-lightbox` — the lightbox now renders as a body child, so the descendant scope no longer matches. Suggestions: - Move the wp_footer flush from priority 100 to 9 so it runs before wp_print_footer_scripts (priority 20) — overlay content that lazy-enqueues frontend assets keeps working. - New $emitted_markers static parallel to $queued_overlays, so the marker also dedupes when the inserter is reached from multiple paths in one request (prevents duplicate page-position-marker DOM IDs that would break document.getElementById lookups). - Defensive access on $popup['options']['trigger_type'] in generate_position_marker(); cast trigger_scroll_progress through absint() to harden the style attribute against weird meta values. - Replace strlen-based dedupe assertion with substr_count of the outer lightbox container id — direct and durable. - New tests: cross-injection-point dedupe + classic-archive marker emission. 197/197 PHPUnit pass. Cleanups: - Switch hook registration to [__CLASS__, 'print_queued_overlays'] — the method is already public static. - PHP 8.3 type hints on the new functions. - @param string $popup → array; drop "for backward compatibility" framing on $include_position_marker (it's a brand-new flag). - Refactor: emit_position_marker_inline() helper that dedupes and wraps in wp:html. Used from all three insertion paths. - En-dashes per project style. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
🎉 This PR is included in version 3.12.1-hotfix-overlay-stacking-context.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Pull request overview
Ports overlay (lightbox) prompts out of the_content and into wp_footer output so overlays render as direct <body> children and are no longer trapped by theme/plugin stacking contexts, while keeping scroll-trigger markers inline to preserve IntersectionObserver behavior.
Changes:
- Added overlay queueing/deduping in
Newspack_Popups_Inserter, flushed viawp_footer, and split scroll position marker generation from overlay lightbox markup. - Updated overlay alignment CSS to reflect the new
<body>-level lightbox location. - Updated/added PHPUnit coverage to validate queueing, dedupe behavior, marker placement, and ordering across injection paths.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
includes/class-newspack-popups-inserter.php |
Introduces overlay queue + footer flush, inline marker emission/dedupe, and updates insertion paths to queue instead of inline overlays. |
includes/class-newspack-popups-model.php |
Adds generate_position_marker() and makes overlay markup optionally omit the marker when rendering from the footer queue. |
src/view/style.scss |
Re-scopes overlay alignment rules from .entry-content to .newspack-lightbox to match new DOM placement. |
tests/wp-unittestcase-pagewithpopups.php |
Adjusts test rendering helper to append flushed footer overlays so DOM assertions still see overlay markup. |
tests/test-overlay-queueing.php |
Adds dedicated tests for overlay queueing, dedupe, marker-inline behavior, and flush semantics. |
tests/test-insertion.php |
Updates assertions to locate overlay markup in combined content+footer output instead of assuming DOM position. |
tests/test-block-theme-header-insertion.php |
Updates overlay specificity ordering assertion to check queued footer output instead of template-part inline output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- emit_position_marker_inline returns raw HTML; only the_content callsite wraps in <!-- wp:html --> (echo paths no longer leak block comments into the rendered page). - style.scss: drop duplicate `.alignfull.wp-block-columns` media block (the inner `&.wp-block-columns` rule under `.alignfull, .alignwide` already covers it). - test_dedupe_across_injection_points: drop the redundant set_popup_options call + clarify the comment to match what the test actually exercises. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
86a26b1 to
7351696
Compare
|
🎉 This PR is included in version 3.12.1-hotfix-overlay-stacking-context.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What
Overlay prompts are emitted inside
.entry-contentviathe_content. Any ancestor that establishes a stacking context (transform, position+z-index, isolation, will-change, contain — common with custom theme CSS or ad-management plugins) traps the lightbox'sz-index: 99999local to that context, so siblings can paint over the close button. Symptom observed in the wild: scroll-triggered Center Overlay prompts on customer sites end up partially obscured by the sticky right-rail.How
the_contentinstead of inlining them. Flush atwp_footerpriority 9 (beforewp_print_footer_scriptsat 20) so they render as direct children of<body>and any assets enqueued during overlay-content render still make it into the footer. Inline / above-header / archive placements unchanged.#page-position-marker_*inline at the content position; only the lightbox itself moves. The marker still drivesIntersectionObserverreveal as before.How to test
Activate Newspack Campaigns; create a Center Overlay prompt with scroll trigger at 0%.
Add a temporary mu-plugin that fakes the stacking trap:
Visit any post. Without the fix the rail paints over the prompt's right edge (close X hidden). With the fix the lightbox is a direct
<body>child at z=99999 and renders above the rail.Devtools sanity check:
document.querySelector('.newspack-lightbox').parentElement.tagName === 'BODY'anddocument.querySelector('[id^=page-position-marker_]').closest('.entry-content') !== null.Tests
197/197 PHPUnit pass. Dedicated tests in
tests/test-overlay-queueing.php: queueing, cross-injection-point dedupe, marker stays inline, archive scroll-trigger marker emission, flush semantics.Notes for review
release; needs forward-port totrunkonce merged.🤖 Generated with Claude Code