Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 12, 2025

Part of https://linear.app/a8c/issue/NL-243/add-import-limit-context-to-the-import-modal

Proposed Changes

  • Added plan-specific import limit notices to the subscriber import forms
  • For free plans: Displays "Free plans have an import limit of 100 subscribers. [Upgrade your plan] to import unlimited subscribers."
  • For paid plans: Displays "Imports of more than 10,000 subscribers will go through a manual review before being added to your site."
  • Notices appear after the Categories section and before the disclaimer text in both "Add subscribers manually" and "Use a CSV file" methods
  • Styled as plain text paragraphs matching the existing form text style (no blue Notice component)

Why are these changes being made?

To reduce support questions by proactively informing users about subscriber import limits based on their plan type. Free plan users need to know about the 100 subscriber limit and the option to upgrade, while paid plan users should be aware of the manual review process for large imports (>10,000 subscribers).

Testing Instructions

  1. Navigate to /subscribers/[site-slug] and click "Add subscribers"
  2. Select either "Add subscribers manually" or "Use a CSV file"
  3. For free plan sites:
    • Verify the notice "Free plans have an import limit of 100 subscribers. [Upgrade your plan] to import unlimited subscribers." appears after the Categories section (if enabled) and before the disclaimer text
    • Verify the "Upgrade your plan" link navigates to /plans/[site-slug]
    • Verify the text is styled as plain text (subtle gray color, no blue border)
  4. For paid plan sites:
    • Verify the notice "Imports of more than 10,000 subscribers will go through a manual review before being added to your site." appears in the same position
    • Verify the text is styled as plain text (subtle gray color, no blue border)
  5. Note: Categories section only appears when newsletter categories are enabled and configured for the site

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…paid plans

Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
Copilot AI changed the title [WIP] Add import limit context to the import modal NL-243: Add import limit context to subscribers modal Dec 12, 2025
Copilot AI requested a review from robertbpugh December 12, 2025 14:59
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 12, 2025
@matticbot
Copy link
Contributor

matticbot commented Dec 12, 2025

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~3252 bytes removed 📉 [gzipped])

name         parsed_size           gzip_size
subscribers     -10200 B  (-0.4%)    -3411 B  (-0.7%)
import            -592 B  (-0.1%)       -6 B  (-0.0%)
people            +459 B  (+0.0%)     +165 B  (+0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@robertbpugh robertbpugh marked this pull request as ready for review December 12, 2025 15:27
Copilot AI review requested due to automatic review settings December 12, 2025 15:27
Copy link
Contributor

Copilot AI left a 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 adds contextual import limit messaging to the subscriber import modal. The notice component now displays plan-specific information for both free and paid plans, replacing the previous implementation that only showed messaging for free plans.

Key Changes:

  • Simplified SubscriberImportLimitNotice component to show appropriate messaging based on plan type
  • Free plans see a 100 subscriber limit notice with an upgrade link
  • Paid plans see information about the 10,000 subscriber manual review threshold

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
client/my-sites/subscribers/components/subscriber-import-limit-notice/index.tsx Refactored component to display plan-specific import limit notices; removed unused imports and props; simplified conditional rendering logic
client/my-sites/subscribers/components/add-subscribers-modal/add-subscribers-modal.tsx Removed closeModal prop from SubscriberImportLimitNotice component calls since it's no longer needed

After thorough review of the code changes, I found no issues that require comments. The implementation is clean, follows React and TypeScript best practices, has clear logic, and properly implements the stated requirements. The refactoring removes unnecessary complexity while adding the new paid plan messaging as intended.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@robertbpugh
Copy link

I just tested and there are issues.

  1. Here is the current implementation. You removed the Categories toggle and copy. Put it back.
CleanShot 2025-12-12 at 12 51 12@2x
  1. Here is your new implementation. The text should be in the same style as the sentence starting with "Upload a CSV..." Do not add the blue vertical line. The copy should be placed after the category section.
CleanShot 2025-12-12 at 12 47 38@2x

@robertbpugh robertbpugh marked this pull request as draft December 12, 2025 15:55
@robertbpugh
Copy link

@copilot I found issues, see comment #107664 (comment)

…ction

Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 12, 2025

@copilot I found issues, see comment #107664 (comment)

I've addressed both issues in commit e6378b1:

  1. ✅ The Categories section is now preserved - I moved the notice logic into the subscriber package forms themselves
  2. ✅ The notice now appears after the categories section with plain text styling (no blue vertical line) - matching the style of the other paragraph text in the forms

The notice text now renders between the Categories section and the disclaimer "By clicking 'Add subscribers'..." in both the manual entry and CSV upload modes.

Copilot AI and others added 2 commits December 12, 2025 16:07
Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
@robertbpugh
Copy link

@copilot I haven't been able to test the free implementation - can you check it and see how if it follows the same feedback I gave for the paid implementation?

Copy link
Contributor Author

Copilot AI commented Dec 12, 2025

@copilot I haven't been able to test the free implementation - can you check it and see how if it follows the same feedback I gave for the paid implementation?

Yes, the free plan implementation follows the same pattern as the paid plan:

Position: Appears after the Categories section, before the disclaimer
Styling: Plain text (no blue border) matching the "Upload a CSV..." paragraph style
Message: "Free plans have an import limit of 100 subscribers. [Upgrade your plan] to import unlimited subscribers."

Both free and paid implementations use the same CSS class (.add-subscriber__import-limit-notice) with identical styling:

  • Font size: 0.875rem
  • Color: subtle gray (var(--studio-gray-50))
  • Links: underlined, matching text color

The implementation is consistent across both "Add subscribers manually" and "Use a CSV file" methods.

@robertbpugh robertbpugh marked this pull request as ready for review December 12, 2025 16:16
@matticbot
Copy link
Contributor

matticbot commented Dec 12, 2025

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug copilot/add-import-limit-context-modal on your sandbox.

@robertbpugh
Copy link

@copilot I tested the paid version again and the categories are still missing

Copy link
Contributor Author

Copilot AI commented Dec 12, 2025

@copilot I tested the paid version again and the categories are still missing

The Categories section code is present in my changes (commit e6378b1). The CategoriesSection component renders when newsletterCategoriesData?.enabled is true and there are categories configured.

Could you clarify:

  1. Are you testing on a site that has newsletter categories enabled and configured?
  2. Have you rebuilt/redeployed the @automattic/subscriber package changes to your test environment?
  3. Have you cleared your browser cache after the latest deployment?

The code diff shows I only added the notice text - the CategoriesSection logic is unchanged from the original implementation. If categories aren't showing, it may be due to:

  • The site configuration (categories not enabled/configured)
  • Build/deployment not picking up the package changes
  • Browser/server caching

Can you share a screenshot of what you're currently seeing so I can better understand the issue?

@robertbpugh
Copy link

@copilot That was it. I hadn't enabled categories. They are now, and it shows correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants