Changed settings menus to styled Shade dropdowns - #29366
Conversation
ref #29363 - the legacy Menu is being phased out in favour of Shade's composable dropdown - the reworked branch includes the standalone Shade stylesheet lane from main - acceptance coverage now verifies generated Shade menu dimensions, preventing functional but unstyled markup from passing
|
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 (1)
WalkthroughMigrated user and theme action menus in admin settings from the legacy Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 9m 38s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-s... |
✅ Succeeded | 2m 20s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 20s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 44s | View ↗ |
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 59s | View ↗ |
nx run-many -t lint -p @tryghost/admin-x-settin... |
✅ 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-15 19:22:04 UTC
ref #29366 The edit modal renders before its fetched offer populates the form, so the test could clear an empty input that was then replaced with valid data.
ref #29366 Referencing the fixture value directly keeps the loading-race assertion stable when the shared offer fixture changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/admin-x-settings/src/components/settings/site/theme/advanced-theme-settings.tsx (1)
170-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider styling the delete action as destructive.
For visual consistency with the user actions menu (in
user-detail-modal.tsx), consider adding the destructive text styling to this "Delete" menu item.✨ Proposed fix to add destructive styling
- {isDeletableTheme(theme) && ( - <DropdownMenuItem onSelect={handleDelete}>Delete</DropdownMenuItem> - )} + {isDeletableTheme(theme) && ( + <DropdownMenuItem className='text-destructive focus:text-destructive' onSelect={handleDelete}> + Delete + </DropdownMenuItem> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/admin-x-settings/src/components/settings/site/theme/advanced-theme-settings.tsx` around lines 170 - 172, Update the Delete DropdownMenuItem in the isDeletableTheme rendering within the advanced theme settings component to apply the existing destructive text styling used by the user actions menu, while preserving its current onSelect={handleDelete} behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/admin-x-settings/src/components/settings/site/theme/advanced-theme-settings.tsx`:
- Around line 170-172: Update the Delete DropdownMenuItem in the
isDeletableTheme rendering within the advanced theme settings component to apply
the existing destructive text styling used by the user actions menu, while
preserving its current onSelect={handleDelete} behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 14cfe113-2c15-4d41-b2ed-f64555c2dd15
📒 Files selected for processing (6)
apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsxapps/admin-x-settings/src/components/settings/site/change-theme.tsxapps/admin-x-settings/src/components/settings/site/theme/advanced-theme-settings.tsxapps/admin-x-settings/test/acceptance/general/users/actions.test.tsapps/admin-x-settings/test/acceptance/site/theme.test.tsapps/admin/src/settings/offers.acceptance.test.tsx
1 participant
What changed
Reworked the admin-x-settings menu migration from #29363 on top of current main:
Why
The earlier PR only proved that the Radix menu behavior worked. A functional-but-unstyled menu could still pass its acceptance coverage. This replacement is based on the standalone Shade stylesheet lane already on main and explicitly verifies that Shade component utilities are compiled and applied. Production Admin continues to use its single centralized Shade CSS lane, so the embedded app does not import a duplicate stylesheet.
Validation
Supersedes #29363.