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

[Request] Allow style overrides via material-ui theme #215

Closed
JonKrone opened this issue Jan 22, 2020 · 22 comments
Closed

[Request] Allow style overrides via material-ui theme #215

JonKrone opened this issue Jan 22, 2020 · 22 comments

Comments

@JonKrone
Copy link

JonKrone commented Jan 22, 2020

Hello!

Context

We're building a themed component library which will be used to build many apps and want to customize the appearance of these SnackbarItems without other developers having to worry about it and maintain consistency with our company's theme.

I understand that the oft-recommended styling strategy is to apply a classes= prop to the SnackbarProvider, however, this isn't an ideal solution for us as it requires overhead for engineers to apply and maintain consistent theming with the rest of our apps. Using theme.overrides allows us to change the appearance everywhere, with ease.

It seems that there have been a few issues around this (#51 is the same question), and there are a lot of questions about how to customize styles here. Is it intentional that we can't override styles via the theme?

Expected Behavior

I would like to style the SnackbarItem via Material-UI theme overrides:

overrides: {
  SnackbarItem: {
    variantInfo: {
      backgroundColor: <our blue theme>
    }
  }
}

Current Behavior

Can not apply global theme overrides.

From my initial digging into withStyles and makeStyles, it seems like the reason these overrides aren't possible is because the SnackbarItem Component does not have a displayName property. Adding that fixes the style merging.

The below also works and is what we're doing now, but slightly less ideal than adopting material-ui's theming pattern.

overrides: {
    MuiSnackbarContent: {
      root: {
        '&[class*="variantInfo"]': {
          backgroundColor: 'red'
        },
        // .. etc for other variants
      }
    },
  }

Steps to Reproduce

I think this is a known/intentional bug/feature but let me know if you'd like a real code sandbox demo.

Your Environment

Tech Version
Notistack v0.9.7
@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jan 22, 2020

I'm wondering, are they an opportunity to use the Alert component inside notistack instead of a custom SnackbarItem? Alternatively, I would recommend something like this:

-export default withStyles(styles, { index: 1 })(SnackbarItem);
+export default withStyles(styles, { name: 'NotistackItem })(SnackbarItem);

(index: 1 looks strange):

@JonKrone
Copy link
Author

use the Alert component inside notistack

Yeah, this would be great - they serve the same purpose. It'd be a significant change, though, impacting styling structure so it'd probably be a major version upgrade I think.

We're fine with the CSS selector approach we're using for now but adding the { name: <x> } would be a great quick fix for this.

index: 1 comes from this issue: #202

@oliviertassinari
Copy link
Contributor

index: 1 comes from this issue: #202

Thanks for the context. I can't find any reproduction in the linked issue. I would highly encourage to remove any custom index value.

@mvitkin
Copy link

mvitkin commented Jan 22, 2020

Can confirm, with notistack 0.9.7, when a snackbar is open, an extra <style data-jss data-meta="MuiPaper">...</style> is injected into the head of the document, which happens to come after the same MuiPaper style provided by regular Material-UI.

We noticed this when switching to a dark theme with Material-UI; all the Papers on the page switch to a white background while the snackbar is open, as the second MuiPaper style is using a light theme.

@iamhosseindhv
Copy link
Owner

Can confirm, with notistack 0.9.7, when a snackbar is open ...

index: 1 comes from this issue: #202

Commit for fixing #202 hasn't been published yet. So it is expected to still have this issue in v0.9.7.


Are they an opportunity to use the Alert component inside notistack instead of a custom SnackbarItem

Although Alert component does look good, I think people are used to the current way notistack snackbars look like and can be customised. This would be a breaking change which requires much more thought to go into it and out of the scope of this issue.


I would highly encourage to remove any custom index value.

Thanks for participating @oliviertassinari. Given that I could reproduce this issue at the time, as well as other people, what are the negative effects of applying a custom index?

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Jan 27, 2020

@iamhosseindhv Without a reproduction, I think that #202 should be considered invalid. I'm curious to hear more about how you have reproduced it. We have a couple of third-party libraries built on top of Material-UI, I'm not aware of anyone forcing an index value.

The downside of this approach is that it forces the developers that want to customize the item to increase the index as well. Could it break people's customizations when released?

@mvitkin
Copy link

mvitkin commented Feb 5, 2020

Commit for fixing #202 hasn't been published yet. So it is expected to still have this issue in v0.9.7.

Sorry, thought that commit had already been released. Do you have an estimate for when the next release can be expected?

iamhosseindhv added a commit that referenced this issue Feb 23, 2020
@mvitkin
Copy link

mvitkin commented Feb 24, 2020

I was actually able to solve my issue; it was because my <SnackbarProvider /> was above my material-ui <ThemeProvider /> in my app.

As per the README:

Note: If you're using material-ui ThemeProvider, make sure SnackbarProvider is a child of it.

@iamhosseindhv
Copy link
Owner

iamhosseindhv commented Feb 27, 2020

Appreciate this isn't the most convenient but it's also not an ugly solution / hack.

overrides: {
    MuiSnackbarContent: {
      root: {
        '&[class*="variantInfo"]': {
          backgroundColor: 'red'
        },
        // .. etc for other variants
      }
    },
  }

@animaonline
Copy link

This doesn't seem to be working in the latest version of notistack and mui

@CarsonF
Copy link

CarsonF commented Apr 30, 2020

I just wrapped the provider to use the MUI theme palette colors which are already defined and easy to customize.

const useStyles = makeStyles(({ palette }) => ({
  // default variant
  contentRoot: {
     backgroundColor: 'aqua',
  },
  variantSuccess: {
    backgroundColor: palette.success.main,
  },
  variantError: {
    backgroundColor: palette.error.main,
  },
  variantInfo: {
    backgroundColor: palette.info.main,
  },
  variantWarning: {
    backgroundColor: palette.warning.main,
  },
}));

export const NotistackProvider: FC = ({ children }) => {
  const classes = useStyles();
  return <SnackbarProvider classes={classes} children={children} />;
};

@iamhosseindhv
Copy link
Owner

This doesn't seem to be working in the latest version of notistack and mui

I can confirm this works in the latest version v0.9.11. Working example:
https://codesandbox.io/s/fervent-franklin-7e0og?file=/App.js

Please provide a reproduction if you think it's not working for you.

@chrisbag
Copy link

Hello @iamhosseindhv,
The codebsandbox example above does not appear to be working. All snackbars should be red. However, they have kept their original color.
I am experiencing the same problem on my side. Do you know how to fix this ?
Thanks a lot for your help

@iamhosseindhv
Copy link
Owner

iamhosseindhv commented Aug 25, 2020

@chrisbag https://github.com/iamhosseindhv/notistack/blob/master/CHANGELOG.md#breaking-changes

Overriding SnackbarContent styles through material ui theme is not supported after v1.0.0.
Refer to #215 (comment) to apply your customisations.

@chrisbag
Copy link

Ok, thanks a lot for the fast reply !

@SimonAmphora
Copy link

SimonAmphora commented Sep 28, 2020

Hi, I'm trying to override the styles as above #215 (comment)

However, I'm facing an issue where my new styles are not taking effect due to the !important flag in the snackbarItem styles:

.SnackbarItem-variantSuccess-380 {
    color: #fff !important;
    background-color: #43a047 !important;
}

you can see an example here:
https://codesandbox.io/s/nice-kapitsa-0k7go?file=/SnackbarProvider2.js
The top row of buttons is using standard styles, second row is using styles with !important

Now i can force them with updating my styles with the !important flag but i would rather not

@rstcruzo
Copy link

I am having same issue as @SimonAmphora . Can we reopen this?

@iamhosseindhv
Copy link
Owner

iamhosseindhv commented May 19, 2021

There has been ongoing work to remove the dependency on Material-UI and adding this feature is stepping exactly in the opposite direction, so I don't think I/we will add support for customisation through MUI theme.

v3 is on its way, downloadable by npm i notistack@alpha which makes customisation much easier.

I'm committed to make sure you guys can apply your customisations somehow, regardless of whether you choose to upgrade to v3 to not.

Also, as of v1.0.9, you do not need to use !important anymore.

@ghost
Copy link

ghost commented Nov 24, 2021

@iamhosseindhv I'm running version 2.0.3 and I see the issue hasn't been solved yet. Are you going to fix it?

@iamhosseindhv
Copy link
Owner

@mbeltramin

I see the issue hasn't been solved yet

If by issue you mean the ability to apply styles via MUI theme, I have previously mentioned it won't be supported. Did you mean some other issue?

@ghost
Copy link

ghost commented Nov 24, 2021

@iamhosseindhv I mean the order of the applyed styles, as @JonKrone mentioned.

@iamhosseindhv
Copy link
Owner

@mbeltramin Things have changes since Jan 2020 when this issue was opened. Please open a new one providing a reproduction example.

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

No branches or pull requests

9 participants