found a redundunt component that duplicates existing Alert#4
found a redundunt component that duplicates existing Alert#4eugenebelov merged 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR deprecates the WarningMessage component by marking it as redundant and enhances the Alert component to support custom icons, making it a suitable replacement. The changes ensure that existing WarningMessage functionality can be replicated using the Alert component.
- Deprecated
WarningMessagecomponent with a JSDoc comment recommendingAlertwithiconSrcprop - Enhanced
Alertcomponent to accept custom icons viaiconSrcprop - Removed
WarningMessageStorybook stories and updatedAlertstories to demonstrate custom icon usage
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| warning-message.tsx | Added deprecation comment and minor formatting fixes |
| warning-message.stories.tsx | Completely removed Storybook stories for deprecated component |
| alert.tsx | Added iconSrc prop support and styling improvements |
| alert.stories.tsx | Updated to modern Storybook format with custom icon example |
| account-info-row.tsx | Code formatting improvements and added text variation properties |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <SvgIcon src={iconPath} alt={`Alert icon with ${status} status`} /> | ||
| <SvgIcon | ||
| src={iconPath} | ||
| alt={`Alert icon with ${statusAlert} status`} |
There was a problem hiding this comment.
When statusAlert is an empty string (custom icon case), the alt text becomes 'Alert icon with status' which contains extra whitespace and is unclear. Consider providing a more descriptive alt text for custom icons or using the original status value.
| alt={`Alert icon with ${statusAlert} status`} | |
| alt={props.iconSrc ? 'Custom alert icon' : `Alert icon with ${statusAlert} status`} |
<WarningMessage />marked as redundunt. Add modifications to<Alert />that we can use it in same way as<WarningMessage />