Changed membership settings dialogs to controlled components

[9larsons]

no ref

Uses the settings DialogPortal from #30014.

The custom field, welcome email and welcome email customize dialogs were the last NiceModal.create dialogs in the membership and email settings areas. They now take an onClose prop and are rendered by the component that opens them: the two SettingsModal dialogs through the settings dialog portal so they paint above the settings chrome, and the customize dialog directly, since its EmailDesignModal is already a portalled Shade Dialog.

With these converted (plus #30014 and #30015), no settings dialog is created or shown through NiceModal any more, which clears the way to remove the provider, the dependency and the compatibility bridges.

Verification

• pnpm test:acceptance src/settings/membership src/settings/email — 114/114 (custom fields, welcome emails and the customize dialog already had thorough coverage; the welcome-email save test is what caught the stacking-context problem the portal fixes)
• tsc --noEmit and eslint clean on apps/admin

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: bf2d7406-cdfa-4f5c-ac31-a1bfd8247ae2

📥 Commits

Reviewing files that changed from the base of the PR and between ad184ce and 34ca429.

📒 Files selected for processing (1)

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

Included review availability: Your plan includes up to 10 reviews per rolling hour; 4 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (2)

• GitHub Check: Setup
• GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (Custom checks)

**/*.{ts,tsx}: Type-safe boundaries: Fail only if the PR:

• consumes boundary data (HTTP input, external API/SDK responses, env/config,
  DB/filesystem reads, queue/webhook/event payloads) without validating it
  first — Zod by default, another format only where an external contract
  requires it; or
• introduces any, unchecked as, @ts-nocheck, or @ts-ignore to bypass
  typing boundary data; or
• hand-writes a type duplicating a shape a Zod schema describes (use z.infer).
  Never fail for: internal function/module calls (no runtime validation needed),
  pre-existing JS files touched incidentally, tests, scripts, or config files.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

**/*

📄 CodeRabbit inference engine (AGENTS.md)

Always use pnpm, never npm or Yarn.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

---

⚙️ CodeRabbit configuration file

**/*: Prioritise concrete correctness, security, data-integrity, compatibility,
and regression risks. Explain the failure mode and point to the affected
code. Do not report formatting, naming, import ordering, type errors, or
other findings already owned by configured static tools or failing GitHub
checks. Do not request speculative abstractions, broad refactors, generic
documentation, or tests unrelated to changed behaviour. Treat nearby
AGENTS.md files and mapped codebase documentation as authoritative; do not
enforce proposals, plans, or historical guidance as current policy.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

apps/admin/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/admin/**/*.{js,jsx,ts,tsx}: Build new features in React,
use admin-x-framework for APIs, and use Shade for UI.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

**/*.{ts,tsx,mts,cts}

⚙️ CodeRabbit configuration file

**/*.{ts,tsx,mts,cts}: Review lens: "where does this data become trusted?"

• Boundary data (HTTP input, external API/SDK responses, env/config,
  DB/filesystem reads, queue/webhook/event payloads) is unknown until
  validated — Zod by default.
• Infer boundary types via z.infer/z.input; flag handwritten duplicates.
• Flag any, unchecked as on boundary data, @ts-nocheck, and unexplained
  @ts-ignore/@ts-expect-error.
• Validated data stays trusted: don't request Zod on internal calls, and flag
  redundant re-validation.
• ghost/core golden path: schema.ts owns Zod schemas + inferred types, with
  codec/serializer modules at the edges (see core/server/services/gift-links).
• Looser typing in tests is fine unless it hides a real defect.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

apps/{admin,activitypub,admin-x-framework,shade}/**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

apps/{admin,activitypub,admin-x-framework,shade}/**/*.{ts,tsx}: Review Admin UI for existing Shade reuse, correct component layer, semantic
tokens, accessible interaction states, and whole-sentence translations. New UI
that depends on backend settings, endpoints, or config must feature-detect old
backend support and cover the not-yet-deployed backend case. Do not apply these
rules to independent public UMD apps. Do not repeat ESLint/Tailwind findings.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

**/*{.,-}{test,spec}.{js,jsx,ts,tsx}

⚙️ CodeRabbit configuration file

**/*{.,-}{test,spec}.{js,jsx,ts,tsx}: Review whether tests prove changed behaviour, meaningful error/edge paths, and
externally observable contracts without coupling to implementation details.
Prefer the lowest useful test layer. Do not demand broad E2E coverage for
isolated logic or repeat test-run failures already visible in GitHub checks.

Files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

🧠 Learnings (4)

📚 Learning: 2026-04-30T10:51:48.759Z

Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 27625
File: apps/admin/src/onboarding/onboarding-route.tsx:30-33
Timestamp: 2026-04-30T10:51:48.759Z
Learning: In the Ghost Admin React app, when you rely on data returned by `useBrowseSite()` (e.g., `site.data?.site.url` in the onboarding flow), assume the hook’s site data is already pre-loaded/cached before the route renders. In this context, the fallback to `"/"` for `site.data?.site.url` should not be treated as a practical runtime path, so avoid adding extra loading guards for `useBrowseSite()` output unless the `useBrowseSite()` preloading/caching guarantee changes.

Applied to files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

📚 Learning: 2026-07-21T19:57:01.324Z

Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 29497
File: apps/admin/src/automations/components/canvas/off-value.tsx:4-4
Timestamp: 2026-07-21T19:57:01.324Z
Learning: Admin UI in Ghost is intentionally not localized. During code review, do not request adding i18n/translation hooks, wrappers, or new locale keys (e.g., updates to `packages/i18n/locales/en/ghost.json`) for Admin UI strings, including any React components under `apps/admin/src/`.

Applied to files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

📚 Learning: 2026-06-04T15:15:20.265Z

Learnt from: JohnONolan
Repo: TryGhost/Ghost PR: 28368
File: apps/admin-x-settings/src/components/settings/site/navigation/navigation-edit-form.tsx:32-32
Timestamp: 2026-06-04T15:15:20.265Z
Learning: In this TryGhost/Ghost codebase (Tailwind CSS v4), use/accept the v4 suffix form of the important modifier in class names (e.g., `opacity-100!`, `flex!`). Do not flag these as incorrect or inconsistent with the older v3 prefix form (`!opacity-100`), since the suffix form is the established convention and aligns with the generated CSS.

Applied to files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

📚 Learning: 2026-08-08T20:30:54.860Z

Learnt from: vershwal
Repo: TryGhost/Ghost PR: 29488
File: apps/admin/src/settings/app/components/settings/advanced/labs/beta-features.tsx:39-47
Timestamp: 2026-08-08T20:30:54.860Z
Learning: When implementing custom error-toast flows in the Admin application, preserve the framework's default dismissal behavior by calling parameterless `toast.dismiss()` before showing the replacement toast. Then call `handleError(error, {withToast: false})` when framework error handling is needed without displaying its default toast.

Applied to files:

• apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx

🔇 Additional comments (1)

apps/admin/src/settings/membership/custom-fields.acceptance.test.tsx (1)

247-248: LGTM!

---

Walkthrough

Settings email and membership screens replace NiceModal with local React state. Converted modals receive explicit onClose callbacks. Custom-field and welcome-email dialogs render through DialogPortal. Archive, reactivation, deletion, save, dirty-close confirmation, and modal dismissal paths now invoke parent close handlers. WelcomeEmailModalProps is exported for local state typing.

Suggested reviewers: rob-ghost

Merge Risk: ⚪ Minimal · up to 34ca4

This change localizes membership and email settings dialogs as controlled components, with reported acceptance, TypeScript, and lint checks passing; no actionable merge-blocking risk remains beyond normal review.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the conversion of settings dialogs from NiceModal to controlled components and includes relevant verification results.
Title check	✅ Passed	The title clearly summarizes the main change: converting membership settings dialogs to controlled components.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Type-Safe Boundaries	✅ Passed	The diff only replaces NiceModal wiring with typed React state and onClose props; it adds no boundary reads, any, unchecked as, TypeScript bypasses, or schema-duplicating types.
New Files Are Typescript	✅ Passed	The PR changes seven pre-existing .tsx files; the HEAD^^..HEAD diff contains no added .js, .jsx, .cjs, or .mjs source files.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch slars/settings-membership-dialogs-controlled

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nx-cloud]

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci

---

View your CI Pipeline Execution ↗ for commit ad184ce

Command	Status	Duration	Result
nx run @tryghost/admin:test:acceptance	✅ Succeeded	6m 47s	View ↗
nx run-many -t test:unit -p @tryghost/admin	✅ Succeeded	2m 39s	View ↗
nx run ghost-monorepo:lint:boundaries	✅ Succeeded	21s	View ↗
nx run @tryghost/admin:build	✅ Succeeded	17s	View ↗
nx run-many -t lint -p @tryghost/admin,ghost-mo...	✅ Succeeded	1m 14s	View ↗
nx run-many --target=build --projects=tag:publi...	✅ Succeeded	<1s	View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗

---

☁️ Nx Cloud last updated this comment at 2026-08-18 14:49:53 UTC

[9larsons]

Reviewed in two independent passes (dialog lifecycle/stacking and tests/accessibility/async behavior), followed by a final re-review cycle after the fix.

One maintenance issue was found and fixed in 34ca4291e1: the custom-fields acceptance test still described global NiceModal cleanup even though this PR makes the modal parent-controlled. The assertion remains and now accurately documents async save completion and controlled unmount.

No remaining actionable findings.

Verification:

• pnpm nx run @tryghost/admin:test:acceptance -- src/settings/membership src/settings/email — 114/114 passed
• pnpm nx run @tryghost/admin:typecheck — passed
• Targeted ESLint — passed
• Final custom-fields acceptance rerun — 20/20 passed