Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[material-ui][Alert] Update TypeScript types to allow color override types to be added to iconMapping and severity props #40551

Merged
merged 6 commits into from
Jan 17, 2024

Conversation

2metres
Copy link
Contributor

@2metres 2metres commented Jan 12, 2024

Change Alert types to allow color override types to be added to iconMapping and severity props

Fixes #40264

@mui-bot
Copy link

mui-bot commented Jan 12, 2024

Netlify deploy preview

https://deploy-preview-40551--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 09013f0

@danilo-leal danilo-leal changed the title Change Alert types to allow color override types to be added to iconMapping and severity props [material-ui][Alert] Change types to allow color override types to be added to iconMapping and severity props Jan 12, 2024
@danilo-leal danilo-leal added package: material-ui Specific to @mui/material component: alert This is the name of the generic UI component, not the React module! labels Jan 12, 2024
@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work typescript labels Jan 13, 2024
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@2metres Thanks for creating a PR to address the issue. Your changes seem solid. However, your branch is significantly behind the master branch. Could you please merge with the latest master to potentially resolve the Argos CI failure issue? Additionally, kindly address the test_static CI job.

Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may also need to update the PropTypes if they don't get updated with the pnpm docs:api script.

--- a/packages/mui-material/src/Alert/Alert.js
+++ b/packages/mui-material/src/Alert/Alert.js
@@ -287,12 +287,15 @@ Alert.propTypes /* remove-proptypes */ = {
    * If you wish to change this mapping, you can provide your own.
    * Alternatively, you can use the `icon` prop to override the icon displayed.
    */
-  iconMapping: PropTypes.shape({
-    error: PropTypes.node,
-    info: PropTypes.node,
-    success: PropTypes.node,
-    warning: PropTypes.node,
-  }),
+  iconMapping: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
+    PropTypes.shape({
+      error: PropTypes.node,
+      info: PropTypes.node,
+      success: PropTypes.node,
+      warning: PropTypes.node,
+    }),
+    PropTypes.objectOf(PropTypes.node),
+  ]),
   /**
    * Callback fired when the component requests to be closed.
    * When provided and no `action` prop is set, a close icon button is displayed that triggers the callback when clicked
.
@@ -308,7 +311,10 @@ Alert.propTypes /* remove-proptypes */ = {
    * The severity of the alert. This defines the color and icon used.
    * @default 'success'
    */
-  severity: PropTypes.oneOf(['error', 'info', 'success', 'warning']),
+  severity: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
+    PropTypes.oneOf(['error', 'info', 'success', 'warning']),
+    PropTypes.string,
+  ]),
   /**
    * The extra props for the slot components.
    * You can override the existing props or add new ones.

@2metres 2metres force-pushed the #40264-alert-type-augmentation branch from 06b0dbd to ed5f137 Compare January 15, 2024 23:10
@2metres
Copy link
Contributor Author

2metres commented Jan 15, 2024

@ZeeshanTamboli I've managed to step through a couple of the CI failures but there are a few others like test_e2e_website and test_unit-1 that appear unrelated to my changes.

@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][Alert] Change types to allow color override types to be added to iconMapping and severity props [material-ui][Alert] Update TypeScript types to allow color override types to be added to iconMapping and severity props Jan 17, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui][Alert] Update TypeScript types to allow color override types to be added to iconMapping and severity props [material-ui][Alert] Update TypeScript types to allow color override types to be added to iconMapping and severity props Jan 17, 2024
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few others like test_e2e_website and test_unit-1 that appear unrelated to my changes.

Yes, they caused problems before, but now it's resolved. Looks good!

@ZeeshanTamboli ZeeshanTamboli merged commit 664e9b7 into mui:master Jan 17, 2024
22 checks passed
@ZeeshanTamboli ZeeshanTamboli mentioned this pull request Jan 17, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: alert This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][Alert] Adding additional color variant breaks iconMapping for all colors
4 participants