feat(upsell): enhance Go Pro tab with social proof and feature highlights#1151
feat(upsell): enhance Go Pro tab with social proof and feature highlights#1151danieliser merged 9 commits intodevelopfrom
Conversation
❌ Commit Validation ErrorYour commits don't pass validation. Common issues: Format RequirementsCommits must follow conventional commit format: Valid types: feat, fix, improve, perf, refactor, docs, style, test, build, ci, chore, revert Examples:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/Controllers/PostTypes.php (1)
541-561: Docblock@returntype is inconsistent with actual return.The method now correctly returns
$replaceto comply with thereplace_editorfilter contract, but the docblock still declares@return void. Additionally, theWP_Posttype hint should be prefixed with a backslash for the fully qualified class name.Proposed fix
/** * Replace the editor interface when classic editor is forced. * - * @param bool $replace Whether to replace the editor. - * @param WP_Post $post The post object. - * @return void + * @param bool $replace Whether to replace the editor. + * @param \WP_Post $post The post object. + * @return bool Whether to replace the editor. */ public function replace_editor( $replace, $post ) {
🤖 Fix all issues with AI agents
In @.coderabbit.yaml:
- Around line 27-28: The configuration key auto_incremental_review is currently
placed at the wrong nesting level under reviews; move it under the auto_review
mapping so the schema is respected: remove auto_incremental_review from its
current location and add it as a child of auto_review (ensuring the auto_review
block exists), preserving its boolean value true and indentation consistent with
other keys; update any references expecting reviews.auto_incremental_review to
auto_review.auto_incremental_review.
In @.github/workflows/ci.yml:
- Around line 116-124: The workflow step named "Setup PHP" uses the action
shivammathur/setup-php@v2 with an invalid input `cache: composer`; remove the
`cache: composer` line from that step and rely on actions/cache for composer
caching or the action's built-in behavior instead, keeping the other inputs
(`php-version`, `extensions`, `tools`, `coverage`) unchanged.
- Around line 315-320: The Setup PHP step ("Setup PHP" using
shivammathur/setup-php@v2) includes an invalid input `cache: composer`; remove
that `cache: composer` line from the step and either add a separate
actions/cache step to cache Composer files or use the supported caching
mechanism documented for shivammathur/setup-php (do not pass `cache: composer`
to the action).
- Around line 353-359: The "Setup PHP" step using shivammathur/setup-php@v2
includes an invalid input "cache: composer"; remove that line from the step (or
replace it with a supported configuration) so the action only uses documented
inputs (php-version, extensions, coverage, etc.); if you want composer caching
when the job is enabled, implement it via actions/cache (cache composer files)
rather than the unsupported "cache: composer" input on
shivammathur/setup-php@v2.
- Line 34: Replace all uses of actions/checkout@v6 with actions/checkout@v5
throughout the workflow; locate every occurrence of the string
"actions/checkout@v6" (multiple spots noted) and change it to
"actions/checkout@v5" while leaving other actions like "actions/setup-node@v6"
unchanged.
In @.github/workflows/notify-support.yml:
- Around line 112-115: The current assignment using single quotes for CHANGELOG
(CHANGELOG='${{ steps.changelog.outputs.changelog }}') can break when the
changelog contains single quotes; update the workflow to capture the output into
a shell-safe variable (e.g., assign with double quotes or use a here-doc /
printf to preserve raw content) and then continue to produce CHANGELOG_ESCAPED
from that safe variable (the CHANGELOG and CHANGELOG_ESCAPED symbols are the
targets to update), ensuring the value is not wrapped in single quotes before
piping into jq -Rs .
In @.github/workflows/request-approval.yml:
- Line 91: The unquoted assignment CHANGELOG=${{
steps.changelog.outputs.changelog }} can break if the changelog contains spaces
or special chars; update the assignment to quote the output value, e.g.
CHANGELOG='${{ steps.changelog.outputs.changelog }}', so the shell receives the
JSON-quoted string intact and no word-splitting or special-character
interpretation occurs.
In @.husky/pre-commit:
- Around line 1-3: The pre-commit Husky hook is missing a shebang so the system
may choose the wrong shell; add a proper shebang as the first line (e.g.,
#!/bin/sh or #!/usr/bin/env sh) to the top of the .husky/pre-commit file, ensure
the rest of the file is valid POSIX shell syntax (or adjust to bash shebang if
using bash-specific features), and keep the hook executable (chmod +x
.husky/pre-commit).
In @.release-please-config.json:
- Around line 11-12: The "changelog-type" value is invalid; replace the current
"changelog-type": "wordpress" entry with a supported option ("default" or
"github") so Release Please can generate changelogs properly, e.g., set
"changelog-type" to "default" (keeping "changelog-path": "changelog.txt"
unchanged).
In @assets/js/src/integration/kaliForms.js:
- Around line 20-92: Both listeners (the global 'kaliformProcessCompleted'
handler and the per-submit 'kaliFormSuccess' successHandler) can call
window.PUM.integrations.formSubmission() twice; add a deduplication flag on the
form element (follow the pumNewsletterHandled pattern) so each form is handled
once: before calling window.PUM.integrations.formSubmission($form, ...) check if
the form has the handled flag (e.g., $form.data('pumFormHandled') or reuse
'pumNewsletterHandled'), return if set, otherwise set the flag and then call
formSubmission; apply this check/set in both the kaliformProcessCompleted
listener and inside the successHandler registered in the submit handler.
In @classes/Admin/BlockEditor.php:
- Around line 68-73: The function register_block_assets declares an unused $hook
parameter which static analysis flags; remove the $hook parameter from the
register_block_assets method signature and update any places that register it as
a callback (e.g., the add_action/add_filter call that references
'Admin\BlockEditor::register_block_assets' or register_block_type callbacks) so
the callback is registered without expecting arguments, then run a quick grep
for register_block_assets to ensure no other callers rely on the removed
parameter and adjust them if found.
- Around line 56-64: The method Admin\BlockEditor::register_block_assets
declares an unused parameter $hook; remove the unused parameter from the
signature (public static function register_block_assets()) or rename it to
$_hook to signal intentional non-usage, and update the docblock to remove or
reflect the renamed parameter so the signature and documentation match.
In @classes/Controllers/Compatibility/Builder/Divi.php:
- Around line 130-139: The docblock above the method that "Force classic editor
for popups when Divi 4 is active" incorrectly states it runs at priority 5 while
the hook is actually registered at priority 999 in setup_divi_hooks(); update
the docblock to reflect the real priority (999) or change the hook registration
in setup_divi_hooks() to 5 so they match—preferably adjust the docblock to say
"priority 999 (before PostTypes filter at priority 10)" and ensure the
referenced priority value in the comment matches the priority used in
setup_divi_hooks().
- Around line 171-178: The code in the Divi compatibility builder assumes
$field['desc'] exists when appending a note, which can trigger a PHP notice;
update the block that handles
$fields['general']['main']['enable_classic_editor'] (the $field reference inside
the if) to ensure 'desc' is set to an empty string if missing before
concatenation, then append the Divi 4 note and set 'disabled' as before so the
concatenation is safe.
In @classes/Integration/Form/Forminator.php:
- Around line 120-142: The on_success method calls
$this->increase_conversion($popup_id) without verifying $popup_id; update
on_success to guard the call the same way as KaliForms (e.g., check if $popup_id
is truthy) before invoking increase_conversion to avoid passing false to
pum_get_popup; keep the existing pum_integrated_form_submission call and leave
the unused $entry and $field_data_array parameters as-is for hook compatibility.
In @classes/Integration/Form/HappyForms.php:
- Around line 122-152: In HappyForms::on_success the code returns early when
$popup_id is falsy which prevents pum_integrated_form_submission() from being
called; change the flow so you only skip $this->increase_conversion( $popup_id )
when $popup_id is falsy but always call pum_integrated_form_submission([...])
with 'popup_id' => $popup_id, keeping the defensive form ID extraction and
existing payload keys; locate the early return that checks $popup_id in
on_success and remove it, ensuring increase_conversion remains conditional while
pum_integrated_form_submission is executed unconditionally.
In @docs/release-please-integration.md:
- Around line 27-31: The .release-please-config.json currently points
changelog-path to "changelog.txt" which doesn't exist; update the config to
reference the actual changelog file by setting the "changelog-path" for the
package entry to "CHANGELOG.md" (e.g., ensure the JSON includes "packages": {
".": { "changelog-path": "CHANGELOG.md" } }); modify the "changelog-path" value
in .release-please-config.json accordingly so Release Please can find and update
the changelog.
In @readme.txt:
- Line 372: The README line "Fix: Popup title aria-labelledby attribute was
being double-quoted. Thanks to the [Equalize
Digital](https://equalizedigital.com/) team." still contains a double space
before "to"; update the text to a single space ("Thanks to") by replacing
"Thanks to" with "Thanks to" in the README entry (the line starting "Fix: Popup
title aria-labelledby attribute...").
🧹 Nitpick comments (15)
includes/integrations/class-pum-woocommerce-integration.php (1)
101-105: Consider extracting the WooCommerce label for consistency.The
'WooCommerce'translation is defined inregister_conditions()as$woocommerce_textbut duplicated here. For maintainability, consider extracting it to a class constant or helper method.♻️ Optional refactor
+ private static function get_woocommerce_label() { + // phpcs:ignore WordPress.WP.I18n.TextDomainMismatch + return __( 'WooCommerce', 'woocommerce' ); + } + public static function register_conditions( $conditions = [] ) { - // phpcs:ignore WordPress.WP.I18n.TextDomainMismatch - $woocommerce_text = __( 'WooCommerce', 'woocommerce' ); + $woocommerce_text = self::get_woocommerce_label(); // ... rest of method } public static function condition_sort_order( $order = [] ) { - $order[ __( 'WooCommerce', 'woocommerce' ) ] = 5.256; // phpcs:ignore WordPress.WP.I18n.TextDomainMismatch + $order[ self::get_woocommerce_label() ] = 5.256; return $order; }classes/Controllers/Compatibility/Builder/Divi.php (1)
102-128: Add blank line between methods for consistency.WordPress coding standards recommend a blank line between method definitions. There's a missing blank line between
is_divi_available()andis_divi4_active().🔧 Suggested fix
$is_divi_available = $theme_available || $plugin_available || $general_available; return $is_divi_available; } + /** * Detect if Divi 4 is active (not 5+).assets/js/src/integration/happyforms.js (1)
23-30: Consider validatingformIdbefore use.If the hidden input is missing or empty,
formIdwill beundefined, which could propagate toformSubmission. The$sameIdFormsfilter would also match all forms with undefined/empty values.Proposed defensive check
// Extract form ID from hidden input. const formId = $form.find( '[name="happyforms_form_id"]' ).val(); + + if ( ! formId ) { + return; + } // Generate instance ID from form element index (for multiple instances of same form).classes/Controllers/WP/Dashboard.php (1)
40-47: Consider adding permission check when registering the widget.The permission check in
render_basic_analytics_widget()prevents content rendering, but the widget is still registered for all admin users. Users without theedit_popupspermission will see an empty widget.♻️ Proposed improvement
public function register_dashboard_widgets() { + if ( ! current_user_can( $this->container->get_permission( 'edit_popups' ) ) ) { + return; + } + // Basic Analytics Widget (will be moved to free plugin). wp_add_dashboard_widget( 'pum_analytics_basic', __( 'Popup Analytics','popup-maker' ), [ $this, 'render_basic_analytics_widget' ] ); }.github/workflows/release-please.yml (1)
69-76: Consider adding error handling for git push failure.If the
git pushfails (e.g., due to branch protection rules or concurrent updates), the workflow will fail silently after the release is already created. This could leave version files out of sync.💡 Suggested improvement
- name: Commit version updates if: ${{ steps.release.outputs.release_created }} run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add -A git diff --staged --quiet || git commit -m "chore(release): update version files to ${{ steps.release.outputs.tag_name }}" - git push + git push || { + echo "::error::Failed to push version updates. Manual intervention may be required." + exit 1 + }.github/workflows/notify-support.yml (1)
197-201: Slack webhook URL exposure risk in error logs.If the curl command fails, error messages might expose the webhook URL. Consider suppressing verbose error output or using
-f(fail silently) with explicit error handling.💡 Suggested improvement
- name: Send to #support-updates if: steps.vars.outputs.dry_run != 'true' env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_SUPPORT }} run: | echo "📤 Sending support notification to Slack..." - curl -X POST "$SLACK_WEBHOOK_URL" \ + HTTP_CODE=$(curl -s -o /tmp/slack_response.txt -w "%{http_code}" -X POST "$SLACK_WEBHOOK_URL" \ -H "Content-Type: application/json" \ - -d '${{ steps.payload.outputs.payload }}' + -d '${{ steps.payload.outputs.payload }}') + if [ "$HTTP_CODE" -ne 200 ]; then + echo "::error::Slack notification failed with HTTP $HTTP_CODE" + exit 1 + fi echo "" echo "✅ Slack message sent to #support-updates!".github/workflows/request-approval.yml (1)
127-131: Fragile JSON quote stripping with bash substring.Using
${CHANGELOG:1:-1}to strip JSON quotes is fragile. If the changelog is empty or malformed, this could produce unexpected output. Consider a more robust approach.💡 Suggested improvement
{ "type": "section", "text": { "type": "mrkdwn", - "text": "*Changelog Preview:*\n${CHANGELOG:1:-1}" + "text": "*Changelog Preview:*\n$(echo $CHANGELOG | jq -r '.')" } },Alternatively, avoid double-encoding by not using
jq -Rsinitially and instead escape just before payload construction.classes/Integration/Form/KaliForms.php (1)
50-70: Consider guardingget_forms()withenabled()check.Unlike
Forminator.phpwhich checks$this->enabled()before querying, this method queries posts unconditionally. If the Kali Forms plugin is deactivated but this method is called, it will still work (returning empty results), but an explicit guard would be more consistent with other integrations.♻️ Optional consistency improvement
public function get_forms() { + if ( ! $this->enabled() ) { + return []; + } + $forms = get_posts( [ 'post_type' => 'kaliforms_forms',classes/Integration/Form/HappyForms.php (1)
78-95: Minor inconsistency: Return type differs from other integrations.This method returns
falseon failure, whileKaliForms.phpreturnsnull. Consider aligning return types for consistency, though this is a minor concern since both are falsy.classes/Integration/Form/Elementor.php (2)
154-161: Consider adding a translators comment for the sprintf format.The format string
'%s (in %s)'concatenates translated strings. While the individual strings are translatable, the combined format lacks a translators comment to provide context for translators.Suggested improvement
foreach ( $forms as $form ) { // Use element_id as the unique identifier (system adds provider prefix). $location = ! empty( $form['post_title'] ) ? $form['post_title'] : __( 'Unknown Page', 'popup-maker' ); + /* translators: 1: Form name, 2: Page/post title where form is located */ $form_selectlist[ $form['element_id'] ] = sprintf( '%s (in %s)', $form['name'], $location ); }
101-122: Potential performance concern:get_post()called in a loop.For sites with many unique Elementor forms, calling
get_post()for each result in the loop may cause multiple database queries. Since this is cached for 1 hour and only runs once per cache miss, the impact is minimal, but worth noting.Consider batch-fetching posts using
get_posts()withpost__inif form count becomes a performance concern in the future.assets/js/src/integration/elementor.js (1)
12-14: Clarify variable naming:$formholds a DOM element, not a jQuery object.The
$prefix convention typically denotes jQuery objects, but$(this)[0]returns the raw DOM element. Consider renaming toformfor clarity, or keep the jQuery object if that's whatformSubmissionexpects.Suggested clarification
- const $form = $( this )[ 0 ]; + const form = $( this )[ 0 ]; // Get element_id from the widget container. // Elementor form widgets are inside a .elementor-element-{id} container. const $widget = $( this ).closest( '[data-id]' ); const elementId = $widget.length ? $widget.attr( 'data-id' ) : 'unknown'; - window.PUM.integrations.formSubmission( $form, { + window.PUM.integrations.formSubmission( form, { formProvider, formId: elementId, } );assets/js/src/integration/newsletter.js (2)
63-65: Remove redundantwindow.PUMguard.Based on learnings, guards that check for
window.PUMin integration files underassets/js/src/integration/are redundant because these files are bundled withwindow.PUM. This check creates dead code.Suggested fix
const handleSuccess = ( container, popupId ) => { // Prevent duplicate handling. if ( container.dataset.pumNewsletterHandled ) { return; } container.dataset.pumNewsletterHandled = 'true'; - if ( ! window.PUM || ! window.PUM.integrations ) { - return; - } - window.PUM.integrations.formSubmission( $( container ), {
176-180: Remove redundantwindow.PUMguard in initialization.Per learnings, this guard is unnecessary since the integration script is bundled with Popup Maker.
Suggested fix
// Initialize on DOM ready. $( () => { - if ( ! window.PUM ) { - return; - } - // Observe existing forms. initObservers();bin/extract-changelog.js (1)
46-48: Consider escaping hyphen for completeness.While the current regex metacharacter escaping works for most cases, adding hyphen (
-) to the escaped characters would makeescapeRegexmore robust for edge cases, though it's unlikely to be needed for version strings.Optional enhancement
function escapeRegex( str ) { - return str.replace( /[.+*?^$[\](){}|\\]/g, '\\$&' ); + return str.replace( /[.+*?^$[\](){}|\\-]/g, '\\$&' ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
.coderabbit.yaml.github/workflows/ci.yml.github/workflows/commitlint.yml.github/workflows/notify-support.yml.github/workflows/phpcs-tests.yml.disabled.github/workflows/phpunit-tests.yml.disabled.github/workflows/release-please.yml.github/workflows/request-approval.yml.husky/commit-msg.husky/pre-commit.release-please-config.json.release-please-manifest.jsonCHANGELOG.mdassets/js/src/integration/elementor.jsassets/js/src/integration/forminator.jsassets/js/src/integration/happyforms.jsassets/js/src/integration/index.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsbin/extract-changelog.jsclasses/Admin/BlockEditor.phpclasses/Admin/Settings.phpclasses/Admin/Templates.phpclasses/Controllers/Compatibility/Builder/Divi.phpclasses/Controllers/PostTypes.phpclasses/Controllers/WP/Dashboard.phpclasses/Extension/License.phpclasses/Integration/Form/Elementor.phpclasses/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/KaliForms.phpclasses/Integration/Form/Newsletter.phpclasses/Integrations.phpcommitlint.config.jscomposer.jsondocs/release-please-integration.mdincludes/integrations/class-pum-woocommerce-integration.phppackage.jsonpackages/fields/src/lib/utils.tsxpackages/fields/src/types/fields.tspackages/icons/package.jsonpackages/types/src/index.d.tsreadme.txt
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use PUM.hooks system for frontend extensibility in JavaScript/TypeScript
Files:
assets/js/src/integration/index.jspackages/fields/src/lib/utils.tsxbin/extract-changelog.jscommitlint.config.jspackages/fields/src/types/fields.tspackages/types/src/index.d.tsassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
**/*.{js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pm-best-practices.mdc)
**/*.{js,jsx}: Use thePUM.hooksevent manager for JavaScript actions and filters on any page where Popup Maker scripts are loaded
Use JavaScript hooks likepum.popup.beforeOpen,pum.popup.afterOpen,pum.popup.beforeClose,pum.popup.afterClose, andpum.integration.form.successfor extending Popup Maker functionality
Use the globalPUMobject to interact with popups via JavaScript methods likePUM.open(),PUM.close(), andPUM.getSettings()
DO NOT rely on the older$.fn.popmakemethods directly in JavaScript if aPUM.*equivalent exists, asPUM.*provides a more stable, public-facing API
Fire thepum.integration.form.successJavaScript action after a successful form submission to integrate a new form plugin with Popup Maker's Form Submission trigger
Files:
assets/js/src/integration/index.jsbin/extract-changelog.jscommitlint.config.jsassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pm-js-api-reference.mdc)
**/*.{js,ts,tsx,jsx}: Use the globalwindow.PUMobject as the primary way to interact with popups on the frontend. Available methods include:PUM.open(),PUM.close(),PUM.getPopup(),PUM.getSettings(),PUM.getSetting(),PUM.getCookie(),PUM.setCookie(),PUM.clearCookie(),PUM.clearCookies(), andPUM.checkConditions()
Use the jQuerypopmake()method directly on popup jQuery objects as an alternative to the globalPUMobject. Available methods include:popmake('open', callback),popmake('close', callback),popmake('getSettings'), andpopmake('getContainer')
Use Popup Maker's hooks system (PUM.hooks) to extend frontend functionality in an event-driven manner. UsePUM.hooks.addAction()for actions andPUM.hooks.addFilter()for filters. Key actions include:pum.popup.beforeOpen,pum.popup.afterOpen,pum.popup.beforeClose,pum.popup.afterClose, andpum.integration.form.success
For form integrations that submit via AJAX, listen for the form's successful submission event and callwindow.PUM.integrations.formSubmission()with the form's jQuery object and an object containingformProviderandformId
Files:
assets/js/src/integration/index.jspackages/fields/src/lib/utils.tsxbin/extract-changelog.jscommitlint.config.jspackages/fields/src/types/fields.tspackages/types/src/index.d.tsassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pm-js-api-reference.mdc)
To extend the Call to Action editor in
@popup-maker/cta-editor, use the Registry API methods:registerEditorTab()to add new tabs,registerEditorHeaderAction()to add header buttons, andregisterEditorHeaderOption()to add menu items
Files:
assets/js/src/integration/index.jspackages/fields/src/lib/utils.tsxbin/extract-changelog.jscommitlint.config.jspackages/fields/src/types/fields.tspackages/types/src/index.d.tsassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
composer.json
📄 CodeRabbit inference engine (.cursor/rules/wordpress-expert.mdc)
Use Composer for dependency management when building advanced plugins or themes; check composer.json for namespace prefixes when using brianhenry/strauss for compiled dependencies
Files:
composer.json
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Usepum_prefixes for all public functions and hooks in PHP
Use custom\PopupMaker\{ExtensionName}\namespace for classes/functions/hooks in PHP
Follow WordPress coding standards enforced by PHPCS
Use dependency injection over global access in PHP
Inline comments in PHP must end in full-stops, exclamation marks, or question marks (Squiz.Commenting.InlineComment.InvalidEndChar)
Never use strict PHP types on methods hooked to third-party actions/filters; use defensive validation instead
All PHP code must be compatible with PHP 7.4+; forbidden features include union types (e.g.,string|int),mixedtype, named arguments, and match expressions
Use%ifor the table name in $wpdb prepared queries
Check for Popup Maker existence before calling functions
Use defensive validation in hook callbacks for graceful handling of third-party plugin parameter changes
**/*.php: Use thepum_orPUM_prefix for your functions, classes, and hooks to maintain consistency with the core plugin's standards
DO NOT use the olderpopmake_prefix. This is for legacy code being phased out. Usepum_equivalent instead
Use thePopupMaker\namespace for new, modern integrations where appropriate, following the structure seen in theclasses/directory
Check for the existence of Popup Maker functions and classes before calling them usingfunction_exists()to prevent fatal errors if the core plugin is not active
DO NOT use deprecated functions or classes in Popup Maker integrations. Refer to the deprecation guide for alternatives
Use thepum_enqueue_scriptandpum_enqueue_stylefunctions for your assets to allow inclusion in Popup Maker's Asset Cache for optimized performance
DO NOT enqueue your own copies of libraries that Popup Maker already includes, such as jQuery or jQuery UI. Use the registered handles as dependencies
Use thepum_registered_triggers,pum_registered_cookies, andpum_registered_conditionsfilters to register custom items so they appea...
Files:
classes/Controllers/PostTypes.phpclasses/Admin/Templates.phpclasses/Admin/BlockEditor.phpclasses/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpclasses/Integrations.phpincludes/integrations/class-pum-woocommerce-integration.phpclasses/Integration/Form/KaliForms.phpclasses/Integration/Form/Newsletter.phpclasses/Admin/Settings.phpclasses/Controllers/Compatibility/Builder/Divi.phpclasses/Extension/License.phpclasses/Controllers/WP/Dashboard.php
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Do not useanytype in TypeScript; useunknownfor dynamic values outside of the type system, or find/define the type in another package or reasonably for internal use
Prioritize@popup-maker/*packages over@wordpress/*when available for better optimization and consistency
Use Popup Maker's data stores (popupStore,callToActionStore) instead of WordPress core data when possible
Files:
packages/fields/src/lib/utils.tsxpackages/fields/src/types/fields.tspackages/types/src/index.d.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pm-js-api-reference.mdc)
**/*.{tsx,jsx}: In React components within the admin area, use@wordpress/data'suseSelecthook with Popup Maker's data stores (callToActionStore,licenseStore,popupStore,settingsStore) to access and manage plugin state
Use reusable React components from the@popup-maker/componentspackage for building admin interfaces, including:ListTable,PopupSelectControl,EntitySelectControl,URLControl, andConfirmDialogue
Use the declarative field configuration system from@popup-maker/fieldspackage to render form fields from configuration objects in admin interfaces
Files:
packages/fields/src/lib/utils.tsx
🧠 Learnings (34)
📚 Learning: 2026-01-09T02:58:44.304Z
Learnt from: danieliser
Repo: PopupMaker/Popup-Maker PR: 1144
File: assets/js/src/integration/happyforms.js:8-35
Timestamp: 2026-01-09T02:58:44.304Z
Learning: In assets/js/src/integration/**/*.js, guards that check for window.PUM (e.g., conditional blocks that run only if window.PUM exists) are redundant because these integration files are bundled with window.PUM. Remove such guards and any branches dependent on the presence of window.PUM to simplify code and avoid dead branches.
Applied to files:
assets/js/src/integration/index.jsassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.{js,jsx} : Fire the `pum.integration.form.success` JavaScript action after a successful form submission to integrate a new form plugin with Popup Maker's Form Submission trigger
Applied to files:
assets/js/src/integration/index.jsclasses/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.jsreadme.txt
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Avoid using the `the_popup_content` filter; use `pum_popup_content` instead
Applied to files:
classes/Controllers/PostTypes.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use `pum_popup_content` filter instead of the deprecated `the_popup_content` filter
Applied to files:
classes/Controllers/PostTypes.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use the `pum_popup_settings_fields` and `pum_theme_settings_fields` filters to add new settings tabs, sections, and fields to the Popup and Theme editors
Applied to files:
classes/Controllers/PostTypes.phpclasses/Admin/Settings.phpclasses/Controllers/Compatibility/Builder/Divi.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use the methods on popup and theme objects to get or update settings (e.g., `$popup->get_setting()`) rather than `get_post_meta` or `update_post_meta`
Applied to files:
classes/Controllers/PostTypes.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use the `pum_enqueue_script` and `pum_enqueue_style` functions for your assets to allow inclusion in Popup Maker's Asset Cache for optimized performance
Applied to files:
classes/Admin/BlockEditor.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.{js,jsx} : Use the `PUM.hooks` event manager for JavaScript actions and filters on any page where Popup Maker scripts are loaded
Applied to files:
classes/Admin/BlockEditor.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : DO NOT enqueue your own copies of libraries that Popup Maker already includes, such as jQuery or jQuery UI. Use the registered handles as dependencies
Applied to files:
classes/Admin/BlockEditor.php
📚 Learning: 2025-12-20T07:31:31.480Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T07:31:31.480Z
Learning: Applies to tests/e2e/**/*.{ts,js} : Write E2E tests using Playwright in the `tests/e2e/` directory to test popup functionality and admin workflows against local WordPress environment
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Create form integrations by extending `PUM_Abstract_Integration_Form` class and registering via `pum_integrations` filter
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpclasses/Integrations.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpassets/js/src/integration/forminator.jsreadme.txt
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use the `PopupMaker\` namespace for new, modern integrations where appropriate, following the structure seen in the `classes/` directory
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpclasses/Integration/Form/KaliForms.phpclasses/Integration/Form/Newsletter.php
📚 Learning: 2025-12-20T07:32:04.308Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-js-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:04.308Z
Learning: Applies to **/*.{js,ts,tsx,jsx} : For form integrations that submit via AJAX, listen for the form's successful submission event and call `window.PUM.integrations.formSubmission()` with the form's jQuery object and an object containing `formProvider` and `formId`
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Form integration JavaScript must fire the `pum.integration.form.success` action on successful form submission
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : DO NOT use deprecated functions or classes in Popup Maker integrations. Refer to the deprecation guide for alternatives
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpclasses/Integration/Form/KaliForms.php
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Hook into `pum_integrated_form_submission` action to respond to successful form submissions from integrated forms
Applied to files:
classes/Integration/Form/Forminator.phpclasses/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Avoid directly instantiating `PUM_Fields`, `PUM_Form`, and related old settings API classes; use React-based admin interfaces instead
Applied to files:
classes/Integration/Form/HappyForms.phpclasses/Integration/Form/Elementor.phpclasses/Integrations.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.{js,jsx} : Use JavaScript hooks like `pum.popup.beforeOpen`, `pum.popup.afterOpen`, `pum.popup.beforeClose`, `pum.popup.afterClose`, and `pum.integration.form.success` for extending Popup Maker functionality
Applied to files:
classes/Integration/Form/HappyForms.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:04.308Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-js-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:04.308Z
Learning: Applies to **/*.{js,ts,tsx,jsx} : Use Popup Maker's hooks system (`PUM.hooks`) to extend frontend functionality in an event-driven manner. Use `PUM.hooks.addAction()` for actions and `PUM.hooks.addFilter()` for filters. Key actions include: `pum.popup.beforeOpen`, `pum.popup.afterOpen`, `pum.popup.beforeClose`, `pum.popup.afterClose`, and `pum.integration.form.success`
Applied to files:
classes/Integration/Form/HappyForms.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Hook into `pum_init` action to execute custom functionality after Popup Maker initialization
Applied to files:
classes/Integration/Form/HappyForms.phpassets/js/src/integration/happyforms.jsassets/js/src/integration/elementor.jsclasses/Integration/Form/KaliForms.phpclasses/Integration/Form/Newsletter.phpassets/js/src/integration/newsletter.jsclasses/Controllers/Compatibility/Builder/Divi.phpassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:04.308Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-js-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:04.308Z
Learning: Applies to **/*.{js,ts,tsx,jsx} : Use the global `window.PUM` object as the primary way to interact with popups on the frontend. Available methods include: `PUM.open()`, `PUM.close()`, `PUM.getPopup()`, `PUM.getSettings()`, `PUM.getSetting()`, `PUM.getCookie()`, `PUM.setCookie()`, `PUM.clearCookie()`, `PUM.clearCookies()`, and `PUM.checkConditions()`
Applied to files:
assets/js/src/integration/happyforms.jsassets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.jsassets/js/src/integration/forminator.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `pum_registered_conditions` filter to register new popup targeting conditions with custom configuration
Applied to files:
includes/integrations/class-pum-woocommerce-integration.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Modern Popup Maker admin interface uses React-based architecture with `wordpress/data` stores. Be aware of this direction for future extensions
Applied to files:
assets/js/src/integration/elementor.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Hook into `pum_save_popup`, `pum_save_theme`, or `pum_save_settings` actions to respond to data persistence events
Applied to files:
assets/js/src/integration/elementor.js
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `pum_enqueue_scripts` action to properly enqueue scripts and styles for Popup Maker extensions
Applied to files:
classes/Integration/Form/KaliForms.php
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Hook into `pum_analytics_conversion` and `pum_analytics_open` actions to track and respond to popup analytics events
Applied to files:
assets/js/src/integration/kaliForms.jsclasses/Integration/Form/Newsletter.phpclasses/Controllers/WP/Dashboard.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.{js,jsx} : Use the global `PUM` object to interact with popups via JavaScript methods like `PUM.open()`, `PUM.close()`, and `PUM.getSettings()`
Applied to files:
assets/js/src/integration/kaliForms.jsassets/js/src/integration/newsletter.js
📚 Learning: 2025-12-20T07:31:31.480Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-20T07:31:31.480Z
Learning: Applies to **/*.{ts,tsx} : Prioritize `popup-maker/*` packages over `wordpress/*` when available for better optimization and consistency
Applied to files:
.release-please-config.json
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `pum_settings_fields`, `pum_popup_settings_fields`, and `pum_theme_settings_fields` filters to modify admin field configurations
Applied to files:
classes/Admin/Settings.php
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `PUM_Utils_Options` static methods (`get_all()`, `get()`, `update()`, `update_all()`, `delete()`) for the core settings API
Applied to files:
classes/Admin/Settings.php
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `pum_get_option()` and `pum_update_option()` functions for accessing and modifying Popup Maker settings
Applied to files:
classes/Admin/Settings.php
📚 Learning: 2025-12-20T07:32:32.164Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-php-api-reference.mdc:0-0
Timestamp: 2025-12-20T07:32:32.164Z
Learning: Applies to *.php : Use `$popup->is_enabled()` to check if a popup is enabled, and `$popup->is_loadable()` to check if it should load on the current page
Applied to files:
classes/Controllers/Compatibility/Builder/Divi.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : Use the `PUM_Extension_Activation` class found in `/includes/pum-sdk/` instead of the deprecated `PUM_Activator` class
Applied to files:
classes/Extension/License.php
📚 Learning: 2025-12-20T07:31:54.054Z
Learnt from: CR
Repo: PopupMaker/Popup-Maker PR: 0
File: .cursor/rules/pm-best-practices.mdc:0-0
Timestamp: 2025-12-20T07:31:54.054Z
Learning: Applies to **/*.php : DO NOT use the older `popmake_` prefix. This is for legacy code being phased out. Use `pum_` equivalent instead
Applied to files:
classes/Extension/License.php
🧬 Code graph analysis (10)
bin/extract-changelog.js (1)
bin/update-versions.js (1)
version(23-23)
classes/Integration/Form/Forminator.php (2)
classes/Abstract/Integration/Form.php (2)
should_process_submission(86-91)increase_conversion(74-78)includes/functions/developers.php (1)
pum_integrated_form_submission(52-70)
classes/Integration/Form/HappyForms.php (2)
classes/Abstract/Integration/Form.php (3)
PUM_Abstract_Integration_Form(12-92)should_process_submission(86-91)increase_conversion(74-78)includes/functions/developers.php (1)
pum_integrated_form_submission(52-70)
classes/Integration/Form/Elementor.php (2)
classes/Abstract/Integration/Form.php (2)
should_process_submission(86-91)increase_conversion(74-78)includes/functions/developers.php (1)
pum_integrated_form_submission(52-70)
classes/Integrations.php (6)
classes/Integration/Form/Newsletter.php (1)
PUM_Integration_Form_Newsletter(25-121)classes/Integration/Form/WPForms.php (1)
PUM_Integration_Form_WPForms(9-112)classes/Integration/Form/BricksBuilder.php (1)
PUM_Integration_Form_BricksBuilder(9-217)classes/Integration/Form/Elementor.php (1)
PUM_Integration_Form_Elementor(9-233)classes/Integration/Form/KaliForms.php (1)
PUM_Integration_Form_KaliForms(9-195)classes/Integration/Form/HappyForms.php (1)
PUM_Integration_Form_HappyForms(9-175)
includes/integrations/class-pum-woocommerce-integration.php (3)
classes/Integrations.php (1)
register_conditions(214-230)classes/Conditions.php (1)
register_conditions(486-531)packages/i18n/src/index.ts (1)
__(29-32)
classes/Integration/Form/KaliForms.php (2)
classes/Abstract/Integration/Form.php (3)
PUM_Abstract_Integration_Form(12-92)should_process_submission(86-91)increase_conversion(74-78)includes/functions/developers.php (1)
pum_integrated_form_submission(52-70)
assets/js/src/integration/kaliForms.js (1)
tests/unit/js/setup.js (1)
event(203-203)
classes/Integration/Form/Newsletter.php (2)
classes/Abstract/Integration/Form.php (3)
PUM_Abstract_Integration_Form(12-92)should_process_submission(86-91)increase_conversion(74-78)includes/functions/developers.php (1)
pum_integrated_form_submission(52-70)
classes/Controllers/WP/Dashboard.php (1)
classes/Plugin/Core.php (1)
get_permission(364-368)
🪛 actionlint (1.7.10)
.github/workflows/ci.yml
124-124: input "cache" is not defined in action "shivammathur/setup-php@v2". available inputs are "coverage", "extensions", "github-token", "ini-file", "ini-values", "php-version", "php-version-file", "tools"
(action)
320-320: input "cache" is not defined in action "shivammathur/setup-php@v2". available inputs are "coverage", "extensions", "github-token", "ini-file", "ini-values", "php-version", "php-version-file", "tools"
(action)
335-335: constant expression "false" in condition. remove the if: section
(if-cond)
359-359: input "cache" is not defined in action "shivammathur/setup-php@v2". available inputs are "coverage", "extensions", "github-token", "ini-file", "ini-values", "php-version", "php-version-file", "tools"
(action)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
5-5: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
13-13: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
17-17: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 PHPMD (2.15.0)
classes/Admin/BlockEditor.php
68-68: Avoid unused parameters such as '$hook'. (undefined)
(UnusedFormalParameter)
classes/Integration/Form/Forminator.php
120-120: Avoid unused parameters such as '$entry'. (undefined)
(UnusedFormalParameter)
120-120: Avoid unused parameters such as '$field_data_array'. (undefined)
(UnusedFormalParameter)
classes/Integration/Form/HappyForms.php
122-122: Avoid unused parameters such as '$submission'. (undefined)
(UnusedFormalParameter)
classes/Integration/Form/Elementor.php
173-173: Avoid unused parameters such as '$record'. (undefined)
(UnusedFormalParameter)
classes/Integration/Form/Newsletter.php
76-76: Avoid unused parameters such as '$id'. (undefined)
(UnusedFormalParameter)
danieliser
force-pushed
the
go-pro-tab-enhancement
branch
from
285ace4 to
974988e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/Integration/Form/NinjaForms.php (1)
95-111: Defensively readform_idin the v3 handler.
If Ninja Forms changes the payload shape, this can trigger notices or send an empty ID. This pattern is already implemented in similar form integrations like FluentForms and aligns with the coding guideline to use defensive validation in hook callbacks for graceful handling of third-party plugin parameter changes.Proposed defensive guard
- pum_integrated_form_submission( + $form_id = isset( $form_data['form_id'] ) ? $form_data['form_id'] : null; + if ( ! $form_id ) { + return; + } + + pum_integrated_form_submission( [ 'popup_id' => $popup_id, 'form_provider' => $this->key, - 'form_id' => $form_data['form_id'], + 'form_id' => $form_id, ] );
🤖 Fix all issues with AI agents
In `@assets/js/src/integration/bitform.js`:
- Around line 31-39: Validate the Bit Form identifier before splitting: ensure
event.detail.formId matches the expected pattern (e.g., starts with "bitforms_"
and contains the expected number of underscore-separated segments) and only then
derive fullIdentifier, parts, formId and formInstanceId; if the pattern fails,
bail out (or skip calling window.PUM.integrations.formSubmission) and optionally
log or return early. Specifically, check event.detail.formId with a regex or
verify parts.length (from fullIdentifier.split('_')) meets expectations before
assigning formId and formInstanceId and before invoking
window.PUM.integrations.formSubmission to avoid passing invalid analytics data.
In `@assets/js/src/integration/htmlforms.js`:
- Around line 19-41: If you can't derive a real formId from formElement (the
data-id or id fallback), don't set it to 'unknown'—return/skip reporting early
instead of calling window.PUM.integrations.formSubmission; also compute the
jQuery index explicitly from $sameIdForms.index(formElement) and guard the
formInstanceId so it can never be zero or negative (e.g., set formInstanceId =
Math.max(index + 1, 1)) before passing it to formSubmission; update the code
around formId, $sameIdForms and formInstanceId accordingly.
In `@classes/Admin/Settings.php`:
- Around line 769-776: Implement the missing
PUM_Admin_Helpers::get_detected_integrations() method: add it to the
PUM_Admin_Helpers class and have it detect active integrations used by
field_go_pro_features() by checking known feature flags/classes for WooCommerce
(e.g. class_exists('WooCommerce') or similar), EDD (e.g.
class_exists('Easy_Digital_Downloads') or EDD constant), and LifterLMS (e.g.
class_exists('LifterLMS')); return an array keyed by the integration slugs
('woocommerce', 'edd', 'lifterlms') for those detected (e.g. set key => true or
include the slug in the returned array) so field_go_pro_features() can safely
inspect $integrations['woocommerce'], $integrations['edd'], and
$integrations['lifterlms'].
In `@classes/Integration/Form/BitForm.php`:
- Around line 147-168: The current negative caching stores null but
get_transient() treats null/false as "no transient", so update the caching to
use a sentinel wrapper (e.g., store an array or object like ['not_found' =>
true] under $cache_key) and adjust the retrieval logic around get_transient() to
check for that sentinel before hitting the DB; specifically modify the code
around $cache_key, $cached, get_transient(), set_transient() and the $form null
branch so you set the sentinel on miss and return null when the sentinel is
found instead of re-querying.
🧹 Nitpick comments (16)
assets/js/src/site/plugins/pum-url-tracking.js (2)
90-102: Consider additional guards forshouldTrackClick.The function currently tracks all non-CTA links including
#anchors andjavascript:void(0)links. These are often used for UI interactions rather than meaningful conversions.Consider filtering out these low-value click events:
💡 Suggested enhancement
shouldTrackClick: function ( url ) { // Skip empty URLs. if ( ! url ) { return false; } // Skip links already tracked via CTA system. if ( url.indexOf( 'cta=' ) !== -1 ) { return false; } + // Skip anchor-only links (often UI interactions). + if ( url === '#' || /^#[\w-]*$/.test( url ) ) { + return false; + } + + // Skip javascript: protocol (UI handlers, not real navigation). + if ( url.indexOf( 'javascript:' ) === 0 ) { + return false; + } + return true; },
110-127:getLinkTypemay misclassify internal absolute URLs as 'external'.At line 123-124, any URL starting with
httpor//returns'external', even if it points to the same domain. This could cause internal links written as absolute URLs (e.g.,https://example.com/page) to be labeled as external.Consider aligning this with
isInternalUrllogic or accepting the mismatch for analytics segmentation purposes.💡 Suggested enhancement for accurate categorization
getLinkType: function ( url ) { if ( url.indexOf( 'mailto:' ) === 0 ) { return 'mailto'; } if ( url.indexOf( 'tel:' ) === 0 ) { return 'tel'; } if ( url.indexOf( 'javascript:' ) === 0 ) { return 'javascript'; } if ( url === '#' || url.indexOf( '#' ) === 0 ) { return 'anchor'; } - if ( url.indexOf( 'http' ) === 0 || url.indexOf( '//' ) === 0 ) { - return 'external'; + if ( url.indexOf( 'http' ) === 0 || url.indexOf( '//' ) === 0 ) { + // Check if it's actually internal + if ( this.isInternalUrl( url ) ) { + return 'internal'; + } + return 'external'; } return 'other'; },classes/Integration/Form/FluentForms.php (1)
96-99: Consider suppressing the unused parameter warnings.Static analysis flags
$submission_idand$form_dataas unused. Since these are passed by thefluentform/before_submission_confirmationhook and cannot be removed from the signature, you may want to add a suppression comment.🔧 Optional: Add phpcs/phpmd suppression
- public function on_success( $submission_id, $form_data, $form ) { + // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundBeforeLastUsed -- Hook signature requires these parameters. + public function on_success( $submission_id, $form_data, $form ) {classes/Services/LinkClickTracking.php (2)
99-108: Action hook should usepum_prefix for consistency.Per coding guidelines, public hooks should use the
pum_prefix. The existing analytics hooks (pum_analytics_conversion,pum_analytics_open) follow this pattern. Consider renaming for consistency.Suggested fix
- do_action( 'popup_maker/link_click_tracked', $popup_id, $event_data ); + do_action( 'pum_link_click_tracked', $popup_id, $event_data );Based on learnings: existing analytics hooks use
pum_analytics_conversionandpum_analytics_openpatterns.
119-147: Atomic increment is solid, but consider checking query success.The atomic SQL update pattern correctly prevents race conditions. The TOCTOU window between
$existscheck andadd_option()is safe sinceadd_option()is idempotent when the option already exists.However,
$wpdb->query()returnsfalseon failure. Silently continuing and returning potentially stale data could mask database issues.Optional: Add error handling for robustness
// Atomic increment (prevents race condition). - $wpdb->query( + $result = $wpdb->query( $wpdb->prepare( 'UPDATE %i SET option_value = option_value + 1 WHERE option_name = %s', $wpdb->options, self::SITE_COUNT_KEY ) ); + if ( false === $result && defined( 'WP_DEBUG' ) && WP_DEBUG ) { + error_log( '[Popup Maker] Failed to increment site link click count.' ); + } + wp_cache_delete( self::SITE_COUNT_KEY, 'options' );CHANGELOG.md (1)
5-24: Use proper heading syntax for section titles.Markdownlint MD036 flags bold text used as headings. Switch to
###headings for consistency.♻️ Proposed fix
-**Features** +### Features ... -**Improvements** +### Improvements ... -**Fixes** +### Fixesclasses/Services/FormConversionTracking.php (1)
122-174: Consider adding duplicate tracking prevention for AJAX path.The non-AJAX path (
track_form_conversion) checks$args['tracked']to prevent duplicates, but this method doesn't have similar protection. If a form submission somehow triggers bothpum_integrated_form_submissionandpum_analytics_conversion, it could be double-counted.This may be intentional if the two paths are guaranteed to be mutually exclusive, but documenting this assumption or adding a similar safeguard would improve robustness.
package.json (1)
174-189: Consider simplifying lint-staged commands.The current pattern runs format, then lint with
--fix, then lint again. This works but has redundancy:
wp-scripts formatandwp-scripts lint-js --fixmay both modify formatting- Running lint twice (with and without
--fix) is intentional to catch unfixable issues, which is validIf
wp-scripts formatand lint rules are aligned, you could simplify to justlint --fixfollowed bylint. Otherwise, this pattern is acceptable.♻️ Optional simplification
"lint-staged": { "*.php": [ "composer format", "composer lint" ], "*.{js,jsx,ts,tsx}": [ - "wp-scripts format", "wp-scripts lint-js --fix", "wp-scripts lint-js" ], "*.{scss,css}": [ - "wp-scripts format", "wp-scripts lint-style --fix", "wp-scripts lint-style" ] }classes/Admin/Templates.php (1)
991-991: Inconsistent translation function usage with line 1193.Line 991 uses
esc_attr_e()for the fallback, but line 1193 uses__(). For consistency and proper escaping in JS attribute context, both should useesc_attr_e().♻️ Make line 1193 consistent
- save_button: pum_admin_vars.I10n.add || '<?php __( 'Add', 'popup-maker' ); ?>' + save_button: pum_admin_vars.I10n.add || '<?php esc_attr_e( 'Add', 'popup-maker' ); ?>'Note: Line 1193 also has a bug where
__()returns but doesn't echo. It should beesc_attr_e()to actually output the fallback.includes/namespaced/core.php (1)
120-143: Prefixgenerate_upgrade_urlto match pum_ conventions.*This is a new public helper but doesn’t use the required
pum_/PUM_prefix. Consider renaming and updating call sites accordingly.♻️ Proposed rename
-function generate_upgrade_url( $medium, $campaign, $content = '' ) { +function pum_generate_upgrade_url( $medium, $campaign, $content = '' ) { $utm_args = [ 'utm_source' => 'plugin', 'utm_medium' => sanitize_key( $medium ), 'utm_campaign' => sanitize_key( $campaign ), ]; @@ return get_upgrade_link( $utm_args ); }As per coding guidelines, use
pum_prefixes for all public functions and hooks in PHP.classes/Upsell.php (1)
198-207: Remove unused$has_eddvariable.It’s computed but never used.
🧹 Suggested cleanup
- $has_woocommerce = in_array( 'WooCommerce', $platforms, true ); - $has_edd = in_array( 'Easy Digital Downloads', $platforms, true ); + $has_woocommerce = in_array( 'WooCommerce', $platforms, true );classes/Integration/Form/Elementor.php (2)
197-211: Consider clearing cache on form creation/update, not just submissions.The cache is cleared only when a new form submission occurs (
elementor_pro/forms/new_record). If an admin creates or renames a form without any submissions, it won't appear in the list until a submission happens or the 1-hour cache expires.Consider also hooking into post save/update actions for Elementor templates to refresh the cache when forms are modified.
79-98: Use Elementor's officially documented Form Actions API instead of internal database query class.
\ElementorPro\Modules\Forms\Submissions\Database\Queryis an internal, undocumented implementation detail that Elementor does not officially support. While the code safely guards against its absence viaclass_exists(), this approach remains fragile to Elementor Pro version updates. Instead, leverage Elementor's publicly documented Form Actions extension point with therun( $record, $ajax_handler )method signature, which is the officially supported way for developers to hook into form submissions.assets/js/src/integration/beaverbuilder.js (1)
32-39: Clarify the success detection logic.The condition
data.error !== false && data.erroris a bit convoluted. Based on the comments, you want to return (skip processing) when there's an error. The current logic:
- Returns if
data.error !== falseANDdata.erroris truthyThis works but could be clearer:
♻️ Suggested clarification
// Check for success (both forms use response.data). const data = response.data || {}; - // Contact form: data.error === false - // Subscribe form: !data.error - if ( data.error !== false && data.error ) { + // Contact form returns data.error = false on success. + // Subscribe form returns data.error as undefined/falsy on success. + // If data.error is explicitly true or a truthy error message, skip. + if ( data.error === true || ( typeof data.error === 'string' && data.error ) ) { return; // Form had errors. }classes/Integration/Form/KaliForms.php (1)
51-71: Consider limitingpost_statusto published forms only.The
get_forms()query doesn't specifypost_status, so it may return draft, pending, or trashed forms that shouldn't appear in admin dropdowns.💡 Suggested improvement
public function get_forms() { $forms = get_posts( [ 'post_type' => 'kaliforms_forms', 'posts_per_page' => -1, + 'post_status' => 'publish', 'orderby' => 'title', 'order' => 'ASC', ] );classes/Integration/Form/HTMLForms.php (1)
90-100: Consider consistent return format withget_forms().
get_form()returns the raw HTML Forms object whileget_forms()returns transformed arrays withidandtitlekeys. This inconsistency could confuse consumers.💡 Suggested improvement for consistency
public function get_form( $id ) { if ( ! $this->enabled() ) { return null; } try { - return hf_get_form( $id ); + $form = hf_get_form( $id ); + if ( ! $form ) { + return null; + } + return [ + 'id' => $form->ID, + 'title' => $form->title, + ]; } catch ( Exception $e ) { return null; } }
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Go Pro upsell feature to admin settings: two field renderers (hero and feature bars), client-side injection/dismissal logic with localStorage, integration detection helper, SCSS for the UI, and a small license handling default initialization. (32 words) Changes
Sequence Diagram(s)sequenceDiagram
participant AdminBrowser as Admin Browser
participant Settings as PUM_Admin_Settings
participant Helpers as PUM_Admin_Helpers
participant License as LicenseService
participant URLGen as UpgradeURLGen
AdminBrowser->>Settings: request settings page
Settings->>Helpers: get_detected_integrations()
Helpers-->>Settings: integrations array
Settings->>License: check license status
License-->>Settings: active / inactive
alt license inactive
Settings->>URLGen: generate_upgrade_url(integration/context)
URLGen-->>Settings: upgrade URL
Settings-->>AdminBrowser: render hidden template + inline script (hero + feature bars + upgrade links)
AdminBrowser->>AdminBrowser: on pum_init clone template into panels, show in go-pro panels, persist dismiss (localStorage, 14d), handle tab clicks
else license active
Settings-->>AdminBrowser: render manage-license messaging instead of upsell
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
classes/Admin/Settings.php (1)
771-771:⚠️ Potential issue | 🔴 CriticalUndefined static helper call will break Go Pro tab rendering.
Line 771 calls
PUM_Admin_Helpers::get_detected_integrations(), but this method is unresolved (also confirmed by static analysis), which can cause a fatal error when this section is rendered.💡 Minimal defensive patch at call site
- $integrations = PUM_Admin_Helpers::get_detected_integrations(); + $integrations = method_exists( 'PUM_Admin_Helpers', 'get_detected_integrations' ) + ? PUM_Admin_Helpers::get_detected_integrations() + : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` at line 771, The call to PUM_Admin_Helpers::get_detected_integrations() is unresolved and can cause a fatal error; guard the call by checking class_exists('PUM_Admin_Helpers') && method_exists('PUM_Admin_Helpers','get_detected_integrations') before invoking it and if the check fails assign a safe default (e.g. an empty array) to $integrations so the Go Pro tab can render without crashing; update the code around the $integrations assignment to perform this defensive check and use the fallback when the helper is not available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@classes/Admin/Settings.php`:
- Line 771: The call to PUM_Admin_Helpers::get_detected_integrations() is
unresolved and can cause a fatal error; guard the call by checking
class_exists('PUM_Admin_Helpers') &&
method_exists('PUM_Admin_Helpers','get_detected_integrations') before invoking
it and if the check fails assign a safe default (e.g. an empty array) to
$integrations so the Go Pro tab can render without crashing; update the code
around the $integrations assignment to perform this defensive check and use the
fallback when the helper is not available.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/Admin/Helpers.php`:
- Around line 430-445: The get_detected_integrations() method currently only
sets 'lifterlms'; add LearnDash detection by checking for LearnDash indicators
(e.g., class_exists('SFWD_LMS') || function_exists('learndash')) and set an
integrations['learndash'] = true entry when found so the Admin/Settings consumer
(which checks integration keys like 'lifterlms') can also detect LearnDash for
contextual LMS messaging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45cb6816-efdd-4161-b194-2162f30eb472
📒 Files selected for processing (1)
classes/Admin/Helpers.php
danieliser
force-pushed
the
go-pro-tab-enhancement
branch
from
979e58c to
9edbc09
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
classes/Admin/Helpers.php (1)
430-446: Consider consolidating integration detection into one shared source.Line 430 adds a detection map that overlaps with
classes/Upsell.php:75-110; keeping both can drift and cause inconsistent contextual messaging over time. Prefer reusing one helper in both places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Helpers.php` around lines 430 - 446, Refactor duplicate integration-detection logic by making get_detected_integrations the single source of truth and invoking it from the Upsell code instead of duplicating checks; update the Upsell class/method that currently performs its own checks (the code block in Upsell around the integration detection) to call Admin\Helpers::get_detected_integrations() and use the returned map, removing the duplicated class_exists/defined/function_exists checks so both places rely on the same helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@classes/Admin/Helpers.php`:
- Around line 430-446: Refactor duplicate integration-detection logic by making
get_detected_integrations the single source of truth and invoking it from the
Upsell code instead of duplicating checks; update the Upsell class/method that
currently performs its own checks (the code block in Upsell around the
integration detection) to call Admin\Helpers::get_detected_integrations() and
use the returned map, removing the duplicated
class_exists/defined/function_exists checks so both places rely on the same
helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 490daa05-1d62-4fe5-a00f-64da5348c5ec
📒 Files selected for processing (2)
classes/Admin/Helpers.phpclasses/Admin/Settings.php
✅ Files skipped from review due to trivial changes (1)
- classes/Admin/Settings.php
There was a problem hiding this comment.
🧹 Nitpick comments (2)
classes/Admin/Settings.php (2)
726-761: Inline styles are acceptable here, but consider extracting to admin CSS if this pattern expands.The hero section implementation is correct with proper escaping. The inline styles work for this self-contained admin component, though if more styled components are added to the Go Pro tab, consolidating styles into a dedicated admin stylesheet would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` around lines 726 - 761, The hero markup in field_go_pro_hero() currently uses extensive inline styles; extract these into a dedicated admin stylesheet and replace inline style attributes with class names (e.g., keep classes pum-go-pro-hero and pum-social-proof and add more specific classes as needed) and update field_go_pro_hero() to output only semantic classes and content (preserve esc_html_e calls), then enqueue the new admin CSS from the appropriate admin hook so the styling lives in a CSS file rather than inline.
833-869: Ecommerce pitch takes precedence over LMS pitch by design.The
elseif ( $has_lms )on line 854 means users with both ecommerce and LMS plugins will only see the ecommerce pitch. If this is intentional (prioritizing ecommerce upsell), this is fine. If both pitches should potentially show, the logic would need adjustment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` around lines 833 - 869, The current conditional uses elseif ( $has_lms ) so when both $has_ecommerce and $has_lms are true only the ecommerce pitch displays; if you want both pitches to show, change the control flow to separate conditionals (use independent if ( $has_ecommerce ) { ... } and if ( $has_lms ) { ... } blocks) rather than if (...) { ... } elseif (...) { ... } so both the Pro+ ecommerce and Pro+ LMS blocks (the markup within the $has_ecommerce and $has_lms sections) can render when both flags are true; if ecommerce should still be prioritized but LMS shown in some cases, implement explicit prioritization logic (e.g., render ecommerce first and then conditionally render LMS based on an additional flag or user preference).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@classes/Admin/Settings.php`:
- Around line 726-761: The hero markup in field_go_pro_hero() currently uses
extensive inline styles; extract these into a dedicated admin stylesheet and
replace inline style attributes with class names (e.g., keep classes
pum-go-pro-hero and pum-social-proof and add more specific classes as needed)
and update field_go_pro_hero() to output only semantic classes and content
(preserve esc_html_e calls), then enqueue the new admin CSS from the appropriate
admin hook so the styling lives in a CSS file rather than inline.
- Around line 833-869: The current conditional uses elseif ( $has_lms ) so when
both $has_ecommerce and $has_lms are true only the ecommerce pitch displays; if
you want both pitches to show, change the control flow to separate conditionals
(use independent if ( $has_ecommerce ) { ... } and if ( $has_lms ) { ... }
blocks) rather than if (...) { ... } elseif (...) { ... } so both the Pro+
ecommerce and Pro+ LMS blocks (the markup within the $has_ecommerce and $has_lms
sections) can render when both flags are true; if ecommerce should still be
prioritized but LMS shown in some cases, implement explicit prioritization logic
(e.g., render ecommerce first and then conditionally render LMS based on an
additional flag or user preference).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96ad8b62-c179-4ff4-b0dc-e98331c0d367
📒 Files selected for processing (1)
classes/Admin/Settings.php
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
assets/js/src/admin/general/styles/partials/_go-pro.scss (1)
104-194: Add a narrow-width fallback for the hero and pitch bars.This layout never collapses: the hero stays side-by-side, the feature list stays two columns, the stats column cannot shrink, and the CTA/title text does not wrap. On smaller admin widths that will get cramped fast.
📱 Suggested responsive fallback
+@media screen and (max-width: 782px) { + .pum-go-pro-hero__body { + flex-direction: column; + } + + .pum-go-pro-hero__features { + grid-template-columns: 1fr; + } + + .pum-go-pro-hero__stats { + width: 100%; + flex-direction: row; + justify-content: space-between; + padding-left: 0; + padding-top: 16px; + border-left: 0; + border-top: 1px solid `#475569`; + } + + .pum-proplus-bar__title { + flex-wrap: wrap; + } + + .pum-go-pro-hero__cta, + .pum-proplus-bar__cta { + white-space: normal; + } +}Also applies to: 223-249, 327-409
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/src/admin/general/styles/partials/_go-pro.scss` around lines 104 - 194, The hero layout lacks a narrow-width responsive fallback—add a media query (e.g., max-width: 720px) that targets .pum-go-pro-hero__body and switches it to flex-direction: column with gap adjusted, make .pum-go-pro-hero__features switch grid-template-columns to 1fr and reduce gap, allow text to wrap by removing no-wrap constraints on .pum-go-pro-hero__title and .pum-go-pro-hero__tagline (ensure min-width: 0 remains on .pum-go-pro-hero__main), and make .pum-go-pro-hero__stats able to shrink/remove or move the left border/padding so it stacks under the main content; ensure .pum-go-pro-hero__logo and .pum-go-pro-hero__pro-badge scale (max-width/height or reduced padding) so CTAs and titles don't overflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@classes/Admin/Settings.php`:
- Around line 793-855: The PHP/HTML template mixes short open/close tags and has
blank lines inside control structures causing PHPCS failures; locate the block
that checks $is_pro_active and the nested $has_ecommerce and $has_lms branches
(look for the if ( ! $is_pro_active ) : and the if ( $has_ecommerce ) : / if (
$has_lms ) : sections and the generate_upgrade_url calls) and refactor so all
PHP control statements are on their own lines (use <?php if (...) : ?> on one
line and <?php endif; ?> on its own line), remove extra blank lines inside the
if/endif blocks, and avoid placing ?> / <?php tokens on the same line as HTML to
comply with WordPress PHPCS rules.
- Around line 946-952: The template pum-pro-upsell-tpl is currently wrapped by
the license inactive guard so field_go_pro_hero() and field_go_pro_features()
never render for Pro users; move rendering of the <template>
(pum-pro-upsell-tpl) and calls to field_go_pro_hero() and
field_go_pro_features() out of the is_license_active() conditional so the
Pro-user copy is always present, and keep only the extra upsell injection (the
cross-tab banner) inside the license-inactive branch so it appears for Free
users only; also correct PHPCS indentation on the echo lines that call
field_go_pro_hero() and field_go_pro_features() to match project coding
standards (align opening PHP tags and function calls to the surrounding
indentation).
---
Nitpick comments:
In `@assets/js/src/admin/general/styles/partials/_go-pro.scss`:
- Around line 104-194: The hero layout lacks a narrow-width responsive
fallback—add a media query (e.g., max-width: 720px) that targets
.pum-go-pro-hero__body and switches it to flex-direction: column with gap
adjusted, make .pum-go-pro-hero__features switch grid-template-columns to 1fr
and reduce gap, allow text to wrap by removing no-wrap constraints on
.pum-go-pro-hero__title and .pum-go-pro-hero__tagline (ensure min-width: 0
remains on .pum-go-pro-hero__main), and make .pum-go-pro-hero__stats able to
shrink/remove or move the left border/padding so it stacks under the main
content; ensure .pum-go-pro-hero__logo and .pum-go-pro-hero__pro-badge scale
(max-width/height or reduced padding) so CTAs and titles don't overflow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1251707d-8543-4e06-9acd-b501f4ad2abb
📒 Files selected for processing (4)
assets/js/src/admin/general/styles.scssassets/js/src/admin/general/styles/partials/_go-pro.scssassets/js/src/admin/settings-page/editor.scssclasses/Admin/Settings.php
💤 Files with no reviewable changes (1)
- assets/js/src/admin/settings-page/editor.scss
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/Admin/Settings.php (1)
150-153:⚠️ Potential issue | 🟠 MajorUse a failure message when
remove_license()returns false.
$succeededalready drives the redirect/error state, but Line 152 always sets success copy. A failed delete currently produces an error notice that says the key was deleted successfully.🛠️ Suggested fix
case 'delete': $succeeded = $license_service->remove_license(); - $message = __( 'License key deleted successfully!', 'popup-maker' ); + $message = $succeeded + ? __( 'License key deleted successfully!', 'popup-maker' ) + : __( 'License key deletion failed.', 'popup-maker' ); break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` around lines 150 - 153, In the case 'delete' block in Settings.php, don't unconditionally set the success message; instead check the boolean returned by $license_service->remove_license() (stored in $succeeded) and set $message to the success string only when $succeeded is true, otherwise set $message to an appropriate failure string (e.g. 'Failed to delete license key') so the redirect/error state driven by $succeeded matches the copy shown to users.
♻️ Duplicate comments (2)
classes/Admin/Settings.php (2)
946-1011:⚠️ Potential issue | 🟠 MajorThis outer license guard makes the Pro-only branch dead in this page flow.
In this file,
field_go_pro_features()is only rendered from the template insideif ( ! \PopupMaker\plugin( 'license' )->is_license_active() ). Active-license users never reach theelsebranch at Lines 850-854, so the new manage-license copy never appears. This block is also where the remaining indentation failures at Lines 948 and 1010 come from.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` around lines 946 - 1011, The upsell template and its call to self::field_go_pro_features() are currently wrapped inside if ( ! \PopupMaker\plugin('license')->is_license_active() ), which prevents the Pro-only branch (the manage-license copy rendered in the else at lines ~850-854) from ever being reached; move the <template id="pum-pro-upsell-tpl"> block (and its script) out of that outer license guard so the template is always present, then use the existing JS (KEY/localStorage and isGoProPanel) or server-side checks to hide/show the banner for active license holders; specifically update Settings.php to render the template and script unconditionally while keeping the license-specific rendering logic in field_go_pro_hero() / field_go_pro_features() and the manage-license else branch intact.
793-855:⚠️ Potential issue | 🟠 MajorRefactor this alternate PHP block so PHPCS passes.
The
if/endifbranches still mix?>/<?phpwith markup on the same lines and keep blank lines inside the control structure, which is why Code Quality is still red on this method.As per coding guidelines: Follow WordPress coding standards enforced by PHPCS.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@classes/Admin/Settings.php` around lines 793 - 855, The alternate PHP block around the is_pro_active conditional mixes short open/close tags and leaves blank lines inside the control structure, violating PHPCS; refactor the template in the method that checks $is_pro_active (and branches on $has_ecommerce and $has_lms) to use consistent PHP control structures (either full <?php ... ?> with braces or keep each control statement and its HTML in separate well-formed blocks), eliminate PHP/HTML on the same lines, and remove extraneous blank lines so the if/else/endif (and the calls to \PopupMaker\generate_upgrade_url) conform to WordPress coding standards.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/js/src/admin/general/styles/partials/_go-pro.scss`:
- Around line 104-194: The layout isn’t responsive for narrow admin widths:
update the upsell styles to add a media query (narrow breakpoint) that stacks
.pum-go-pro-hero__body (change to column/flex-direction: column), make
.pum-go-pro-hero__features single-column (grid-template-columns: 1fr), allow CTA
buttons to wrap (remove nowrap on your CTA selector(s)), and make
.pum-go-pro-hero__stats stop being a fixed rail (remove/adjust padding-left,
border-left and allow full-width or move below content) while ensuring
min-width/flex-shrink rules (e.g., .pum-go-pro-hero__main,
.pum-go-pro-hero__features, .pum-go-pro-hero__stats) prevent overflow; implement
these changes inside a single media query so the current two-column layout
remains for larger viewports.
---
Outside diff comments:
In `@classes/Admin/Settings.php`:
- Around line 150-153: In the case 'delete' block in Settings.php, don't
unconditionally set the success message; instead check the boolean returned by
$license_service->remove_license() (stored in $succeeded) and set $message to
the success string only when $succeeded is true, otherwise set $message to an
appropriate failure string (e.g. 'Failed to delete license key') so the
redirect/error state driven by $succeeded matches the copy shown to users.
---
Duplicate comments:
In `@classes/Admin/Settings.php`:
- Around line 946-1011: The upsell template and its call to
self::field_go_pro_features() are currently wrapped inside if ( !
\PopupMaker\plugin('license')->is_license_active() ), which prevents the
Pro-only branch (the manage-license copy rendered in the else at lines ~850-854)
from ever being reached; move the <template id="pum-pro-upsell-tpl"> block (and
its script) out of that outer license guard so the template is always present,
then use the existing JS (KEY/localStorage and isGoProPanel) or server-side
checks to hide/show the banner for active license holders; specifically update
Settings.php to render the template and script unconditionally while keeping the
license-specific rendering logic in field_go_pro_hero() /
field_go_pro_features() and the manage-license else branch intact.
- Around line 793-855: The alternate PHP block around the is_pro_active
conditional mixes short open/close tags and leaves blank lines inside the
control structure, violating PHPCS; refactor the template in the method that
checks $is_pro_active (and branches on $has_ecommerce and $has_lms) to use
consistent PHP control structures (either full <?php ... ?> with braces or keep
each control statement and its HTML in separate well-formed blocks), eliminate
PHP/HTML on the same lines, and remove extraneous blank lines so the
if/else/endif (and the calls to \PopupMaker\generate_upgrade_url) conform to
WordPress coding standards.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3a4fc07-f054-4864-a2bf-4fcdc11cbf6f
📒 Files selected for processing (4)
assets/js/src/admin/general/styles.scssassets/js/src/admin/general/styles/partials/_go-pro.scssassets/js/src/admin/settings-page/editor.scssclasses/Admin/Settings.php
💤 Files with no reviewable changes (1)
- assets/js/src/admin/settings-page/editor.scss
✅ Files skipped from review due to trivial changes (1)
- assets/js/src/admin/general/styles.scss
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@assets/js/src/admin/general/styles/partials/_go-pro.scss`:
- Around line 485-509: The responsive media query is targeting non-existent
classes so the narrow-screen layout won’t apply; update the selectors inside the
`@media` (max-width: 960px) block to match the real class names used earlier:
change .pum-go-pro-hero__inner to .pum-go-pro-hero__body and change
.pum-go-pro-stats to .pum-go-pro-hero__stats, leaving the other selectors
(.pum-go-pro-hero__cta, .pum-proplus-bar, .pum-proplus-bar__cta) as-is so the
stacked layout works correctly on small screens.
In `@classes/Admin/Settings.php`:
- Around line 973-1048: Indentation inside the if ( $upsell_hero ||
$upsell_features ) block is incorrect; re-indent all HTML and the <script> block
(everything between the opening if and the endif;) to use three tabs (one extra
level inside the conditional) so the template id="pum-pro-upsell-tpl", the
injected JS (functions like isGoProPanel, isDismissed, updateVisibility) and the
closing endif; align with the same 3-tab level; ensure the closing "endif;"
statement is indented to match the opening conditional block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c2f84e7-6711-410a-a32b-111f7f31b49a
📒 Files selected for processing (2)
assets/js/src/admin/general/styles/partials/_go-pro.scssclasses/Admin/Settings.php
…ghts - Add hero section with social proof stats (700K+ users, 30B+ popups, 4.95★) - Add feature grid showcasing 6 key Pro features - Exit Intent Triggers - Conversion Analytics - Advanced Targeting - A/B Testing - Mobile-First Features - Form Integrations - Add contextual Pro+ pitch for ecommerce users (WooCommerce/EDD detected) - Add contextual Pro+ pitch for LMS users (LifterLMS/LearnDash detected) - All upgrade links use generate_upgrade_url() for UTM tracking - Different display for Pro users vs Free users (license management message) Implements: monday-release-quick-wins-status.md #4
- Remove LearnDash check from LMS detection logic - Only LifterLMS is currently supported - Prevents showing LMS pitch for unsupported platform
Add PUM_Admin_Helpers::get_detected_integrations() which was called in Settings::field_go_pro_features() but never defined. Detects WooCommerce, EDD, and LifterLMS for contextual Pro+ messaging on the Go Pro tab.
Replace inline-styled hero and feature cards with proper SCSS and semantic HTML. Dark hero with PM mark, green accent glows, stats, feature pills. Horizontal Pro+ pitch bars for detected integrations.
- Hero: benefit headline, "Increase My Conversion Rate" CTA, price below - Pro+ bars: show for any user with WooCommerce/EDD/LifterLMS missing addon - Benefit-driven Pro+ CTAs: "Track My Popup Revenue" / "Boost My Course Sales" - Stacked bar support when both ecom + LMS platforms detected - Dismissible via localStorage on non-Go-Pro tabs - Dismiss button adapts color for dark vs light backgrounds - Updated stats: 4,271 five-star reviews, 4.9 rating - Replaced CTA Management tag with Referrer Detection
localStorage dismiss auto-expires after 14 days, resurfacing the banners for another impression cycle.
- Fix responsive media query selectors to match actual class names - Fix PHPCS indentation inside upsell template conditional - Fix license delete showing success message on failure - Remove dead Pro-user manage-license branch (unreachable)
danieliser
force-pushed
the
go-pro-tab-enhancement
branch
from
4dae749 to
e24aa76
Compare
1 participant
Summary
Enhances the Go Pro settings tab with professional social proof, feature highlights, and contextual Pro+ pitches to improve upgrade conversion rates.
Changes
Impact
Files Changed
Testing Checklist
Related
Part of Monday release quick wins (Feature #4)
Summary by CodeRabbit
New Features
Bug Fixes
Style