Added Customize button to welcome emails behind labs flag#26770
Merged
Added Customize button to welcome emails behind labs flag#26770
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a feature-flagged "Customize" button to the member welcome emails UI that appears when the 🚥 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 |
cmraible
force-pushed
the
NY-1142-welcome-email-customize-button
branch
from
7ec1100 to
82d97c1
Compare
cmraible
force-pushed
the
NY-1142-welcome-email-customize-button
branch
from
4c66d91 to
a8907e4
Compare
ref https://linear.app/ghost/issue/NY-1142 Matched the automated email rows to newsletter-style table hover behavior and fixed width/flex layout so title and description fill available space without changing functionality.
ref https://linear.app/ghost/issue/NY-1142 Updated Membership sidebar item and section heading to use Email wording for consistency with the new email settings structure.
ref https://linear.app/ghost/issue/NY-1142 Restored automated emails as a separate Membership section, reverted newsletters section ordering and naming, and aligned sidebar labels/navigation accordingly.
The rename to "Automated emails" was premature — keeping it as "Welcome emails" for now
The button had no functionality — removing it until it's needed
Added data-testid to welcome email title for simpler test selectors, simplified section order test using toEqual, and used toBeHidden for clearer visibility assertions.
The button is gated behind the `welcomeEmailsDesignCustomization` private feature flag and currently has no functionality wired up
Opens a simple modal when clicking the Customize button (behind the welcomeEmailsDesignCustomization labs flag)
This reverts commit 1744627.
cmraible
force-pushed
the
NY-1142-welcome-email-customize-button
branch
from
a8907e4 to
7dc7fe7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsx (1)
2-2: Consider usingshadedesign system instead ofadmin-x-design-system.The coding guidelines indicate that
admin-x-design-systemis being phased out and new components should use theshadedesign system. Ifshadehas an equivalent Modal component, consider migrating to it.As per coding guidelines: "Use
shadedesign system for new components in Admin UI. Avoidadmin-x-design-systemas it is being phased out."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsx` at line 2, The import currently pulls Modal from '@tryghost/admin-x-design-system'; update the component to use the Shade design system Modal by replacing the import of Modal with the equivalent from 'shade' (or the correct Shade package name), update any prop names or usage inside welcome-email-customize-modal (the Modal component and any props like onClose, open, title, children) to match Shade's API, and run the component build/tests to ensure the new Modal's props and styling are compatible and adjust markup where prop names differ.apps/admin-x-settings/src/components/settings/membership/member-emails.tsx (1)
7-7: Same design system consideration applies here.The
Buttonimport from@tryghost/admin-x-design-systemfollows the same pattern flagged in the modal file. For consistency, if migrating toshade, both components should be updated together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin-x-settings/src/components/settings/membership/member-emails.tsx` at line 7, Update the Button import in member-emails.tsx to match the migration to the new "shade" package: replace importing Button from '@tryghost/admin-x-design-system' with the equivalent Button from the shade package (while leaving Icon, Table, TableRow, Toggle, showToast, withErrorBoundary imported from '@tryghost/admin-x-design-system'), and ensure the paired component you changed in the modal file is updated to import Button from the same shade package so both components come from the same source.e2e/tests/admin/settings/member-welcome-emails.test.ts (1)
129-142: Consider splitting into separate tests for better isolation.This test covers multiple scenarios: (1) customize button is visible, (2) clicking opens modal, and (3) close button dismisses modal. Per E2E guidelines, each test should cover one scenario for clearer failure diagnostics and test isolation.
As per coding guidelines: "One test should equal one scenario - never mix multiple scenarios in a single test."
♻️ Suggested test split
- test('customize button opens modal when labs flag is enabled', async ({page}) => { + test('customize button is visible when labs flag is enabled', async ({page}) => { const welcomeEmailsSection = new MemberWelcomeEmailsSection(page); - await welcomeEmailsSection.goto(); - await expect(welcomeEmailsSection.customizeButton).toBeVisible(); - await welcomeEmailsSection.customizeButton.click(); - - await expect(welcomeEmailsSection.customizeModal).toBeVisible(); + }); - await welcomeEmailsSection.customizeModal.getByRole('button', {name: 'Close'}).click(); + test('customize button opens modal', async ({page}) => { + const welcomeEmailsSection = new MemberWelcomeEmailsSection(page); + await welcomeEmailsSection.goto(); + await welcomeEmailsSection.customizeButton.click(); + await expect(welcomeEmailsSection.customizeModal).toBeVisible(); + }); - await expect(welcomeEmailsSection.customizeModal).toBeHidden(); + test('customize modal can be closed', async ({page}) => { + const welcomeEmailsSection = new MemberWelcomeEmailsSection(page); + await welcomeEmailsSection.goto(); + await welcomeEmailsSection.customizeButton.click(); + await welcomeEmailsSection.customizeModal.getByRole('button', {name: 'Close'}).click(); + await expect(welcomeEmailsSection.customizeModal).toBeHidden(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/admin/settings/member-welcome-emails.test.ts` around lines 129 - 142, Split the single multi-assert test into separate focused tests: (1) verify the customize button is visible by asserting MemberWelcomeEmailsSection.customizeButton isVisible(), (2) verify clicking MemberWelcomeEmailsSection.customizeButton opens the modal by clicking it and asserting MemberWelcomeEmailsSection.customizeModal isVisible(), and (3) verify the modal can be dismissed by clicking the Close button inside MemberWelcomeEmailsSection.customizeModal and asserting it becomes hidden; keep navigation via MemberWelcomeEmailsSection.goto() in each test for isolation and give each test a descriptive name like "shows customize button", "opens customize modal on click", and "closes customize modal on close button click".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsx`:
- Around line 14-19: The modal currently shows okLabel='Save' while onOk only
calls modal.remove(), which misleads users; change the okLabel to a neutral
label like 'OK' or 'Close' (e.g., okLabel='OK' or okLabel='Close') or implement
a real save handler in the component so that the label matches behavior—update
the JSX where okLabel and onOk are set (okLabel and onOk props and the
modal.remove call) to keep label and action consistent.
---
Nitpick comments:
In `@apps/admin-x-settings/src/components/settings/membership/member-emails.tsx`:
- Line 7: Update the Button import in member-emails.tsx to match the migration
to the new "shade" package: replace importing Button from
'@tryghost/admin-x-design-system' with the equivalent Button from the shade
package (while leaving Icon, Table, TableRow, Toggle, showToast,
withErrorBoundary imported from '@tryghost/admin-x-design-system'), and ensure
the paired component you changed in the modal file is updated to import Button
from the same shade package so both components come from the same source.
In
`@apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsx`:
- Line 2: The import currently pulls Modal from
'@tryghost/admin-x-design-system'; update the component to use the Shade design
system Modal by replacing the import of Modal with the equivalent from 'shade'
(or the correct Shade package name), update any prop names or usage inside
welcome-email-customize-modal (the Modal component and any props like onClose,
open, title, children) to match Shade's API, and run the component build/tests
to ensure the new Modal's props and styling are compatible and adjust markup
where prop names differ.
In `@e2e/tests/admin/settings/member-welcome-emails.test.ts`:
- Around line 129-142: Split the single multi-assert test into separate focused
tests: (1) verify the customize button is visible by asserting
MemberWelcomeEmailsSection.customizeButton isVisible(), (2) verify clicking
MemberWelcomeEmailsSection.customizeButton opens the modal by clicking it and
asserting MemberWelcomeEmailsSection.customizeModal isVisible(), and (3) verify
the modal can be dismissed by clicking the Close button inside
MemberWelcomeEmailsSection.customizeModal and asserting it becomes hidden; keep
navigation via MemberWelcomeEmailsSection.goto() in each test for isolation and
give each test a descriptive name like "shows customize button", "opens
customize modal on click", and "closes customize modal on close button click".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 577d5637-b6c9-4b65-80b2-90a579912f84
📒 Files selected for processing (4)
apps/admin-x-settings/src/components/settings/membership/member-emails.tsxapps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-customize-modal.tsxe2e/helpers/pages/admin/settings/sections/member-welcome-emails-section.tse2e/tests/admin/settings/member-welcome-emails.test.ts
Replaced legacy admin-x-design-system Modal with Shade Dialog component to align with the design system migration direction
cmraible
commented
Mar 11, 2026
| @@ -0,0 +1,23 @@ | |||
| import NiceModal, {useModal} from '@ebay/nice-modal-react'; | |||
| import {Button, Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle} from '@tryghost/shade'; | |||
Collaborator
Author
There was a problem hiding this comment.
I'm not sure exactly how this will workout, but I think our Plan A should be to build the new reusable design customization modal based on shade, to avoid further dependence on admin-x-design-system.
troyciesco
reviewed
Mar 11, 2026
| test.describe('Ghost Admin - Welcome Email Customize Button', () => { | ||
| test.use({labs: {welcomeEmailsDesignCustomization: true}}); | ||
|
|
||
| test('customize button opens modal when labs flag is enabled', async ({page}) => { |
Contributor
There was a problem hiding this comment.
maybe a second test to make sure it doesn't show when disabled? so we'd have two tests (however you want to nest them and set them up):
test.describe('Ghost Admin - Welcome Email Customize Button - flag enabled', () => {
test.use({labs: {welcomeEmailsDesignCustomization: true}});
test('customize button opens modal when labs flag is enabled', async ({page}) => {
const welcomeEmailsSection = new MemberWelcomeEmailsSection(page);
await welcomeEmailsSection.goto();
await expect(welcomeEmailsSection.customizeButton).toBeVisible();
await welcomeEmailsSection.customizeButton.click();
await expect(welcomeEmailsSection.customizeModal).toBeVisible();
await welcomeEmailsSection.customizeModal.getByRole('button', {name: 'Close'}).click();
await expect(welcomeEmailsSection.customizeModal).toBeHidden();
});
});
test.describe('Ghost Admin - Welcome Email Customize Button - flag disabled', () => {
test('customize button is hidden when labs flag is disabled', async ({page}) => {
const welcomeEmailsSection = new MemberWelcomeEmailsSection(page);
await welcomeEmailsSection.goto();
await expect(welcomeEmailsSection.customizeButton).toBeHidden();
});
});
Collaborator
Author
There was a problem hiding this comment.
Added the flag disabled test and setting this PR to automerge
troyciesco
approved these changes
Mar 11, 2026
Contributor
troyciesco
left a comment
troyciesco
left a comment
There was a problem hiding this comment.
lgtm, though i think we should add the extra test to make sure it doesn't show so we don't mistakenly move it or something
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes https://linear.app/ghost/issue/NY-1135/add-the-customize-button-and-an-empty-modal
Summary
welcomeEmailsDesignCustomizationprivate feature flag (Labs → Developer experiments)