ENG-2891: Replace Ant default icons with Carbon icons in Alert component#7613
ENG-2891: Replace Ant default icons with Carbon icons in Alert component#7613gilluminate merged 3 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR replaces Ant Design's default icons with Carbon icons in the Alert component by introducing a CustomAlert HOC that wraps Ant's Alert and automatically applies Carbon icons when showIcon is enabled and no custom icon is provided. The public Alert export is updated to use the wrapped component. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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)
Comment |
Greptile SummaryThis PR extends Ant Design's
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 2dbb5de |
speaker-ender
left a comment
There was a problem hiding this comment.
Approving as is but left some comments/questions. Overall looks good!
- Remove compact story variants per @speaker-ender's suggestion - Remove unnecessary `as FeedbackType` cast in CustomAlert - Remove hardcoded icon colors, inherit from Ant theme tokens instead Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/fidesui/src/hoc/CustomAlert.tsx`:
- Around line 11-15: The Compact vs full icon sizing uses a truthy check
(!!description) in the CustomAlert render (the arrow function that accepts
showIcon, icon, type, description) which treats valid React nodes like 0 or ""
as absent; change that to an explicit presence check (e.g., description !== null
&& description !== undefined && description !== false) when calling
getDefaultAlertIcon(type, ...), so numeric or empty-string descriptions are
treated as present and get the 24px icon; keep the existing showIcon && icon ===
undefined gating intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 533eaddc-efba-447e-bb76-442693d1a236
📒 Files selected for processing (2)
clients/fidesui/src/hoc/CustomAlert.tsxclients/fidesui/src/lib/carbon-icon-defaults.tsx
…ent (#7613) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Ticket ENG-2891
Description Of Changes
Extend Ant Design's
<Alert>component in FidesUI to use Carbon icons as the default icons for each feedback type (info, success, warning, error). This follows the same pattern established in #7569 for Modal, Message, and Notification.Since Alert is a declarative component (not an imperative API), this uses an HOC approach rather than the FidesUIProvider wrapping used for the others. Icons are sized at 16px for compact alerts and 24px when a
descriptionis present.No consumer changes needed. All existing
<Alert>imports fromfidesuiautomatically get Carbon icons whenshowIconis true. Alerts withoutshowIconare unaffected, and passing a customiconprop still overrides the default.Code Changes
clients/fidesui/src/lib/carbon-icon-defaults.tsx- AddALERT_ICON_CONFIGmap andgetDefaultAlertIcon()getter with size-aware icon selectionclients/fidesui/src/hoc/CustomAlert.tsx- New HOC wrapping Ant's Alert, injecting Carbon icons whenshowIconis true and no customiconis providedclients/fidesui/src/hoc/index.tsx- Export newCustomAlertHOCclients/fidesui/src/index.ts- Swap rawAlertre-export from antd withCustomAlert as Alert; addAlertPropstype exportclients/fidesui/src/components/feedback/Alert.stories.tsx- AddshowIcon: trueto Primary story; add compact (no description) story variants for all 4 typesSteps to Confirm
cd clients/fidesui && npm run storybook)showIconoff in the Primary story controls and confirm the icon disappearsnpx tsc --noEmitinclients/fidesuiandclients/admin-uito confirm no type errorsPre-Merge Checklist
CHANGELOG.mdupdatedSummary by CodeRabbit
Release Notes