-
Notifications
You must be signed in to change notification settings - Fork 2k
NL-243: Add import limit context to subscribers modal #107664
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
base: trunk
Are you sure you want to change the base?
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
…paid plans Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~3252 bytes removed 📉 [gzipped]) 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. Generated by performance advisor bot at iscalypsofastyet.com. |
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 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
SubscriberImportLimitNoticecomponent 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.
|
@copilot I found issues, see comment #107664 (comment) |
…ction Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
I've addressed both issues in commit e6378b1:
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. |
Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
Co-authored-by: robertbpugh <52668747+robertbpugh@users.noreply.github.com>
|
@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 Both free and paid implementations use the same CSS class (
The implementation is consistent across both "Add subscribers manually" and "Use a CSV file" methods. |
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
|
@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 Could you clarify:
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:
Can you share a screenshot of what you're currently seeing so I can better understand the issue? |
|
@copilot That was it. I hadn't enabled categories. They are now, and it shows correctly. |


Part of https://linear.app/a8c/issue/NL-243/add-import-limit-context-to-the-import-modal
Proposed Changes
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
/subscribers/[site-slug]and click "Add subscribers"/plans/[site-slug]Pre-merge Checklist
💡 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.