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

ThemeOptions TS typings appear to be incomplete #11853

Closed
2 tasks done
Slessi opened this issue Jun 14, 2018 · 20 comments
Closed
2 tasks done

ThemeOptions TS typings appear to be incomplete #11853

Slessi opened this issue Jun 14, 2018 · 20 comments

Comments

@Slessi
Copy link
Contributor

Slessi commented Jun 14, 2018

In the documentation for themes it is suggested that you can do the following:

const theme = createMuiTheme({
  palette: {
    contrastThreshold: 3,
    tonalOffset: 0.2,
  },
  status: {
    danger: 'orange',
  },
});

But it appears that the ThemeOptions interface accepted by createMuiTheme does not have the properties status, contrastThreshold or tonalOffset. Not sure if its the docs or the typings which are incorrect.

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Typings match what the documentation says you should be able to do

Current Behavior

Typings do not match what the documentation says you should be able to do

Your Environment

Tech Version
Material-UI v1.2.1
React v16.4.0
browser Chrome
@pelotom
Copy link
Member

pelotom commented Jun 25, 2018

Would you like to submit a PR?

@franklixuefei
Copy link
Contributor

I'll take a stab on it.

@franklixuefei
Copy link
Contributor

@Slessi I think only status is missing in the ThemeOptions. Both contrastThreshold and tonalOffset are props under PaletteOptions, which are already there.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 27, 2018

status is just an example. People can provide what ever they wish. We shouldn't have any code specific for it.

@franklixuefei
Copy link
Contributor

@oliviertassinari I see. Then I think we should allow the flexiblity in typings for people to specify any props. I'll make the change accordingly in that PR.

@pelotom
Copy link
Member

pelotom commented Jun 27, 2018

If this is just about theme customization, is it not covered by this?

@franklixuefei
Copy link
Contributor

@pelotom Good point. I didn't read that part.

@Slessi
Copy link
Contributor Author

Slessi commented Jun 28, 2018

Both contrastThreshold and tonalOffset are props under PaletteOptions, which are already there.

@franklixuefei They were in Palette but not PaletteOptions, not sure why the need for the multiple interfaces but I see in the PR you submitted you've added them now, thanks :)

@Slessi
Copy link
Contributor Author

Slessi commented Jun 28, 2018

status is just an example. People can provide what ever they wish. We shouldn't have any code specific for it.

I see. Then I think we should allow the flexiblity in typings for people to specify any props. I'll make the change accordingly in that PR.

@franklixuefei Was this handled in your PR? Doesn't work for me when I try it out with the changes.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 28, 2018

Was this handled in your PR?

@Slessi No, you need to follow the docs.

@Slessi
Copy link
Contributor Author

Slessi commented Jun 28, 2018

The docs are what brought me here: https://material-ui.com/customization/themes/#createmuitheme-options-theme

This was listed as an example:

import { createMuiTheme } from '@material-ui/core/styles';
import purple from '@material-ui/core/colors/purple';
import green from '@material-ui/core/colors/green';

const theme = createMuiTheme({
  palette: {
    primary: purple,
    secondary: green,
  },
  status: {
    danger: 'orange',
  },
});

And in TS this results in:

Argument of type '{ palette: { primary: Color; secondary: Color; }; status: { danger: string; }; }' is not assignable to parameter of type 'ThemeOptions | undefined'.
Object literal may only specify known properties, and 'status' does not exist in type 'ThemeOptions | undefined'.

@pelotom
Copy link
Member

pelotom commented Jun 28, 2018

@Slessi for theme customization in TypeScript see https://material-ui.com/guides/typescript/#customization-of-theme

@Slessi
Copy link
Contributor Author

Slessi commented Jun 28, 2018

@pelotom aha, didn't realise that was how you're expected to do it in TS. Thanks

@nagarajay
Copy link

nagarajay commented Jun 29, 2018

@pelotom @Slessi - I am also stuck on this problem. I tried it as the docs say but could not get contrastThreshold and tonalOffset to work. Appreciate your help.

I think I got it to work as with the same logic, as given in docs, I changed the PaletteOptions in createPalette declaration.

import {
  Palette,
  PaletteOptions
} from "@material-ui/core/styles/createPalette";

declare module "@material-ui/core/styles/createPalette" {
  interface Palette {
    contrastThreshold: number;
    tonalOffset: number;
  }

  interface PaletteOptions {
    contrastThreshold?: number;
    tonalOffset?: number;
  }
}

If my approach is incorrect please let me know.

@pelotom
Copy link
Member

pelotom commented Jun 29, 2018

@nagarajay so it's working then? It looks fine to me.

@nagarajay
Copy link

@pelotom Yes! It seems to be working. Thanks for providing direction.

@micahstubbs
Copy link

@nagarajay naive question here: what did you name the file that you put the augmented module declaration in?

@oliviertassinari
Copy link
Member

@micahstubbs Does this help? https://stackoverflow.com/questions/42262565/how-to-augment-typescript-interface-in-d-ts

@micahstubbs
Copy link

yes it does, thanks @oliviertassinari! in our project, we ended up following that pattern and putting it in:

src/types/material-ui.d.ts

@Huzaifa181
Copy link

Huzaifa181 commented Dec 22, 2021

You have to declare the modules to extend the default styles of material UI e.g for pallete...
export declare module "@material-ui/core/styles/createPalette" {
interface PaletteOptions {
BgColor?: string;
}
}
Or by making the title.d.ts file and export these modules inside this file

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

No branches or pull requests

7 participants