-
Notifications
You must be signed in to change notification settings - Fork 58
feat: metered content countdown banner #4315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a countdown banner feature for metered content gates, allowing sites to display a fixed banner showing readers how many articles they've viewed before hitting the paywall. The implementation reuses existing content gifting infrastructure for efficiency.
Key Changes:
- New countdown banner feature with configurable settings (message, button label/URL, light/dark style)
- Refactored code organization: moved editor assets to
src/content-gate/editor/subdirectory and renamedcontent-gifting.jstocontent-banner.jsto support both features - Extracted content gifting and countdown banner settings into separate reusable React components
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Updates entry points to reflect reorganized editor files and renamed shared banner asset |
| src/wizards/audience/views/setup/countdown-banner.js | New component for countdown banner configuration UI with preview |
| src/wizards/audience/views/setup/content-gifting.js | Extracted content gifting settings component from content-gating.js |
| src/wizards/audience/views/setup/content-gating.js | Refactored to use extracted components, cleaner separation of concerns |
| src/content-gate/metering.js | Adds logic to remove countdown banner when content is locked |
| src/content-gate/editor/metering-settings.js | Extracted metering settings component for reusability |
| src/content-gate/editor/editor.scss | Updates import path after file reorganization |
| src/content-gate/editor/editor.js | Refactored to use extracted MeteringSettings component, removes unused imports |
| src/content-gate/editor/block-patterns.scss | New file containing block pattern styles previously in different location |
| src/content-gate/editor/block-patterns.js | New entry point for block patterns styles |
| src/content-gate/content-banner.scss | Renamed and extended from content-gifting.scss to support both features with shared styles |
| src/content-gate/content-banner.js | Renamed from content-gifting.js, adds countdown banner functionality alongside existing gifting code |
| includes/wizards/audience/class-audience-wizard.php | Adds API endpoint handling for countdown banner settings |
| includes/content-gate/content-gifting/class-content-gifting.php | Updates asset references to use renamed content-banner files |
| includes/content-gate/class-metering.php | Adds checkout modal check to prevent metering in modal context |
| includes/content-gate/class-metering-countdown.php | New class implementing countdown banner backend functionality (settings, rendering, asset enqueuing) |
| includes/content-gate/class-content-gate.php | Includes new metering-countdown class file |
Comments suppressed due to low confidence (1)
src/content-gate/content-banner.js:8
- Incorrect operator precedence. The expression
'metering-' + settings.gate_id || 0evaluates as('metering-' + settings.gate_id) || 0due to operator precedence. Since string concatenation produces a truthy string (even'metering-undefined'), the|| 0fallback will never be reached. The intended logic should use parentheses and template literals:
const storeKey = `metering-${settings.gate_id || 0}`;💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $classes = [ $style_class ]; | ||
| $total_views = Metering::get_total_metered_views( \is_user_logged_in() ); | ||
| if ( false === $total_views ) { | ||
| return ''; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent return type. The function is declared to return void (no return type specified), but on line 163 it returns an empty string return ''. This should either return nothing or the function signature should document what it returns.
| return ''; | |
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: @return void in the docblock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in dbb794b—added @return void in the docblock and made sure to return nothing instead of an empty string.
| /** | ||
| * WooCommerce Content Gate Metering. | ||
| * | ||
| * @package Newspack | ||
| */ | ||
|
|
||
| namespace Newspack; | ||
|
|
||
| /** | ||
| * WooCommerce Content Gate Metering class. |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file header and class docblock describe this as "WooCommerce Content Gate Metering" but this class is actually for the countdown banner feature, not WooCommerce metering. The documentation should accurately describe the class purpose as "Content Gate Metering Countdown Banner" or similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in dbb794b
| * | ||
| * @param string $key Optional key to get a specific setting. If not provided, all settings will be returned. | ||
| * | ||
| * @return array Countdown settings. |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @return documentation states it returns an array but doesn't specify what type of value it could return when a $key parameter is provided. When $key is provided, it returns a single value (mixed type) from get_option(), not an array. The documentation should reflect both return types: @return array|mixed Array of all settings, or a single setting value if $key is provided.
| * @return array Countdown settings. | |
| * @return array|mixed Array of all settings, or a single setting value if $key is provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a strong opinion about this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the docblock in dbb794b
| 'cta_label' => __( 'Subscribe now and get unlimited access.', 'newspack' ), | ||
| 'button_label' => __( 'Subscribe now', 'newspack' ), |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent text domain. Lines 36-37 use 'newspack' as the text domain, but lines 192 and 207 use 'newspack-plugin'. Based on the rest of the codebase (including other files in the same directory), the correct text domain should be 'newspack-plugin'.
| 'cta_label' => __( 'Subscribe now and get unlimited access.', 'newspack' ), | |
| 'button_label' => __( 'Subscribe now', 'newspack' ), | |
| 'cta_label' => __( 'Subscribe now and get unlimited access.', 'newspack-plugin' ), | |
| 'button_label' => __( 'Subscribe now', 'newspack-plugin' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in dbb794b
| * | ||
| * @param array $settings New countdown settings. | ||
| * | ||
| * @return array|\WP_Error Updated countdown settings or error if update fails. |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The @return documentation states it returns array|\WP_Error but the function never returns a \WP_Error. It always returns the $current_settings array. Either the documentation should be updated to only indicate @return array, or error handling should be implemented to actually return a \WP_Error when appropriate.
| * @return array|\WP_Error Updated countdown settings or error if update fails. | |
| * @return array Updated countdown settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in dbb794b to actually return an error if there is one
| if ( empty( $value ) ) { | ||
| delete_option( self::OPTION_PREFIX . $key ); | ||
| $current_settings[ $key ] = $default_settings[ $key ]; | ||
| continue; | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enabled setting should be handled specially like the style setting. Using empty( $value ) on line 75 will treat boolean false as empty, which means if someone tries to disable the banner by setting enabled to false, it will instead reset to the default value (also false), but the option won't be deleted. For boolean values, explicit type checking should be used instead of empty(). Consider:
if ( $key === 'enabled' ) {
update_option( self::OPTION_PREFIX . $key, (bool) $value );
$current_settings[ $key ] = (bool) $value;
continue;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can cause issues in the future, but I'm not a fan of the proposed approach. We could either enforce the $settings to always include the entire schema and do ! isset() to delete, or skip this entirely and not handle deletion in this method. If needed, a separate delete_setting() could be implemented eventually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, in dbb794b I removed the delete_option from this for now.
| delete_option( self::OPTION_PREFIX . $key ); | ||
| $current_settings[ $key ] = $default_settings[ $key ]; | ||
| continue; | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing input sanitization. The update_settings() method receives unsanitized user input from the REST API and stores it directly using update_option(). While the style field has validation (lines 72-74), the cta_label, button_label, and cta_url fields are stored without sanitization. These should be sanitized before storing:
if ( $key === 'cta_label' || $key === 'button_label' || $key === 'cta_url' ) {
$value = sanitize_text_field( $value );
}Note that similar fields in the content gifting feature are properly sanitized (see lines 649, 652, 655 in the wizard file).
| } | |
| } | |
| if ( $key === 'cta_label' || $key === 'button_label' || $key === 'cta_url' ) { | |
| $value = sanitize_text_field( $value ); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be implemented as a separate sanitizer function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dbb794b implements a separate sanitize_setting method which should handle this as well as sanitize user input before saving to the DB.
| </ToggleGroupControl> | ||
| <div style={ { gridColumn: '1 / -1' } }> | ||
| <BaseControl id="newspack-countdown-banner-cta-preview" label={ __( 'Preview', 'newspack-plugin' ) }> | ||
| <div className="newspack-countdown-banner__cta-preview"> |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing inert attribute on the preview container. The content gifting preview (line 134 in content-gifting.js) uses inert to prevent interaction with the preview, but the countdown banner preview is missing this attribute. For consistency and to prevent users from accidentally interacting with preview links/buttons, add the inert attribute:
<div className="newspack-countdown-banner__cta-preview" inert>| <div className="newspack-countdown-banner__cta-preview"> | |
| <div className="newspack-countdown-banner__cta-preview" inert> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in dbb794b
| <span class="newspack-countdown-banner__cta__content__message newspack-ui__font--xs"> | ||
| <?php echo esc_html( $settings['cta_label'] ); ?> | ||
| <a href="#signin_modal"><?php echo esc_html( __( 'Sign in to an existing account', 'newspack-plugin' ) ); ?></a>. | ||
| </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to what was implemented for content gifting, I expected this link to be "Create an account" (#register_modal) if there's registration metering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the Countdown banner, the purpose of the countdown CTA is a bit different. It's to highlight that even though you have n "free" views remaining, you can skip ahead to get unlimited access by just purchasing a membership at anytime. The "sign in" link is for users who might already have an active subscription but just aren't logged in yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if they're not logged in, they may get more content by registering. It's a lead opportunity for readers who are not willing to subscribe.
I also just noticed that this should be behind a is_user_logged_in() condiitonal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some discussion, added some additional logic:
- If the user is NOT logged in and metering settings allow for free views for registered users, show a "Create account" link to allow the user to get additional metered views
- If the user is NOT logged in and metering settings do NOT allow for free registered views, show a "Sign in" link for users who might already have an account with a subscription
- If the user is logged in, show nothing
miguelpeixe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested well! I've added some comments inline and commented Copilot's review
|
@miguelpeixe thanks to you and Copilot for the feedback! This should be ready for another review. |
miguelpeixe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the revisions!
|
Hey @dkoo, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
# [6.28.0-alpha.1](v6.27.1...v6.28.0-alpha.1) (2025-12-11) ### Bug Fixes * **content-gating:** never gate special pages ([#4340](#4340)) ([a4dcfdd](a4dcfdd)) * **countdown-banner:** don't show on unrestricted posts ([#4349](#4349)) ([66d2c94](66d2c94)) * **indesign-export:** only register the attribute for allowed blocks ([#4330](#4330)) ([e1801cb](e1801cb)) * **my-account:** safe content argument to skip sanitization ([#4326](#4326)) ([b066de5](b066de5)) * **my-account:** set new payment method as default ([#4343](#4343)) ([6ebcaf9](6ebcaf9)) * **my-account:** support dynamic content around shortcode ([#4328](#4328)) ([36b9524](36b9524)) * **subscription-tiers-modal:** skip private products ([#4337](#4337)) ([564d803](564d803)) ### Features * **content-gate:** content rules ([#4265](#4265)) ([b5b8cd9](b5b8cd9)) * **content-gate:** implement restriction rules ([#4251](#4251)) ([4034103](4034103)) * metered content countdown banner ([#4315](#4315)) ([c9a68cc](c9a68cc)) * **payment-notice:** detect equivalent subscription ([#4333](#4333)) ([9a98889](9a98889)) * **ras:** OAuth OTP flow improvements ([#4341](#4341)) ([8b345fa](8b345fa))
|
🎉 This PR is included in version 6.28.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [6.28.0](v6.27.4...v6.28.0) (2026-01-05) ### Bug Fixes * **content-gating:** never gate special pages ([#4340](#4340)) ([a4dcfdd](a4dcfdd)) * **countdown-banner:** don't show on unrestricted posts ([#4349](#4349)) ([66d2c94](66d2c94)) * **countdown-banner:** never show more views than total ([#4369](#4369)) ([0ef3a24](0ef3a24)) * **indesign-export:** only register the attribute for allowed blocks ([#4330](#4330)) ([e1801cb](e1801cb)) * **my-account:** safe content argument to skip sanitization ([#4326](#4326)) ([b066de5](b066de5)) * **my-account:** set new payment method as default ([#4343](#4343)) ([6ebcaf9](6ebcaf9)) * **my-account:** support dynamic content around shortcode ([#4328](#4328)) ([36b9524](36b9524)) * **subscription-tiers-modal:** skip private products ([#4337](#4337)) ([564d803](564d803)) ### Features * **content-gate:** content rules ([#4265](#4265)) ([b5b8cd9](b5b8cd9)) * **content-gate:** implement restriction rules ([#4251](#4251)) ([4034103](4034103)) * metered content countdown banner ([#4315](#4315)) ([c9a68cc](c9a68cc)) * **payment-notice:** detect equivalent subscription ([#4333](#4333)) ([9a98889](9a98889)) * **ras:** OAuth OTP flow improvements ([#4341](#4341)) ([8b345fa](8b345fa))
|
🎉 This PR is included in version 6.28.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Uses a fixed banner to display metering information, intended to be used as a simpler and more global alternative to #4176.
Repurposes existing admin and front-end assets from #4207 for efficiency.
Note that this includes a newNevermind, this component is only useful when used inside the block editor due to its dependency on theColorPaletteControlscomponent—I originally created this with the intention of being able to customize banner colors, but ultimately opted to use the same "light"/"dark" toggle as the content gifting banner instead. I think the component might come in handy in the future, so I left it in, but it's unused for now.ColorPaletteControlcomponent from the@wordpress/block-editorpackage. Importing this component outside of that context borks our Jest tests, so I'll bring it back but outside of the Components package if we need it in the future.Closes NPPM-2204.
How to test the changes in this Pull Request:
Other information: