Skip to content

## Fix: Coupon Popup themes not applying their default content (themes 8 / 11 / 12 / 13) - #152

Open
ShahrearMSf wants to merge 2 commits into
masterfrom
2318
Open

## Fix: Coupon Popup themes not applying their default content (themes 8 / 11 / 12 / 13)#152
ShahrearMSf wants to merge 2 commits into
masterfrom
2318

Conversation

@ShahrearMSf

Copy link
Copy Markdown
Contributor

On a default (untouched) publish, the new Coupon Popup themes rendered incorrectly — the electronics (theme-twelve) and multi-tier (theme-thirteen) themes shipped with the wrong/empty subtitle, wrong coupon code, and missing coupon tickets, so the front end and the live builder Preview didn't match the design. This affected both Add New and Quick Builder. This PR fixes the theme-default application so every coupon theme renders as designed out of the box.

Reported issues

  1. Add New — themes twelve & thirteen don't render per design on the front end when content is left at default.
  2. Live Preview — theme-thirteen preview is broken (no subtitle, empty "OFF" ticket).
  3. Quick Builder — same default-content mismatch as Add New for theme-thirteen.

Root cause

Three compounding defects:

  • Invalid default theme keyTypes/Popup.php declared default_theme = 'popup_theme-one', but theme keys are prefixed to popup_notification_theme-one. The initial theme was invalid.
  • Conflicting field-level defaultspopup_subtitle and popup_coupon_code carried hardcoded field defaults ("Would like to get the latest news…", "GET35OFF") that pre-filled the fields and blocked each theme's own defaults.
  • Framework clobberpopup_subtitle, popup_coupon_code and popup_coupon_repeater are the only fields whose rules exclude the initial theme, so they mount after the builder applies theme defaults and re-initialize to their field default — wiping the theme value. (Fields visible at the default theme, e.g. title/content, were unaffected — which is why only these fields broke.) theme-twelve saved GET35OFF instead of ELEC 4512658; theme-thirteen lost its subtitle and saved a junk repeater row [{index, chosen, selected}] (no tickets).

The fix

  • includes/Types/Popup.phpdefault_themepopup_notification_theme-one (fully-prefixed key).
  • includes/Extensions/PopupNotification/PopupNotification.php
    • Removed the conflicting field-level defaults on popup_subtitle and popup_coupon_code.
    • Added fill_theme_defaults() — fills only empty theme-specific fields from the selected theme's defaults (a repeater row containing only framework metadata counts as empty), so user-typed content is never overridden.
    • Wired it into both save_post (persisted data + front end) and preview_settings (live builder Preview). Both builders share PostType::save_post, so Add New and Quick Builder are covered identically.

Testing / verification

All from fresh, untouched publishes on this branch:

Check theme-twelve theme-thirteen
Saved subtitle 64% And Get Discount Now
Saved coupon ELEC 4512658 ✅ (was GET35OFF) tickets #SAVE20 / #SAVE40 ✅ (was junk)
Front-end render ✅ per design ✅ per design
Live builder Preview ✅ (was broken)
Quick Builder save
  • theme-two preview also confirmed working; all 11 theme grid previews load correctly.
  • No PHP fatals; php -l clean on both changed files.
  • Fix touches PHP only — no frontend rebuild required.

cc @priyomukul bhai — please review before merge.

shuvo7670 and others added 2 commits July 28, 2026 14:14
- Correct Popup type default_theme to the prefixed key popup_notification_theme-one
  (the unprefixed 'popup_theme-one' matched no registered theme, leaving the
  initial theme invalid and breaking default application for rules-gated fields)
- Drop conflicting field-level defaults on popup_subtitle and popup_coupon_code
  that pre-filled the fields and blocked each theme's own defaults
- Apply the selected theme's defaults to empty fields at save and preview so
  coupon themes (eleven/twelve/thirteen) render as designed in Add New, Quick
  Builder, live Preview and on the front end (subtitle, coupon code, tickets)
@ShahrearMSf ShahrearMSf self-assigned this Aug 2, 2026
ShahrearMSf added a commit that referenced this pull request Aug 3, 2026
…ree release

Pre-release integration of three NotificationX Free candidates onto nx-dev
(branched from nx-dev, which was even with master) so the combined result can be
built and tested before the individual PRs merge to master. No version bump.

Cards / candidates
------------------
- 81117  BitIntegrations: unify $module ('modules_bitintegrations') and title
         across BitIntegrationsConversions / EmailSubscription / Reviews (and the
         module list in Types/Conversions + OfferAnnouncement) so the builder
         renders ONE "Bit Integrations" source section instead of two. Fix the
         doc links (bit-integrations-notification-alert, configure-discount-alert)
         - both resolve HTTP 200.
- 2318 (PR #152)  Coupon Popup: themes 8/11/12/13 now apply their own default
         content on a default/untouched publish - subtitle, coupon code, and the
         theme-thirteen coupon tickets. Fixes the Popup type default_theme key,
         drops conflicting field-level defaults, and fills empty theme fields via
         save_post + preview_settings.
- 78650 (PR #153)  Targeting: country + user-role targeting (incl. a Guest /
         logged-out option) for ALL notification types, decoupled from the bar
         into Core\Targeting. Adds visitor-country geo-lookup caching
         (request-level memo + per-IP transient + CDN header shortcut) so the
         ip-api.com lookup no longer runs on every notification load.

Merge
-----
Merge order: nx-dev -> 81117 -> 2318 -> 78650. All feature code lives in disjoint
files (BitIntegrations vs Popup vs Targeting/Core), so there were no code
conflicts - only the shared translation template collided (see below).

POT file conflict fix
---------------------
languages/notificationx.pot was the only merge conflict, because every branch
regenerates it. Resolved by regenerating a fresh union with `npm run pot`
(wp i18n make-pot) so the template contains all three features' strings.

Zip build (standard dev way; node per .nvmrc = 16)
--------------------------------------------------
- nvm use 16  (node v16.20.2, npm 8.19.4)
- rm -rf node_modules/@wordpress/compose/node_modules \
         node_modules/@wordpress/scripts/node_modules
    # required: fixes "TS2786: 'Pagination' cannot be used as a JSX component"
    # (duplicate @types/react nested under @wordpress/*)
- npm run build && npm run bb && npm run cd && npm run pot
- package honoring .distignore. `wp dist-archive .` needs wp-cli >= 2.13 and the
  build machine has 2.11, so packaging used the equivalent
  rsync --exclude-from=.distignore (identical file selection).
- artifact: notificationx-nx-dev-3.2.12.zip  (Free 3.2.12)

Testing
-------
Installed the built zip on local with Pro 3.1.5 active (Free-new + Pro compat).
0 fatals; all three features working; country targeting also verified live on a
Cloudflare host. No PR and no merge to master.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants