material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[ListItemText] primaryTypographyProps doesn't include variantMapping

Open ImCheesecake opened this issue 2 years ago • 2 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Current behavior 😯

While using ListItemText and providing a variant to primaryTypographyProps it defaults to a span. And since there's an issue with using component if you create your custom components with typescript, which we do.

  <List>
    <ListItem key="1">
      <ListItemText
        primary="some random text"
        primaryTypographyProps={{
          variant: 'h4',
        }}
      />
    </ListItem>
    <ListItem key="2">
      <ListItemText
        primary="some other random text"
        primaryTypographyProps={{
          variant: 'h3',
        }}
      />
    </ListItem>

     // Testing with just Typography
    <Typography variant="h4">Testing testing</Typography>
    <Typography variant="h3">Testing testing</Typography>
  </List>

Both will render the same classes as you can see in the provided screenshots but only the Typography component will create a h3 while ListItemText will create a span image image

Expected behavior 🤔

I expect to have VariantMapping while using ListItemText as the documents clearly states that primaryTypographyProps

will be forwarded to the primary typography component (as long as disableTypography is not true).

Steps to reproduce 🕹

codesandbox example

Context 🔦

Accessibility and other WCAG guidlines is pretty important in some projects and by just including the typography variant mapping would help a lot.

Your environment 🌎

npx @mui/envinfo
System:
    OS: Windows 10 10.0.22000
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
    Memory: 18.65 GB / 31.69 GB
  Binaries:
    Node: 16.15.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.5.5 - ~\AppData\Roaming\npm\npm.CMD
  Managers:
    pip3: 22.0.4 - C:\Python310\Scripts\pip3.EXE
  Utilities:
    Git: 2.35.3. - /mingw64/bin/git
  IDEs:
    VSCode: 1.68.1 - C:\Program Files\Microsoft VS Code\bin\code.CMD
    Visual Studio: 17.2.32602.215 (Visual Studio Professional 2022)
  Languages:
    Bash: 4.4.23 - C:\Program Files\Git\usr\bin\bash.EXE
    Perl: 5.34.0 - C:\Program Files\Git\usr\bin\perl.EXE
    Python: 3.10.4 - /c/Python310/python
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (102.0.1245.44)
    Internet Explorer: 11.0.22000.120

  tsconfig:
  "compilerOptions": {
    "module": "CommonJS",
    "target": "es5",
    "jsx": "react",
    "outDir": "dist",
    "strict": true,
    "sourceMap": true,
    "moduleResolution": "node",
    "declaration": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowJs": true,
    "noImplicitAny": true,
    "noImplicitThis": true,
    "strictNullChecks": true,
    "lib": ["es2018", "dom", "es6", "ES2016", "ES2017"]
  },
  "include": ["src/**/*", ".storybook/*.tsx", "@types/*.ts"],
  "exclude": ["node_modules", "dist", ".storybook"]

ImCheesecake avatar Jun 22 '22 11:06 ImCheesecake

Looks like a bug.

As a workaround, you can change the underlying element for a one-off situation like this with the component prop (Keeping the variant styles intact). Take a look at the CodeSandbox. The underlying element is changed to h4 and h3 by adding the component key.

Element should directly be the provided variant but since the component passed is hardcoded to span it does not get overridden unless explicitly provided.

Also, providing your own custom variant mapping is not working.

ZeeshanTamboli avatar Jul 01 '22 13:07 ZeeshanTamboli

@ZeeshanTamboli It seems we can easily remove the line to fix it. If the solution is good, I wanna create a PR.

diff --git a/packages/mui-material/src/ListItemText/ListItemText.js b/packages/mui-material/src/ListItemText/ListItemText.js
index 29b4f5a768..17d2e21be0 100644
--- a/packages/mui-material/src/ListItemText/ListItemText.js
+++ b/packages/mui-material/src/ListItemText/ListItemText.js
@@ -84,7 +84,6 @@ const ListItemText = React.forwardRef(function ListItemText(inProps, ref) {
       <Typography
         variant={dense ? 'body2' : 'body1'}
         className={classes.primary}
-        component="span"
         display="block"
         {...primaryTypographyProps}
       >

iamxukai avatar Aug 09 '22 17:08 iamxukai

@iamxukai We will have to fallback to span if no variant is provided. Also, we need to support custom variantMapping if passed to the primaryTypographyProps.

We will also need tests for it.

You can try it out. Thanks.

ZeeshanTamboli avatar Aug 10 '22 06:08 ZeeshanTamboli