Skip to content

Removed legacy settings dirty and focus contexts#29545

Merged
9larsons merged 1 commit into
mainfrom
codex/admin-settings-shade-dirty-guards
Jul 22, 2026
Merged

Removed legacy settings dirty and focus contexts#29545
9larsons merged 1 commit into
mainfrom
codex/admin-settings-shade-dirty-guards

Conversation

@9larsons

Copy link
Copy Markdown
Contributor

What changed

  • added a shared Shade DirtyConfirmDialog pattern and controller hook, with a Storybook story and unit coverage
  • migrated Settings exit guards, tips preview, welcome-email design, and legacy modal close paths off the NiceModal-based confirmIfDirty helper
  • moved legacy text fields, editors, and Settings sidebar focus tracking onto Shade's existing focus context
  • removed the duplicate dirty/focus providers and legacy dirty-confirmation implementation from admin-x-design-system
  • allowed AlertDialog consumers to raise the overlay with the content so confirmations stay above legacy Settings modals

Why

Settings was nested inside the consolidated Admin Shade provider but the legacy design-system wrapper created a second dirty/focus state tree. That split state between newer Shade controls and legacy modal/form consumers, and left duplicate confirmation implementations to maintain. This slice makes Shade the single owner while preserving the existing Stay/Leave behavior during the design-system retirement.

Validation

  • Shade unit tests: 230 existing tests plus 3 new dirty-confirmation tests passed
  • admin-x-design-system unit tests: 13 passed
  • admin-x-settings unit tests: 213 passed
  • affected Settings acceptance suite: 8 files, 102 tests passed
  • post-review acceptance rerun: layout, welcome emails, and tips; 41 tests passed
  • Shade and admin-x-design-system builds passed
  • Shade, admin-x-design-system, and admin-x-settings lint/typechecks passed
  • two independent review passes reported no significant findings

Manual validation

  • edit a Settings field, then use the top-right X: Stay keeps the edit; Leave exits Settings
  • repeat with Escape and confirm the same Stay/Leave behavior
  • in a dirty legacy modal (for example Navigation, Newsletter, Integration, or Staff profile), check Close/X/Escape/backdrop behavior and ensure the confirmation appears above the modal
  • in Member emails, dirty both the standard welcome-email editor and the Customize email-design modal; verify Stay preserves changes, Leave discards them, and Escape first closes the confirmation rather than the underlying editor
  • focus a legacy text field, CodeMirror field, or Koenig editor and verify Settings search/keyboard shortcuts do not steal focus

no ref

Settings was maintaining duplicate dirty and focus providers, which split state between Shade and the legacy design system. A shared Shade confirmation pattern now keeps navigation guards consistent while the legacy settings layer is retired.
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: beb465e0-7502-4834-bb45-d4da7b1e36f0

📥 Commits

Reviewing files that changed from the base of the PR and between 588f4cd and d4fe3df.

📒 Files selected for processing (22)
  • apps/admin-x-design-system/src/global/form/text-field.tsx
  • apps/admin-x-design-system/src/global/modal/modal.tsx
  • apps/admin-x-design-system/src/global/modal/preview-modal.tsx
  • apps/admin-x-design-system/src/hooks/use-global-dirty-state.tsx
  • apps/admin-x-design-system/src/index.ts
  • apps/admin-x-design-system/src/providers/design-system-provider.tsx
  • apps/admin-x-design-system/src/utils/modals.tsx
  • apps/admin-x-settings/src/components/code-editor-view.tsx
  • apps/admin-x-settings/src/components/exit-settings-button.tsx
  • apps/admin-x-settings/src/components/koenig-editor-base.tsx
  • apps/admin-x-settings/src/components/settings/email-design/dirty-confirm-modal.tsx
  • apps/admin-x-settings/src/components/settings/email-design/email-design-modal.tsx
  • apps/admin-x-settings/src/components/settings/growth/tips-and-donations.tsx
  • apps/admin-x-settings/src/components/settings/membership/member-emails/welcome-email-modal.tsx
  • apps/admin-x-settings/src/components/sidebar.tsx
  • apps/admin-x-settings/src/hooks/use-setting-group.tsx
  • apps/admin-x-settings/src/main-content.tsx
  • apps/shade/src/components/patterns/dirty-confirm-dialog.stories.tsx
  • apps/shade/src/components/patterns/dirty-confirm-dialog.tsx
  • apps/shade/src/components/ui/alert-dialog.tsx
  • apps/shade/src/patterns.ts
  • apps/shade/test/unit/components/patterns/dirty-confirm-dialog.test.tsx

Walkthrough

The changes add a reusable Shade dirty-confirmation hook and dialog with exports, stories, and tests. The design-system provider and public exports are narrowed by removing focus and legacy dirty-confirmation APIs. Design-system modals and settings components now use useDirtyConfirmation and render DirtyConfirmDialog. Focus-related imports are moved to Shade, and global dirty-state imports are updated accordingly.

✨ 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 codex/admin-settings-shade-dirty-guards

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

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

@nx-cloud

nx-cloud Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 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 d4fe3df

Command Status Duration Result
nx run @tryghost/admin:test:acceptance ✅ Succeeded 6m 31s View ↗
nx run-many -t test:unit -p @tryghost/admin-x-d... ✅ Succeeded 5m 7s View ↗
nx run ghost-admin:test ✅ Succeeded 2m 56s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 23s View ↗
nx run-many -t lint -p @tryghost/admin-x-design... ✅ Succeeded 2m 13s View ↗
nx run @tryghost/admin:build ✅ Succeeded 2m 7s View ↗
nx run @tryghost/activitypub:test:acceptance ✅ Succeeded 46s 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-07-22 21:12:31 UTC

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.79%. Comparing base (588f4cd) to head (d4fe3df).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29545      +/-   ##
==========================================
+ Coverage   74.77%   74.79%   +0.01%     
==========================================
  Files        1611     1611              
  Lines      141659   141678      +19     
  Branches    17311    17319       +8     
==========================================
+ Hits       105926   105962      +36     
+ Misses      34691    34673      -18     
- Partials     1042     1043       +1     
Flag Coverage Δ
admin-tests 55.62% <ø> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@9larsons
9larsons marked this pull request as ready for review July 22, 2026 21:34
@9larsons
9larsons merged commit bac54d9 into main Jul 22, 2026
47 of 48 checks passed
@9larsons
9larsons deleted the codex/admin-settings-shade-dirty-guards branch July 22, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant