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

Not following MUI's theme value name on Notification #8510

Closed
Seojun-Park opened this issue Dec 16, 2022 · 2 comments · Fixed by #8519
Closed

Not following MUI's theme value name on Notification #8510

Seojun-Park opened this issue Dec 16, 2022 · 2 comments · Fixed by #8519

Comments

@Seojun-Park
Copy link
Contributor

Summary

<Notification /> uses MUI's <SnackBar /> component and overrides style according to type of notification(success, error, warning, undo...etc).

StyledSnackbar uses MUI's error palette on both case, error(error.dark) and warning(error.light).

  • react-admin
    [`& .${NotificationClasses.success}`]: {
    backgroundColor: theme.palette.success.main,
    color: theme.palette.success.contrastText,
    },
    [`& .${NotificationClasses.error}`]: {
    backgroundColor: theme.palette.error.dark,
    color: theme.palette.error.contrastText,
    },
    [`& .${NotificationClasses.warning}`]: {
    backgroundColor: theme.palette.error.light,
    color: theme.palette.error.contrastText,
    },
    [`& .${NotificationClasses.undo}`]: {
    color:
    type === 'success'
    ? theme.palette.success.contrastText
    : theme.palette.primary.light,
    },

But, there is warning palette on MUI`'s default palette. (please refer the below screenshot, or link)

  • MUI
    image

Is there any reason that not using warning value from MUI's default palette?

@Seojun-Park Seojun-Park changed the title Notification theme field name Not following MUI's theme value name on Notification Dec 16, 2022
@slax57
Copy link
Contributor

slax57 commented Dec 16, 2022

Thanks for submitting this.
I don't think it's intended, so indeed it might be better to change the warning notification to use the 'warning' palette color.

Would you like to submit a PR to change this?

@Seojun-Park
Copy link
Contributor Author

@slax57 Of course

I will make PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants