Removed legacy settings upload components#29541
Conversation
no ref Settings uploads now share Shade primitives and image-preview behavior, allowing the legacy design-system upload implementations and their duplicated consumers to be removed.
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change removes the admin design-system’s 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:test:acceptance |
✅ Succeeded | 6m 21s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-d... |
✅ Succeeded | 4m 56s | View ↗ |
nx run ghost-admin:test |
✅ Succeeded | 3m 9s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 23s | View ↗ |
nx run-many -t lint -p @tryghost/admin-x-design... |
✅ Succeeded | 2m 10s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 1m 58s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 44s | 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 19:04:46 UTC
no ref The compact upload actions now share Shade's secondary button recipe with their adjacent Edit actions, keeping their size and visual hierarchy consistent.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/admin-x-settings/src/components/settings/advanced/labs/migration-options.tsx (1)
16-42: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDisable each Dropzone while its operation is pending.
The loading labels do not prevent another selection/drop. This can enqueue duplicate imports or overwrite a concurrent redirects/routes upload.
apps/admin-x-settings/src/components/settings/advanced/labs/migration-options.tsx#L16-L42: passdisabled={uploading}toDropzone.apps/admin-x-settings/src/components/settings/advanced/migration-tools/universal-import-modal.tsx#L29-L57: passdisabled={uploading}toDropzone.apps/admin-x-settings/src/components/settings/advanced/labs/beta-features.tsx#L83-L102: passdisabled={redirectsUploading}toDropzone.apps/admin-x-settings/src/components/settings/advanced/labs/beta-features.tsx#L113-L132: passdisabled={routesUploading}toDropzone.🤖 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/advanced/labs/migration-options.tsx` around lines 16 - 42, Disable each Dropzone while its corresponding upload is pending by passing disabled={uploading} in migration-options.tsx and universal-import-modal.tsx, disabled={redirectsUploading} for the redirects Dropzone in beta-features.tsx, and disabled={routesUploading} for the routes Dropzone in beta-features.tsx.apps/admin-x-settings/src/components/settings/email-design/header-image-field.tsx (1)
13-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing error handling on image upload.
handleUploadhas no try/catch, unlike every sibling implementation in this cohort (e.g.custom-integration-modal.tsx,global-settings.tsx,theme-setting.tsx,look-and-feel.tsx,user-detail-modal.tsx,seo-meta.tsx), which all wrap theuploadImagecall and route failures throughuseHandleError. Here a rejected upload (bad file type, network failure) becomes an unhandled promise rejection with no toast/feedback to the user — the field just appears to do nothing.🛠️ Proposed fix
+import {useHandleError} from '`@tryghost/admin-x-framework/hooks`'; + const HeaderImageField: React.FC<HeaderImageFieldProps> = ({value, onChange}) => { const {mutateAsync: uploadImage} = useUploadImage(); + const handleError = useHandleError(); const handleUpload = async (file: File) => { - const imageUrl = getImageUrl(await uploadImage({file})); - onChange(imageUrl); + try { + const imageUrl = getImageUrl(await uploadImage({file})); + onChange(imageUrl); + } catch (e) { + handleError(e); + } };🤖 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/email-design/header-image-field.tsx` around lines 13 - 19, Update HeaderImageField’s handleUpload to wrap the uploadImage call and getImageUrl/onChange flow in try/catch, then route failures through the existing useHandleError mechanism used by sibling components, preserving the successful upload behavior and providing user feedback for rejected uploads.apps/admin-x-settings/src/components/settings/site/design-and-branding/global-settings.tsx (1)
1-1: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winAdd image type restrictions to the remaining image dropzones.
ImageUploadDropzonespreads props through to Shade’sDropzone, so the image upload dropzones can and should passaccept={{'image/*': [...]}}now instead of letting users drop files only to get a 415 after upload.🤖 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/design-and-branding/global-settings.tsx` at line 1, Update the remaining image upload dropzones in global-settings.tsx to pass Shade Dropzone’s accept prop with an image/* restriction, using the existing ImageUploadDropzone instances and preserving their current upload behavior.
🧹 Nitpick comments (2)
apps/admin-x-settings/src/components/settings/site/design-and-branding/theme-setting.tsx (1)
89-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLabel not associated with the upload input.
FieldLabelhas nohtmlFor, so it isn't programmatically tied to theImageUploadDropzone's file input (inputId={custom-${setting.key}}) — unlikeheader-image-field.tsx, which wireshtmlForto the matchinginputId.♿ Proposed fix
- <FieldLabel>{humanizeSettingKey(setting.key)}</FieldLabel> + <FieldLabel htmlFor={`custom-${setting.key}`}>{humanizeSettingKey(setting.key)}</FieldLabel>🤖 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/design-and-branding/theme-setting.tsx` around lines 89 - 109, The image setting’s FieldLabel is not associated with its file input. In the `case 'image'` rendering within the settings component, set `FieldLabel`’s `htmlFor` to the same `custom-${setting.key}` identifier used by `ImageUploadDropzone`’s `inputId`, matching the pattern in `header-image-field.tsx`.apps/admin-x-settings/src/components/settings/advanced/integrations/custom-integration-modal.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
altto the remainingImageUploadImagepreviews.
ImageUploadImagedefaultsaltto an empty string unless passed explicitly, so these screens won’t announce rawsrcURLs. Provide a meaningful label, or an explicit emptyaltonly when the preview is decorative, at each site:
apps/admin-x-settings/src/components/settings/advanced/integrations/custom-integration-modal.tsx#L103apps/admin-x-settings/src/components/settings/email/newsletters/newsletter-detail-modal.tsx#L303apps/admin-x-settings/src/components/settings/general/seo-meta.tsx#L224and#L279apps/admin-x-settings/src/components/settings/general/user-detail-modal.tsx#L358apps/admin-x-settings/src/components/settings/membership/portal/look-and-feel.tsx#L118apps/admin-x-settings/src/components/settings/site/design-and-branding/global-settings.tsx#L182,#L212, and#L241apps/admin-x-settings/src/components/settings/site/design-and-branding/theme-setting.tsx#L95🤖 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/advanced/integrations/custom-integration-modal.tsx` at line 1, Update every listed ImageUploadImage preview, including the instance in custom-integration-modal.tsx and the corresponding instances in newsletter-detail-modal.tsx, seo-meta.tsx, user-detail-modal.tsx, look-and-feel.tsx, global-settings.tsx, and theme-setting.tsx, to pass an explicit meaningful alt label or an empty alt value when the image is decorative.
🤖 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.
Outside diff comments:
In
`@apps/admin-x-settings/src/components/settings/advanced/labs/migration-options.tsx`:
- Around line 16-42: Disable each Dropzone while its corresponding upload is
pending by passing disabled={uploading} in migration-options.tsx and
universal-import-modal.tsx, disabled={redirectsUploading} for the redirects
Dropzone in beta-features.tsx, and disabled={routesUploading} for the routes
Dropzone in beta-features.tsx.
In
`@apps/admin-x-settings/src/components/settings/email-design/header-image-field.tsx`:
- Around line 13-19: Update HeaderImageField’s handleUpload to wrap the
uploadImage call and getImageUrl/onChange flow in try/catch, then route failures
through the existing useHandleError mechanism used by sibling components,
preserving the successful upload behavior and providing user feedback for
rejected uploads.
In
`@apps/admin-x-settings/src/components/settings/site/design-and-branding/global-settings.tsx`:
- Line 1: Update the remaining image upload dropzones in global-settings.tsx to
pass Shade Dropzone’s accept prop with an image/* restriction, using the
existing ImageUploadDropzone instances and preserving their current upload
behavior.
---
Nitpick comments:
In
`@apps/admin-x-settings/src/components/settings/advanced/integrations/custom-integration-modal.tsx`:
- Line 1: Update every listed ImageUploadImage preview, including the instance
in custom-integration-modal.tsx and the corresponding instances in
newsletter-detail-modal.tsx, seo-meta.tsx, user-detail-modal.tsx,
look-and-feel.tsx, global-settings.tsx, and theme-setting.tsx, to pass an
explicit meaningful alt label or an empty alt value when the image is
decorative.
In
`@apps/admin-x-settings/src/components/settings/site/design-and-branding/theme-setting.tsx`:
- Around line 89-109: The image setting’s FieldLabel is not associated with its
file input. In the `case 'image'` rendering within the settings component, set
`FieldLabel`’s `htmlFor` to the same `custom-${setting.key}` identifier used by
`ImageUploadDropzone`’s `inputId`, matching the pattern in
`header-image-field.tsx`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd39759a-3692-4641-9083-d761c9d4525c
📒 Files selected for processing (25)
apps/admin-x-design-system/src/global/form/file-upload.stories.tsxapps/admin-x-design-system/src/global/form/file-upload.tsxapps/admin-x-design-system/src/global/form/image-upload.stories.tsxapps/admin-x-design-system/src/global/form/image-upload.tsxapps/admin-x-design-system/src/index.tsapps/admin-x-settings/src/components/settings/advanced/integrations/custom-integration-modal.tsxapps/admin-x-settings/src/components/settings/advanced/integrations/pintura-modal.tsxapps/admin-x-settings/src/components/settings/advanced/labs/beta-features.tsxapps/admin-x-settings/src/components/settings/advanced/labs/migration-options.tsxapps/admin-x-settings/src/components/settings/advanced/migration-tools/universal-import-modal.tsxapps/admin-x-settings/src/components/settings/email-design/header-image-field.tsxapps/admin-x-settings/src/components/settings/email/newsletters/newsletter-detail-modal.tsxapps/admin-x-settings/src/components/settings/general/seo-meta.tsxapps/admin-x-settings/src/components/settings/general/user-detail-modal.tsxapps/admin-x-settings/src/components/settings/membership/portal/look-and-feel.tsxapps/admin-x-settings/src/components/settings/site/design-and-branding/global-settings.tsxapps/admin-x-settings/src/components/settings/site/design-and-branding/theme-setting.tsxapps/admin-x-settings/src/components/settings/site/theme-modal.tsxapps/shade/src/components/patterns/image-upload.stories.tsxapps/shade/src/components/patterns/image-upload.tsxapps/shade/src/components/ui/dropzone.stories.tsxapps/shade/src/components/ui/dropzone.tsxapps/shade/src/patterns.tsapps/shade/tailwind.theme.cssapps/shade/test/unit/components/ui/dropzone.test.tsx
💤 Files with no reviewable changes (5)
- apps/admin-x-design-system/src/global/form/image-upload.stories.tsx
- apps/admin-x-design-system/src/global/form/file-upload.stories.tsx
- apps/admin-x-design-system/src/global/form/image-upload.tsx
- apps/admin-x-design-system/src/global/form/file-upload.tsx
- apps/admin-x-design-system/src/index.ts
no ref The Tailwind class order is now canonical so clean CI environments produce the same lint result as local development.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29541 +/- ##
==========================================
- Coverage 74.75% 74.54% -0.21%
==========================================
Files 1610 1609 -1
Lines 141605 141494 -111
Branches 17292 17269 -23
==========================================
- Hits 105854 105479 -375
- Misses 34680 34980 +300
+ Partials 1071 1035 -36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
What changed
DropzoneDropzonewith filled and button variants plus input accessibility/test hooksImageUploadandFileUploadcomponentsWhy
This is Group 8 of the admin settings Shade migration. Upload behavior was duplicated across Settings and still depended on legacy design-system components. Centralizing image preview, edit, remove, and drop behavior in Shade lets those legacy components be deleted while preserving the existing flows.
Validation