backstage / backstage

Backstage is an open framework for building developer portals
https://backstage.io/
Apache License 2.0
26.9k stars 5.58k forks source link

šŸ› Bug Report: Unable to Apply Overrides for MuiTypography #22136

Open angeelsanchez opened 5 months ago

angeelsanchez commented 5 months ago

šŸ“œ Description

Hello, I am currently in the process of migrating to the new system for managing themes. However, I have encountered a bug related to the overrides for colorPrimary and colorTextSecondary in the MuiTypography component.

(Before updating to createUnifiedTheme, these classes were not causing any problems)

šŸ‘ Expected behavior

The overrides for colorPrimary and colorTextSecondary in the MuiTypography component should successfully apply the specified font color from the new theme.

šŸ‘Ž Actual Behavior with Screenshots

The overrides for colorPrimary and colorTextSecondary seem to be ineffective, and the font color is not being applied as intended.

šŸ‘Ÿ Reproduction steps

  1. Attempt to apply overrides for colorPrimary and colorTextSecondary in the MuiTypography component using the new theme system.
  2. Observe that the overrides do not take effect as expected.

šŸšØ Error Messages

I encountered the following error message in TypeScript while attempting to apply overrides for colorPrimary and colorTextSecondary in the MuiTypography component:

json
[{
    "resource": "/path/to/your/file.ts",
    "owner": "typescript",
    "code": "2353",
    "severity": 8,
    "message": "The object literal can only specify known properties and 'colorPrimary' does not exist in the type 'Partial<OverridesStyleRules<keyof TypographyClasses, \"MuiTypography\", Omit<Theme, \"components\">>>'.",
    "source": "ts",
    "startLineNumber": 378,
    "startColumn": 9,
    "endLineNumber": 378,
    "endColumn": 21
}]

šŸ“ƒ Provide the context for the Bug.

MuiTypography: {
  styleOverrides: {
    colorPrimary: ({ theme }) => ({
      color: theme.page.fontColor,
    }),
    colorTextSecondary: ({ theme }) => ({
      color: theme.page.fontColor,
    }),
  },
},

šŸ–„ļø Your Environment

No response

šŸ‘€ Have you spent some time to check if this bug has been raised before?

šŸ¢ Have you read the Code of Conduct?

Are you willing to submit PR?

None

angeelsanchez commented 5 months ago

Additionally, I've noticed that even though this error occurs, it works if you apply it and start the application. However, TypeScript errors occur when running yarn tsc.

angeelsanchez commented 5 months ago

I've noticed another thing ā€“ it seems like overriding styles isn't working the same way it used to within a specific class, or at least not as straightforward.

const theme = useTheme();

const customTheme = createMuiTheme({
  ...myTheme,
  overrides: {
    ...myTheme.overrides,

    MuiInputBase: {
      root: {
        ...myTheme.overrides,
        backgroundColor: `${
          theme.palette.type === 'dark'
            ? vars.dark.background.white
            : vars.light.background.white
        }`,
        color: 'black',
        padding: '10px',
        borderRadius: '12px',
      },
    },
  },
});

It seems like the usual method of overwriting styles within a specific class might not be working as expected

awanlin commented 5 months ago

Can you pease include your full theme or as much of it as you can as well as the steps you've taken to use it like the changes to App.tsx šŸ‘

tudi2d commented 5 months ago

This is an interesting issue. The reason for this is that there has been a change from v4 to v5 in the class names for MuiTypography, which is not covered by our UnifiedTheme compatibility layer.

I'm wondering though if overriding the component is the best approach - specifically as it is typography. You are able to set this colors throught the theme palette. This way you affect all places text is used & not only those where it is rendered inside of a MuiTypography component.

Let me know if that helps as it is a real issue & we should fix it otherwise.

angeelsanchez commented 5 months ago

This is an interesting issue. The reason for this is that there has been a change from v4 to v5 in the class names for MuiTypography, which is not covered by our UnifiedTheme compatibility layer.

I'm wondering though if overriding the component is the best approach - specifically as it is typography. You are able to set this colors throught the theme palette. This way you affect all places text is used & not only those where it is rendered inside of a MuiTypography component.

Let me know if that helps as it is a real issue & we should fix it otherwise.

In this case, if I want MuiTypography to have different colorPrimary and colorSecondary than the default ones in the palette for the rest of the project, I couldn't do it without fixing this issue, right?

If I am mistaken, please, I would like to better understand what you tried to explain to me in the provided link.

On the other hand (it's not my case), but still, if someone needs to add something extra that is not just color (shadows, text transformation, or others), it could be useful to have the Override working.

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.