feat(templates): popup template library with block patterns and editor picker - #1249
Open
danieliser wants to merge 42 commits into
Open
feat(templates): popup template library with block patterns and editor picker#1249danieliser wants to merge 42 commits into
danieliser wants to merge 42 commits into
Conversation
…ditor picker Adds a tiered popup template library for the Gutenberg popup editor: - TemplateLibrary service: registry of popup content templates with categories, tiers (free/pro/pro_plus), recommended trigger/cookie presets, and a popup_maker/popup_templates filter for Pro & addons. Premium templates surface as locked teasers with upgrade URLs when their provider isn't active. - TemplateLibrary controller: registers templates as popup-post-type scoped block patterns with Popup Maker pattern categories. - Block editor template picker: modal with category browsing, search, live block previews, and pro teasers; auto-opens for new empty popups. Recommended triggers/cookies apply through the settings metabox's own row APIs (PUM_Admin.triggers/cookies.rows.add) so applied presets save with the form instead of being clobbered by the metabox POST. - 12 free templates (subscribe, promos, lead capture, compliance, engagement) built from core blocks + CTA button blocks, theme- inheriting, with validated serialized markup. - Integration tests covering registry, teasers, normalization, editor data, block parsing of every template, and pattern registration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughIntroduces a Popup Template Library with a PHP service and controller, twelve built-in templates, block-pattern registration, localized editor data, a block editor picker for insertion and recommended settings, and supporting PHP tests. ChangesPopup Template Library
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant WP as WordPress init
participant Controller as TemplateLibrary Controller
participant Service as TemplateLibrary Service
WP->>Controller: init hook at priority 15
Controller->>Service: get categories and templates
Controller->>Service: check insertability
Controller->>WP: register block pattern categories and patterns
sequenceDiagram
participant User
participant Plugin as TemplateLibraryPlugin
participant Modal as TemplateLibraryModal
participant Editor as Block Editor Store
participant Apply as applyRecommendedSettings
User->>Plugin: open popup editor or new popup
Plugin->>Modal: open template picker
User->>Modal: select template
Modal->>Editor: parse and insert or reset blocks
Modal->>Apply: apply recommended settings
Apply-->>Modal: return trigger and cookie counts
Modal->>User: show notice and close picker
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/block-editor/src/plugins/template-library/modal.tsx (1)
275-295: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap
applyRecommendedSettingsin a try-catch.
applyRecommendedSettingscalls the externalPUM_Adminmetabox API (rows.add), which could throw. If it does,setApplyStep(null)andonClose()never execute, leaving the modal stuck in the apply step with no recovery path.🛡️ Proposed fix
const finishApplyStep = ( apply: boolean ) => { if ( apply && applyStep ) { - const applied = applyRecommendedSettings( - applyStep.recommended, - popupId - ); - - if ( applied.triggers || applied.cookies ) { - createSuccessNotice( - __( - 'Recommended settings added. Review them in the Popup Settings box, then save.', - 'popup-maker' - ), - { type: 'snackbar' } - ); + try { + const applied = applyRecommendedSettings( + applyStep.recommended, + popupId + ); + + if ( applied.triggers || applied.cookies ) { + createSuccessNotice( + __( + 'Recommended settings added. Review them in the Popup Settings box, then save.', + 'popup-maker' + ), + { type: 'snackbar' } + ); + } + } catch { + createErrorNotice( + __( 'Could not apply recommended settings. You can configure them manually in the Popup Settings box.', 'popup-maker' ), + { type: 'snackbar' } + ); } } setApplyStep( null ); onClose(); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/block-editor/src/plugins/template-library/modal.tsx` around lines 275 - 295, The finishApplyStep flow in modal.tsx should guard applyRecommendedSettings against exceptions so the modal always cleans up. Wrap the applyRecommendedSettings call in a try-catch inside finishApplyStep, keep the existing success notice logic on successful apply, and ensure setApplyStep(null) and onClose() still run from a finally path even if PUM_Admin rows.add throws.classes/Services/TemplateLibrary.php (1)
192-204: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueDynamic
includeof globbed file paths — low risk but worth hardening.Static analysis flagged the
include $fileat line 199 as a dynamic file inclusion pattern. The path originates from the plugin's ownincludes/popup-templates/directory viacontainer->get_path(), so exploitation requires filesystem write access to the plugin directory. Risk is low, but adding arealpath()containment check is a cheap defense-in-depth measure.[source_static_analysis_hints]
🛡️ Optional hardening
foreach ( $files as $file ) { + // Ensure the file is within the expected template directory. + $real = realpath( $file ); + if ( false === $real || 0 !== strpos( $real, $path ) ) { + continue; + } + $template = include $file;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@classes/Services/TemplateLibrary.php` around lines 192 - 204, The dynamic include in the template-loading logic needs a containment check before loading files. In the method that builds templates from globbed PHP files, resolve each candidate with realpath() and verify it stays under the expected popup-templates directory before calling include. Keep the existing slug-based template registration flow in place, but harden the include path so only files from the intended directory can be executed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/popup-templates/cookie-notice.php`:
- Around line 26-43: The popup template markup nests wp:popup-maker/cta-buttons
inside wp:buttons, which is invalid because the Buttons block only accepts
button children and can break editor/save behavior. Update the structure in the
cookie notice template so the row uses wp:group with a flex layout instead of
wp:buttons, and keep the existing Reject All and Accept All button markup inside
that group. Use the surrounding block construction in the cookie notice template
as the place to adjust this layout.
In `@packages/block-editor/src/plugins/template-library/modal.tsx`:
- Around line 322-351: The category selection buttons in the template-library
modal only expose the active state through the is-active class, so update the
Button rendering in modal.tsx to add aria-current={ true } when the selected
category matches the current button. Apply this to both the “All templates”
button and the buttons generated from Object.entries(data.categories), using the
existing category state and slug comparison so screen readers can identify the
active selection.
- Around line 75-83: The nested action inside the template card is using Button
in the pro lock overlay, which creates invalid button-inside-button markup.
Update the template-library modal rendering so the lock UI in the
template.proRequired branch uses a non-interactive element instead of Button,
keeping the same icon/label styling while leaving the outer card button as the
only interactive control.
- Around line 187-189: `getCurrentPostId()` is being narrowed to `number`, which
hides the unsaved-post case and can pass `undefined` into the template flow.
Update the `editor` type in `template-library/modal.tsx` so `getCurrentPostId`
returns `number | undefined`, then add a guard in `finishApplyStep` before
calling `applyRecommendedSettings` to skip or defer processing when `popupId` is
undefined. Use the `finishApplyStep`, `applyRecommendedSettings`, and
`getCurrentPostId` symbols to locate the affected logic.
---
Nitpick comments:
In `@classes/Services/TemplateLibrary.php`:
- Around line 192-204: The dynamic include in the template-loading logic needs a
containment check before loading files. In the method that builds templates from
globbed PHP files, resolve each candidate with realpath() and verify it stays
under the expected popup-templates directory before calling include. Keep the
existing slug-based template registration flow in place, but harden the include
path so only files from the intended directory can be executed.
In `@packages/block-editor/src/plugins/template-library/modal.tsx`:
- Around line 275-295: The finishApplyStep flow in modal.tsx should guard
applyRecommendedSettings against exceptions so the modal always cleans up. Wrap
the applyRecommendedSettings call in a try-catch inside finishApplyStep, keep
the existing success notice logic on successful apply, and ensure
setApplyStep(null) and onClose() still run from a finally path even if PUM_Admin
rows.add throws.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cab428dd-4fc0-4680-ae7b-9c5dffa3e176
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
classes/Controllers/Assets.phpclasses/Controllers/TemplateLibrary.phpclasses/Plugin/Core.phpclasses/Services/TemplateLibrary.phpincludes/popup-templates/age-verification.phpincludes/popup-templates/announcement-banner.phpincludes/popup-templates/contact-us.phpincludes/popup-templates/cookie-notice.phpincludes/popup-templates/discount-coupon.phpincludes/popup-templates/flash-sale-promo.phpincludes/popup-templates/lead-magnet-download.phpincludes/popup-templates/newsletter-signup.phpincludes/popup-templates/social-follow.phpincludes/popup-templates/testimonial-proof.phpincludes/popup-templates/video-showcase.phpincludes/popup-templates/welcome-mat.phppackages/block-editor/package.jsonpackages/block-editor/src/plugins/index.tspackages/block-editor/src/plugins/template-library/apply-settings.tspackages/block-editor/src/plugins/template-library/editor.scsspackages/block-editor/src/plugins/template-library/index.tsxpackages/block-editor/src/plugins/template-library/modal.tsxpackages/block-editor/src/plugins/template-library/types.tstests/php/tests/TemplateLibrary_Test.php
Comment on lines
+26
to
+43
| '<!-- wp:buttons {"layout":{"type":"flex","justifyContent":"space-between"}} -->', | ||
| '<div class="wp-block-buttons">', | ||
| '<!-- wp:button {"className":"is-style-outline popmake-close"} -->', | ||
| sprintf( | ||
| '<div class="wp-block-button is-style-outline popmake-close"><a class="wp-block-button__link wp-element-button">%s</a></div>', | ||
| esc_html__( 'Reject All', 'popup-maker' ) | ||
| ), | ||
| '<!-- /wp:button -->', | ||
| '<!-- wp:popup-maker/cta-buttons -->', | ||
| '<div class="wp-block-popup-maker-cta-buttons"><!-- wp:popup-maker/cta-button -->', | ||
| sprintf( | ||
| '<div class="wp-block-popup-maker-cta-button"><a class="pum-cta wp-block-popup-maker-cta-button__link wp-element-button">%s</a></div>', | ||
| esc_html__( 'Accept All', 'popup-maker' ) | ||
| ), | ||
| '<!-- /wp:popup-maker/cta-button --></div>', | ||
| '<!-- /wp:popup-maker/cta-buttons -->', | ||
| '</div>', | ||
| '<!-- /wp:buttons -->', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby templates.
git ls-files 'includes/popup-templates/*.php' | sed -n '1,120p'
printf '\n--- cookie-notice.php excerpt ---\n'
cat -n includes/popup-templates/cookie-notice.php | sed -n '1,120p'
printf '\n--- search for similar block patterns ---\n'
rg -n --hidden --glob 'includes/popup-templates/*.php' 'wp:buttons|popup-maker/cta-buttons|wp:group' includes/popup-templatesRepository: PopupMaker/Popup-Maker
Length of output: 12195
wp:buttons cannot contain wp:popup-maker/cta-buttons core/buttons only allows core/button children, so this markup makes the template invalid in the editor and can drop the CTA block on save. Use wp:group with a flex layout for this row instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@includes/popup-templates/cookie-notice.php` around lines 26 - 43, The popup
template markup nests wp:popup-maker/cta-buttons inside wp:buttons, which is
invalid because the Buttons block only accepts button children and can break
editor/save behavior. Update the structure in the cookie notice template so the
row uses wp:group with a flex layout instead of wp:buttons, and keep the
existing Reject All and Accept All button markup inside that group. Use the
surrounding block construction in the cookie notice template as the place to
adjust this layout.
Comment on lines
+75
to
+83
| { template.proRequired ? ( | ||
| <div className="pum-template-library__card-lock"> | ||
| <Button | ||
| icon={ lock } | ||
| variant="link" | ||
| tabIndex={ -1 } | ||
| label={ __( 'Upgrade required', 'popup-maker' ) } | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Replace nested Button with a non-interactive element.
The Button component renders a <button> element, and placing it inside the outer card <button> produces invalid HTML (<button> inside <button>). Browsers may handle this inconsistently. Since the inner element is purely decorative (the outer card handles the click), use a <span> or <div> instead.
🔧 Proposed fix
<div className="pum-template-library__card-lock">
- <Button
- icon={ lock }
- variant="link"
- tabIndex={ -1 }
- label={ __( 'Upgrade required', 'popup-maker' ) }
- />
+ <span
+ aria-label={ __( 'Upgrade required', 'popup-maker' ) }
+ className="pum-template-library__card-lock-icon"
+ >
+ { lock }
+ </span>
</div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/block-editor/src/plugins/template-library/modal.tsx` around lines 75
- 83, The nested action inside the template card is using Button in the pro lock
overlay, which creates invalid button-inside-button markup. Update the
template-library modal rendering so the lock UI in the template.proRequired
branch uses a non-interactive element instead of Button, keeping the same
icon/label styling while leaving the outer card button as the only interactive
control.
Comment on lines
+187
to
+189
| const editor = select( 'core/editor' ) as unknown as { | ||
| getCurrentPostId: () => number; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Type getCurrentPostId() return as number | undefined.
WordPress's getCurrentPostId() can return undefined for unsaved posts. The type assertion () => number hides this. If popupId is undefined, resolveTokens in apply-settings.ts replaces {popup_id} with the string "undefined", producing invalid settings that get saved to the metabox.
🛡️ Proposed fix
const editor = select( 'core/editor' ) as unknown as {
- getCurrentPostId: () => number;
+ getCurrentPostId: () => number | undefined;
};Then guard before calling applyRecommendedSettings in finishApplyStep:
if ( apply && applyStep ) {
+ if ( ! popupId ) {
+ setApplyStep( null );
+ onClose();
+ return;
+ }
const applied = applyRecommendedSettings(
applyStep.recommended,
popupId
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const editor = select( 'core/editor' ) as unknown as { | |
| getCurrentPostId: () => number; | |
| }; | |
| const editor = select( 'core/editor' ) as unknown as { | |
| getCurrentPostId: () => number | undefined; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/block-editor/src/plugins/template-library/modal.tsx` around lines
187 - 189, `getCurrentPostId()` is being narrowed to `number`, which hides the
unsaved-post case and can pass `undefined` into the template flow. Update the
`editor` type in `template-library/modal.tsx` so `getCurrentPostId` returns
`number | undefined`, then add a guard in `finishApplyStep` before calling
`applyRecommendedSettings` to skip or defer processing when `popupId` is
undefined. Use the `finishApplyStep`, `applyRecommendedSettings`, and
`getCurrentPostId` symbols to locate the affected logic.
Comment on lines
+322
to
+351
| <li> | ||
| <Button | ||
| variant="link" | ||
| className={ | ||
| 'all' === category ? 'is-active' : '' | ||
| } | ||
| onClick={ () => setCategory( 'all' ) } | ||
| > | ||
| { __( 'All templates', 'popup-maker' ) } | ||
| </Button> | ||
| </li> | ||
| { Object.entries( data.categories ).map( | ||
| ( [ slug, label ] ) => ( | ||
| <li key={ slug }> | ||
| <Button | ||
| variant="link" | ||
| className={ | ||
| slug === category | ||
| ? 'is-active' | ||
| : '' | ||
| } | ||
| onClick={ () => | ||
| setCategory( slug ) | ||
| } | ||
| > | ||
| { label } | ||
| </Button> | ||
| </li> | ||
| ) | ||
| ) } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add aria-current to the active category button.
The active category is only indicated visually via the is-active CSS class. Screen reader users cannot identify which category is selected. Add aria-current={ true } to the active button.
♿ Proposed fix
<Button
variant="link"
className={
'all' === category ? 'is-active' : ''
}
+ aria-current={ 'all' === category }
onClick={ () => setCategory( 'all' ) }
>And for the category list buttons:
<Button
variant="link"
className={
slug === category
? 'is-active'
: ''
}
+ aria-current={ slug === category }
onClick={ () =>
setCategory( slug )
}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <li> | |
| <Button | |
| variant="link" | |
| className={ | |
| 'all' === category ? 'is-active' : '' | |
| } | |
| onClick={ () => setCategory( 'all' ) } | |
| > | |
| { __( 'All templates', 'popup-maker' ) } | |
| </Button> | |
| </li> | |
| { Object.entries( data.categories ).map( | |
| ( [ slug, label ] ) => ( | |
| <li key={ slug }> | |
| <Button | |
| variant="link" | |
| className={ | |
| slug === category | |
| ? 'is-active' | |
| : '' | |
| } | |
| onClick={ () => | |
| setCategory( slug ) | |
| } | |
| > | |
| { label } | |
| </Button> | |
| </li> | |
| ) | |
| ) } | |
| <li> | |
| <Button | |
| variant="link" | |
| className={ | |
| 'all' === category ? 'is-active' : '' | |
| } | |
| aria-current={ 'all' === category } | |
| onClick={ () => setCategory( 'all' ) } | |
| > | |
| { __( 'All templates', 'popup-maker' ) } | |
| </Button> | |
| </li> | |
| { Object.entries( data.categories ).map( | |
| ( [ slug, label ] ) => ( | |
| <li key={ slug }> | |
| <Button | |
| variant="link" | |
| className={ | |
| slug === category | |
| ? 'is-active' | |
| : '' | |
| } | |
| aria-current={ slug === category } | |
| onClick={ () => | |
| setCategory( slug ) | |
| } | |
| > | |
| { label } | |
| </Button> | |
| </li> | |
| ) | |
| ) } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/block-editor/src/plugins/template-library/modal.tsx` around lines
322 - 351, The category selection buttons in the template-library modal only
expose the active state through the is-active class, so update the Button
rendering in modal.tsx to add aria-current={ true } when the selected category
matches the current button. Apply this to both the “All templates” button and
the buttons generated from Object.entries(data.categories), using the existing
category state and slug comparison so screen readers can identify the active
selection.
The selector validation added for issue #993 used native querySelector, which rejects digit-leading class names (.2026-selector) per the CSS spec. jQuery's engine performs the actual click-trigger matching and has always accepted them, so fall back to it before rejecting a selector. Malformed selectors (unbalanced quotes/brackets) still fail both checks and are skipped with the console warning. Ref: https://wordpress.org/support/topic/extra-selectors-starting-with-a-number-no-longer-work/
Admin menu, page titles, list UI, and bulk-action notices used "Call to Actions"; the grammatically correct plural is "Calls to Action" (CTAs for the abbreviation). Ref: https://wordpress.org/support/topic/plural-of-call-to-action/
… restorable focus previouslyFocused is captured via $( ':focus' ), which never matches <body> (jQuery's matcher requires type/href/non-negative tabindex), so for auto-open popups the stored set was empty and pumAfterClose left post-close focus to browser defaults. Browsers generally drop focus to <body> themselves when the focused popup hides — verified in Chromium, where Tab already resumed from the top — so this usually behaved correctly by accident, but the outcome was implicit and environment-dependent rather than guaranteed. Per WCAG 2.4.3, focus still returns to the previously focused element when one exists and is visible. Otherwise it is now explicitly set on <body> via a temporary tabindex, making Tab-from-the-top deterministic, including when the prior element was removed or hidden while the popup was open. A tabindex already present on <body> is left untouched; the temporary attribute is only added and removed when body had none. Ref: https://wordpress.org/support/topic/closing-a-popup-should-move-focus-to-the-top-of-the-page/
force_ajax() attached the shortcode_atts_gravityforms filter on the popmake_popup_before_inner/after_inner template hooks, but since popup content preloading (1.21.0) the GF shortcode renders during preload_popup() at wp_enqueue_scripts:11 and the footer template only echoes the cached string — so the filter wrapped nothing and forms in popups rendered without AJAX. Full-page POSTs broke text confirmations and Form Submission triggers (thank-you popups never opened), while preload() still enqueued GF's ajax spinner scripts targeting an iframe that never existed. Sandwich the pum_popup_content pipeline (blocks at 9, shortcodes at 11) with the same filter at priorities 5/99 instead. Verified against GF on a local site: popup form gains target='gform_ajax_frame_N' + iframe, on-page instances of the same form remain non-AJAX. Ref: https://wordpress.org/support/topic/form-submission-thank-you-doesnt-appear-on-popup/
- Gravity Forms: esc_attr() the JSON settings attribute, store only whitelisted keys, and intersect known keys on read so poisoned option data cannot reach the render sink. - Assets: strip all *_license_key values from the currentSettings JS localization exposed to block-editor users. - Admin bar: scope the toolbar-action handler to #wpadminbar, use currentTarget, and require a strictly numeric popup ID.
Flip the pum_asset_cache_sslverify default to true so server-side asset fetches validate certificates. Still filterable for self-signed hosts.
- Pin only-allow to 1.2.2 in the preinstall guard. - Verify the strauss PHAR sha256 before executing it. - build-release.js: copy the versioned zip via fs (not a shell) and validate the tag-derived zip name, closing a command-injection sink.
- deploy-to-wordpress: always build from checked-out source instead of a mutable release zip, and force dry-run on any non-master ref. - deploy-readme-assets: gate to refs/heads/master with a pinned checkout. - build: pass user-controlled dispatch inputs via env instead of interpolating them into run scripts.
- Licensing: kses the remote license API error_message before it reaches the unescaped triple-mustache license template (stored XSS). - AdminBar: HTML-escape the computed selector before innerHTML in the Get Selector modal (DOM XSS from attacker-influenced page markup).
- Previews: require edit_post on the specific popup before force-loading a preview (shared preview nonce leaked draft/private popups to lower roles). - Settings::page: gate on manage_settings; reachable via the edit_posts Go Pro submenu and dumps all settings into inline JS. - PostTypes: map the full primitive-capability set for popup/theme/cta so every status stays behind the intended permission (editors could edit published). - RestAPI: require delete_post before trashing a CTA via the status field. - ObjectSearch: enforce the target post-type/taxonomy capability, not just the broad edit_posts gate. - CTA shortcode: only render published CTAs (was exposing non-public UUIDs). - Upsell: gate the integration notice on edit_popups (plugin enumeration). - Telemetry: require manage_settings before enabling telemetry opt-in.
…dering - CallToActions: force the pid tracking redirect same-site via wp_validate_redirect (open redirect); gate notrack on edit_popups; only act on published CTAs so a stale UUID cache can't keep a disabled CTA live. - KaliForms: type-check $_POST['data'] before decode (PHP 8 fatal) and validate pum_is_popup() before counting a conversion (forged popup IDs). - ACF: scope the non-public-post guard relaxation to the current popup's own ID so [acf post_id=<other private post>] can't read unrelated fields.
- AssetCache: canonicalize local asset paths with realpath and confirm they stay within WP_CONTENT_DIR/ABSPATH; reject stream wrappers. Stops directory traversal and file://phar:// reads being embedded into the public cache. - Debug: load the react-scan profiler from a pinned version over HTTPS with a Subresource Integrity hash instead of an unpinned protocol-relative CDN URL.
- bump-and-publish: validate npm names/versions and commit via execFileSync (no shell) so a malicious package manifest can't inject commands. - update-google-fonts: read the API key from GOOGLE_FONTS_API_KEY instead of a hard-coded key in source. - ci: fail the CI Summary gate when change-detection fails/cancels (skipped downstream jobs were counted as passes). - webpack: scope devServer allowedHosts instead of 'all' (DNS-rebinding). - .phpstorm.meta.php: bail at runtime to avoid a path-disclosing fatal on direct web access.
Monthly (and manual) job that refreshes includes/google-fonts.json from the Google Fonts API and opens a PR with any changes. Kept out of the release pipeline so releases stay reproducible and font-list changes stay reviewable. Uses the GOOGLE_FONTS_API_KEY repository secret.
refresh_license_status caught an exception and passed null into the array-typed update_license_status, and deactivate_license called it (and dereferenced ['license']) before its empty check — both fatal on an empty/error API body. Guard the callers so a transient failure no longer overwrites stored status, and make the setter defensively nullable.
… cta-buttons core/buttons is a container block; the transform only read the container's own attributes and created one empty core/button per selection, dropping the real inner button content. Clone the inner blocks through instead.
- icons/components: drop the module field that pointed at a nonexistent src/index.js (matches other packages; exports map is authoritative). - use-query-params: re-export ReactRouter6Adapter from the root and drop the malformed 'adapters' exports key; import it from the root in cta-admin. - types: define the previously-undefined IconType used by the Tab types. - block-library: add @popup-maker/icons to tsconfig paths/references. - block-editor: remove a stray token that broke the SCSS compile.
The importer files moved under includes/legacy/importer/ but the require paths still pointed at includes/importer/, so the admin Easy Modal import fatally failed.
build-release.js --output-dir only relocates the zip and cleanup removes the assembled tree, so the SVN deploy received a zip instead of the plugin files. Use --keep-build and copy the assembled ./popup-maker tree into BUILD_DIR.
The settings resolver dispatched SETTINGS_FETCH_ERROR even after a successful load, and both it and changeActionStatus dispatched fields at the top level while the reducer reads action.payload.* — throwing on the destructure. Wrap dispatches in payload and only report an error when settings actually failed to load.
…ded_popups current_popup() ignored its setter argument and get_loaded_popups() returned a plain array despite its documented WP_Query contract. Restore the legacy setter behavior and return a populated WP_Query so old callers don't break.
…oading The effect cleared url/linkTarget/rel whenever the CTA wasn't loaded yet, wiping the link during async resolution. Only clear once hasFinishedResolution is true and the CTA is genuinely absent.
…markup Blocks saved with the previous wp-block-buttons class had no deprecation and failed validation. Add a deprecation reproducing the old save output and migrating it forward.
The native editor referenced clientId before destructuring it and used several components (PanelBody, RichText, etc.) without importing them. Fix the ordering and add the missing imports. Native-only file; no web-build impact.
- Template::render guards extract() against non-array args. - get_post_type_labels casts labels to an array so the label helper can index it. - UpgradeStream::send_event normalizes scalar payloads (send_error passes strings). - Container::get_controller matches registration (Interfaces\Controller), so interface-only controllers are retrievable. - Drop the duplicate popmake_tag_args filter invocation. - Admin reset display reads the opens key (falls back to legacy views).
… conversion - url-search: return after a successful result instead of also dispatching error. - settings: guard the undeclared popupMakerCoreData global; apiPath returns the relative 'settings' so restBase isn't doubled into the namespace. - custom-fields: evaluate numeric field dependencies. - fields conversion: preserve object-selector args (post_type/taxonomy/object_key) and legacy html field content instead of dropping them.
…reviews - cta-button editor: create the CTA draft-first on save (via the 'new' editor id) instead of immediately publishing one on selection, so cancelling no longer leaves an orphaned published CTA. - Generate CTA URLs and block links using the filtered param names (localized via paramNames) instead of hardcoding cta/pid/notrack. - Add a cta-button deprecation for the previous wp-element-button markup. - Trigger preview reads the WP_Post-shaped localized popups (ID/post_title).
- Beaver Builder: type-check ajax settings.data (string/URLSearchParams/FormData) before parsing so non-string payloads don't throw. - Newsletter: always (re)attach the success observer, appending the hidden popup field only when absent. - pum-integrations: track non-AJAX conversions against the popup passed in args rather than the empty form-derived popup.
- Don't call onSave when saveEditorValues() reports failure. - Report a save as successful based on the action actually used (create vs update), so update success is no longer ignored. - Remove the duplicate beforeunload listener from the modal HOC (kept in with-data-store). - Only suppress the generic save notice when field-level notices were created, so validation errors can't be silently hidden. - Sort the CTA list by id/type/conversions/title explicitly. - REST stats schema declares 'conversions' to match the payload and sort key.
…ction The flat integration helper emits 'easy_digital_downloads' (derived from the label), but the Go Pro upsell only checked the legacy 'edd' key, so EDD-only sites weren't treated as having an ecommerce integration. Accept both keys.
omit() copied only the specified keys (behaving like pick), the inverse of its name and Omit<T,K> type. Return a shallow copy with those keys removed; update the characterization tests to assert the correct behavior.
…d UI guards - Block category icon points at assets/images/mark.svg (was 404ing). - License details show 'Deactivated' text for deactivated keys instead of always 'Active'. - New-install detection reads installed_on from popup_maker_version_info (the legacy pum_installed_on option is deleted after migration). - Guard the CTA type-filter label lookup against prototype keys from the URL. - Re-enable the upgrade button when connect-info validation fails.
- Base\Upgrade::stream() returns an anonymous class whose __call no-ops, so the fallback mock's methods are actually callable (a stdClass with closure properties fatals when called as methods). - Blocks::find_blocks honors the documented 'prefix/*' wildcard (and its own default of pum/*) instead of only exact-matching the literal string.
…wheel listener - load_plugin_textdomain gets a WP_PLUGIN_DIR-relative languages path, not an absolute one, so bundled translations load. - The package 'head' flag now maps to head placement (was inverted, sending head-flagged scripts to the footer). - _nx() wrapper matches WordPress's (single, plural, number, context, domain) signature instead of dropping the plural argument. - Select2 wheel listener is non-passive so its boundary preventDefault works.
PURGE compared numeric ids against string object keys (Object.entries), so byId, editedEntities, editHistory and editHistoryIndex kept purged records. Compare via a stringified id set; update the characterization tests to the correct behavior.
a2b72b7 removed the proven download-release-zip-and-extract path (the path all successful .org deploys used) in favor of build-from-source only, which had never run. Restore it as the primary path, keep the master-only dry-run guard, and keep the build-from-source fallback (now producing an unpacked tree, not a zip). Note: the mutable-release-asset concern that motivated the removal is real but modest (requires maintainer-level access, same as pushing to protected master); tracking integrity verification separately rather than dropping the working path.
…ller 955645e re-inlined the legacy WP_Query construction in the PUM_Site_Popups shim, undoing the refactor's controller delegation. Add get_loaded_popups_query() to the Frontend\Popups controller (which owns the shaping) and reduce the legacy method to a thin delegate that still populates the legacy static props for external readers.
- Introduced AGENTS.md to provide comprehensive guidance for Codex when working with the Popup Maker WordPress plugin, including project overview, quick start commands, architecture details, and extension APIs. - Added php-type-validator.toml for a specialized agent focused on validating PHPStan docblock types against actual code implementations, ensuring type accuracy and compliance. - Included phpstan-docblock-typer.toml for an agent dedicated to adding PHPStan-compliant docblocks to achieve higher compliance levels without modifying method implementations.
…uttons
Registers "Open Popup" as a Click Action option on Beaver Builder's Button
module (alongside Link, Button, Lightbox). Selecting it reveals a popup picker
and outputs the `popmake-{id}` class on the button, which Popup Maker's Click
Open trigger uses to open the popup — the same mechanism as the block editor
button and shortcodes.
The referenced popup is force-loaded on the page via maybe_preload_popup() so
it opens even when its own display conditions wouldn't otherwise match, while
still respecting the popup's enabled state. Preloading is skipped in the admin
and Beaver Builder editor. Popup options are labeled by post title.
Closes #607.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a tiered popup template library for the Gutenberg popup editor — 26 professionally designed popup layouts (12 free in core, 14 premium registered by Pro) based on competitive research across OptinMonster, Wisepops, OptiMonk, Elementor, Hustle, Poptin, and others.
Architecture
TemplateLibraryservice (template_librarycontainer key): registry of popup content templates with categories, tiers (free/pro/pro_plus), keywords, and recommended trigger/cookie presets. Extensible via thepopup_maker/popup_templatesfilter — Pro and addons register their templates there; registrations with content override core's locked teaser entries by slug.TemplateLibrarycontroller: registers every insertable template as a block pattern scoped topostTypes: ['popup'], plus Popup Maker pattern categories, so templates also surface in the native inserter.packages/block-editor/src/plugins/template-library/): auto-opens for new empty popups; category browsing, search, liveBlockPreviewcards; premium teasers render locked with tier badges and upgrade URLs (following thePUM_Upselllocked-preview conventions).PUM_Admin.triggers/cookies.rows.add()— into the settings metabox's live form state, because REST writes made while the editor is open are overwritten by the metabox form POST on save.Free templates (12)
Newsletter Signup, Announcement Banner, Flash Sale Promo, Discount Coupon, Lead Magnet Download, Welcome Mat, Contact Us, Cookie Notice, Age Verification, Video Showcase, Testimonial Proof, Social Follow.
All built from core blocks +
popup-maker/cta-button(shipped withoutctaIdso the user connects/creates a CTA after insert), theme-inheriting styling,.popmake-closedismiss actions, and translatable copy.Notes
popupMakerBlockEditorvars converted to a lazy closure; template data only localizes on the popup editor screen.@wordpress/blocksto the block-editor package dependencies.Test plan
TemplateLibrary_Test— 8 integration tests, 78 assertions: registry loading, teaser backfill/override, normalization, editor data shape,parse_blocksvalidation of every template (no stray non-block content), pattern + category registration.🤖 Generated with Claude Code
Summary by CodeRabbit