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] Inform about specific files for DataGrid locales #30411

Merged
merged 3 commits into from
Jan 13, 2022

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Dec 27, 2021

Fix mui/mui-x#3514

It is not clear that when using the DataGrid component locales must be imported from the DataGrid package.

Also the codesandbox did not work because the following line is using outerTheme to know if the documentation is in light or dark mode.

theme={(outerTheme) => createTheme(outerTheme, locales[locale])}

But the codeSandbox, does not declareanother them, so outerTheme is not defined, leading to a bug.

I removed the demo toolbar because there is an example on top of the demo

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 27, 2021

No bundle size changes

Generated by 🚫 dangerJS against c80e7d8

@mnajdova
Copy link
Member

mnajdova commented Dec 28, 2021

Another way would be to fix the line you've provided and handle cases when outerTheme does not exists, maybe something like:

-theme={(outerTheme) => createTheme(outerTheme, locales[locale])}
+theme={(outerTheme) => createTheme(outerTheme ?? {}, locales[locale])}

@alexfauquette
Copy link
Member Author

@mnajdova I do not manage to make your solution works because JS does not enter in the function. From what I understand, the bug occurs in emotion which is looking for the outerTheme because the theme is a function.

I found the following solution to make it works both in CodeSandbox and the doc

const theme = useTheme();
const themeWithLocale = createTheme(theme, locales[locale]); 

https://codesandbox.io/s/locales-material-demo-forked-0yj0w?file=/demo.tsx

But I'm not sure it is a good practice to show in the documentation

import * as locales from '@mui/material/locale';

export default function Locales() {
const [locale, setLocale] = React.useState('zhCN');

const theme = useTheme();
const themeWithLocale = createTheme(theme, locales[locale]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better, let's just memo this and change it only if the locale changes.

@alexfauquette
Copy link
Member Author

To test the new demo and check codeSandbox works:
https://deploy-preview-30411--material-ui.netlify.app/guides/localization/#Locales.tsx

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@mnajdova mnajdova merged commit e52ef5c into mui:master Jan 13, 2022
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
@zannager zannager added the docs Improvements or additions to the documentation label Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MUI Translation doesn't work
5 participants