Typography variantMapping should merge with the default value

[tiuvi]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Current behavior 😯

variantMapping es sobreescrito por defaultVariantMapping estropeando la variantMapping propiedad.

https://github.com/mui/material-ui/blob/92a4866ce9a06769592cddc06cfa7455b4644544/packages/mui-material/src/Typography/Typography.js#L102

Se podría cambiar esta linea por variantMapping["defaultVariantMapping"] = defaultVariantMapping,

En esta linea variantMapping[variant] no se lee nunca porque las propiedades quedan sobre escritas. https://github.com/mui/material-ui/blob/92a4866ce9a06769592cddc06cfa7455b4644544/packages/mui-material/src/Typography/Typography.js#L121

Expected behavior 🤔

Que variantMapping funcione de manera correcta, sin tener que usar la api de componentes.

Steps to reproduce 🕹

Crear un tema y probar las opciones de typography con diferentes api typography: {

fontFamily: "libre franklin",
"em":styleText(400 ,{name: "backgroundColor", value:"lawngreen"}),
/*
variantMapping: {
  "em": 'em',
},*/
defaultProps: {
  variantMapping: {
    "em": 'em',
  },
},

},

components: { MuiTypography: {

  variants: [
    {
        props: { variant: 'mark' },
        style: {
          backgroundColor:"lightskyblue",
        },
    },
  ],

/ Nofunciona variantMapping: { "mark": 'mark', }, / defaultProps: { variantMapping: { subtitle1: 'header', subtitle2: 'header', "mark": 'mark', "em": 'em', }, }, }, },

Context 🔦

Al ver porque usabais defaultProps me di cuenta de este error.

Your environment 🌎

No response

[mnajdova]

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-latest), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

[tiuvi]

Sorry I don't have time to fix this right now, just use defaultProps , and everything works fine. In the code it is clearly seen that you overwrite the variable by doing variantMapping = defaultVariantMapping, So here it doesn't read because defaultVariantMapping overwrites variantMapping, there's not much to prove. (paragraph ? 'p' : variantMapping[variant] || defaultVariantMapping[variant])

[siriwatknp]

I think the docs should mention that the custom variantMapping will replace the defaultVariantMapping, so developers have to provide all of the default mapping + custom mapping.

https://mui.com/material-ui/react-typography/#changing-the-semantic-element

[siriwatknp]

I think the expected behavior should be merging not replacing. I don't see any blockers if we want to improve this:

• It won't affect the project that already provides the custom variantMapping, it will override the default one anyway.
• No difference in terms of bundle size
• It works fine for a project that want to revamp the typography variants.

diff --git a/packages/mui-material/src/Typography/Typography.js b/packages/mui-material/src/Typography/Typography.js
index 37d1d450cf..340294b8b0 100644
--- a/packages/mui-material/src/Typography/Typography.js
+++ b/packages/mui-material/src/Typography/Typography.js
@@ -99,9 +99,10 @@ const Typography = React.forwardRef(function Typography(inProps, ref) {
     noWrap = false,
     paragraph = false,
     variant = 'body1',
-    variantMapping = defaultVariantMapping,
+    variantMapping: variantMappingProp,
     ...other
   } = props;
+  const variantMapping = { ...defaultVariantMapping, ...variantMappingProp };

   const ownerState = {
     ...props,
@@ -116,10 +117,7 @@ const Typography = React.forwardRef(function Typography(inProps, ref) {
     variantMapping,
   };

-  const Component =
-    component ||
-    (paragraph ? 'p' : variantMapping[variant] || defaultVariantMapping[variant]) ||
-    'span';
+  const Component = component || (paragraph ? 'p' : variantMapping[variant]) || 'span';

   const classes = useUtilityClasses(ownerState);