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

[docs] MUI free template has a typo in Typography's theme set up #44324

Closed
wohenjue opened this issue Nov 5, 2024 · 2 comments · Fixed by #44338
Closed

[docs] MUI free template has a typo in Typography's theme set up #44324

wohenjue opened this issue Nov 5, 2024 · 2 comments · Fixed by #44338
Assignees
Labels
bug 🐛 Something doesn't work design This is about UI or UX design, please involve a designer docs Improvements or additions to the documentation package: material-ui Specific to @mui/material

Comments

@wohenjue
Copy link

wohenjue commented Nov 5, 2024

Steps to reproduce

Go to https://github.com/mui/material-ui/blob/v6.1.6/docs/data/material/getting-started/templates/shared-theme/themePrimitives.js.
Line 163 fontFamily: ['"Inter", "sans-serif"'].join(','), should be changed to fontFamily: ["Inter", "sans-serif"].join(','),
Notice how the original array only has one element which is a string.

Current behavior

Browser interprets "Inter", "sans-serif" as the font.

Expected behavior

Browser choose "Inter" or "sans-serif" in that order.

Context

I borrowed the template and found the font doesn't work in chrome/safari but works in edge.

Your environment

chrome, safari render differently from edge

Search keywords: typo

@wohenjue wohenjue added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 5, 2024
@zannager zannager added the component: Typography The React component. label Nov 5, 2024
@ZeeshanTamboli ZeeshanTamboli added docs Improvements or additions to the documentation and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 7, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title MUI free template has a typo in Typography's theme set up [docs] MUI free template has a typo in Typography's theme set up Nov 7, 2024
@ZeeshanTamboli ZeeshanTamboli added package: material-ui Specific to @mui/material design This is about UI or UX design, please involve a designer and removed component: Typography The React component. labels Nov 7, 2024
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Nov 9, 2024

Yes, the current code is incorrect. However, the expected behavior you mentioned also needs adjusting: CSS-defined generic font-family names like sans-serif shouldn’t be quoted. The browser should apply it as:

font-family: Inter, sans-serif;

To fix this, remove .join(',') and use a simple string:

--- a/docs/data/material/getting-started/templates/shared-theme/themePrimitives.ts
+++ b/docs/data/material/getting-started/templates/shared-theme/themePrimitives.ts
@@ -183,7 +183,7 @@ export const getDesignTokens = (mode: PaletteMode) => {
       },
     },
     typography: {
-      fontFamily: ['"Inter", "sans-serif"'].join(','),
+      fontFamily: 'Inter, sans-serif',
       h1: {
         fontSize: defaultTheme.typography.pxToRem(48),
         fontWeight: 600,

@ZeeshanTamboli ZeeshanTamboli added the bug 🐛 Something doesn't work label Nov 9, 2024
Copy link

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

Note

@wohenjue How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work design This is about UI or UX design, please involve a designer docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants