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

Cannot use theme.overrides with the lab #19427

Closed
2 tasks done
Jarlotee opened this issue Jan 27, 2020 · 23 comments · Fixed by #21279
Closed
2 tasks done

Cannot use theme.overrides with the lab #19427

Jarlotee opened this issue Jan 27, 2020 · 23 comments · Fixed by #21279
Labels
package: lab Specific to @mui/lab ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript

Comments

@Jarlotee
Copy link

Jarlotee commented Jan 27, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

There are no definitions for overriding Alert styles.

Expected Behavior 🤔

MuiAlert should be overridable.

Context 🔦

Using dark theme, alerts are not white on a dark/filled background.

createMuiTheme({
  palette: {
    type: 'dark',
    primary: {
      main: '#FFD900',
    },
    secondary: {
      main: '#3E464F',
    },
    error: {
      main: '#E01F1F',
    },
    background: {
      default: '#2A343D',
      paper: '#545B63',
    },
  },
  overrides: {
    MuiAlert: {}, // missing
  },
});

Your Environment 🌎

Tech Version
Material-UI v4.9.0
React v16.12.0
Browser chrome v79.0.3945.130
TypeScript v3.7.5
notistack v0.9.7
@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label Jan 27, 2020
@oliviertassinari
Copy link
Member

Please provide a full reproduction test case. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

@Jarlotee
Copy link
Author

@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Jan 27, 2020
@oliviertassinari
Copy link
Member

It seems to be a generic concern with the lab. We don't include the overrides in the default TypeScript definitions. I wonder what would be the best answer here.

@oliviertassinari oliviertassinari changed the title Cannot Override MuiAlert in theme Cannot use theme.overrides with the lab Jan 27, 2020
@levrik
Copy link

levrik commented Mar 11, 2020

I hit this issue today. Worked around it by putting the following code into my theme.ts.

import { SkeletonClassKey } from '@material-ui/lab/Skeleton';

declare module '@material-ui/core/styles/overrides' {
  export interface ComponentNameToClassKey {
    MuiSkeleton: SkeletonClassKey;
  }
}

With TypeScript 3.8 I would recommend using import type instead of just a regular import.

@oliviertassinari Maybe the lab package could include something like this in their typings directly? This way lab components should be automatically available when someone has @material-ui/lab installed.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Mar 11, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 11, 2020

I think that it would be interesting to document this problem. Somebody relying on a third-party component would face the same problem. So it's important to have it covered in the docs.

@levrik Would it force us to have core depending on the lab? I don't think that developers should expect the same quality from the lab than from the core. I think that we should be willing to take a tradeoff that discriminates against the lab.

@levrik
Copy link

levrik commented Mar 11, 2020

@oliviertassinari No, not at all. I could imagine that lab requires core being installed but I would assume this is already required? In the end this is the same technique you're recommending in the docs when adding custom colors to a theme's palette.

@oliviertassinari
Copy link
Member

@levrik Ok so the best approach would be to document?

import { SkeletonClassKey } from '@material-ui/lab/Skeleton';

declare module '@material-ui/core/styles/overrides' {
  export interface ComponentNameToClassKey {
    MuiSkeleton: SkeletonClassKey;
  }
}

@levrik
Copy link

levrik commented Mar 12, 2020

@oliviertassinari I think that something like this could be integrated in @material-ui/lab's typings.
This would "extend" the typings from @material-ui/core when @material-ui/lab is installed.
I'm not sure what happens if only @material-ui/lab would be installed. But I think that TypeScript handles errors in typings gracefully if I remember correctly.
But I'm assuming anyway that @material-ui/lab already needs @material-ui/core to work. Is this assumption correct?
I can take a look at this and open a PR for this.

@eps1lon
Copy link
Member

eps1lon commented Mar 12, 2020

@levrik That's be great!

@oliviertassinari
Copy link
Member

Would it require a circular dependency?

@eps1lon
Copy link
Member

eps1lon commented Mar 12, 2020

@oliviertassinari /core wouldn't depend on the lab. We would use module augmentation in the lab which already depends on /core.

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 15, 2020

@eps1lon This sounds great, @levrik we would love to see a pull request :).

@oliviertassinari oliviertassinari added ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed docs Improvements or additions to the documentation labels Mar 15, 2020
@levrik
Copy link

levrik commented Mar 19, 2020

@oliviertassinari I'll try to work on this the coming weekend.

@bartvanenter
Copy link

is it still on the agenda to fix this issue?

@ghost
Copy link

ghost commented Oct 9, 2020

Still not overriding

@ManacyJohn
Copy link

is there a documentation for overrides? Cant really understand how to use overrides in material-ui DatePicker. Please give an example atleast!

@oliviertassinari
Copy link
Member

@ManacyJohn See https://next.material-ui.com/components/about-the-lab/#typescript, it will work as soon as we move the date picker components in the lab.

@protzman
Copy link

@oliviertassinari So the theme augmentation is only on 5.0 for now? Would you recommend @levrik's approach for 4.X?

@oliviertassinari
Copy link
Member

@protzman You are right, it only works with v5. You could probably copy and paste the augmentation approach of into your own codebase.

@potofpie
Copy link

Would this also work for lab components when trying to props?

When I do this it changes the variant: (core)
const coreTheme = { ..., props: { MuiButton: {variant: 'contained'} } }

When I do this it changes the variant: (lab)
const coreTheme = { ..., props: { MuiAutocomplete: {variant: 'outlined'} } }

The overrides extension did work. Just need to take things a little further.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 15, 2021

@potofpie Yes, this should work. Also, in v5, the autocomplete was promoted in the core.

@leolozes
Copy link

@oliviertassinari any similar workaround for the DataGrid? I was trying to override MuiDataGrid styles with createTheme, but it seems it's not possible yet (at least with TS). Since the DataGrid theming documentation only refers to v4, what is the plan for v5?

Thanks!

@oliviertassinari
Copy link
Member

@leolozes Thanks for raising the problem. Yes, great idea. We need to do the same for MUI-X. Issue opened: mui/mui-x#1755.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: lab Specific to @mui/lab ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants