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.

Sign up for GitHub

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] Disable translations #34820

Merged
merged 11 commits into from
Dec 1, 2022
Merged

[docs] Disable translations #34820

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6882659
[docs] Disable translations
mnajdova Oct 19, 2022
a8bdc0a
Fix lint issues
mnajdova Oct 19, 2022
e8557fe
Remove unused import
mnajdova Oct 19, 2022
9c5857c
Remove unnecessary package
mnajdova Oct 19, 2022
1f9ceff
Remove unused import
mnajdova Oct 19, 2022
b861c0d
Update docs/public/_redirects
mnajdova Oct 19, 2022
f3b9064
Update docs/public/_redirects
mnajdova Oct 19, 2022
5f6bd10
Merge branch 'master' into core/remove-translations
mnajdova Oct 26, 2022
db2336b
Merge branch 'core/remove-translations' of https://github.com/mnajdov…
mnajdova Oct 26, 2022
f28bbdc
Remove Crowdin from README and Backers
mnajdova Oct 26, 2022
17d66c3
Merge branch 'master' into core/remove-translations
mnajdova Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 2 additions & 43 deletions docs/pages/_app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import { loadCSS } from 'fg-loadcss/src/loadCSS';
import NextHead from 'next/head';
import PropTypes from 'prop-types';
import acceptLanguage from 'accept-language';
mnajdova marked this conversation as resolved.
Show resolved Hide resolved
import { useRouter } from 'next/router';
import { unstable_useEnhancedEffect as useEnhancedEffect } from '@mui/utils';
import pages from 'docs/src/pages';
import basePages from 'docs/data/base/pages';
import materialPages from 'docs/data/material/pages';
Expand All @@ -16,14 +14,9 @@ import PageContext from 'docs/src/modules/components/PageContext';
import GoogleAnalytics from 'docs/src/modules/components/GoogleAnalytics';
import { CodeCopyProvider } from 'docs/src/modules/utils/CodeCopy';
import { ThemeProvider } from 'docs/src/modules/components/ThemeContext';
import { pathnameToLanguage, getCookie } from 'docs/src/modules/utils/helpers';
import { LANGUAGES, LANGUAGES_IGNORE_PAGES } from 'docs/src/modules/constants';
import { LANGUAGES } from 'docs/src/modules/constants';
import { CodeVariantProvider } from 'docs/src/modules/utils/codeVariant';
import {
UserLanguageProvider,
useSetUserLanguage,
useUserLanguage,
} from 'docs/src/modules/utils/i18n';
import { UserLanguageProvider } from 'docs/src/modules/utils/i18n';
import DocsStyledEngineProvider from 'docs/src/modules/utils/StyledEngineProvider';
import createEmotionCache from 'docs/src/createEmotionCache';
import findActivePage from 'docs/src/modules/utils/findActivePage';
Expand All @@ -39,39 +32,6 @@ const clientSideEmotionCache = createEmotionCache();
// Set the locales that the documentation automatically redirects to.
acceptLanguage.languages(LANGUAGES);

function LanguageNegotiation() {
Copy link
Member

Choose a reason for hiding this comment

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

We should do the same in MUI X and synchronize docs deployment to production to avoid redirect loop for users that previously selected language different than en.
Here's what happens on preview deployment when user has userLanguage=pt cookie:

Screen.Recording.2022-10-19.at.11.17.20.mov

Copy link
Member

Choose a reason for hiding this comment

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

I'll open a PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch, let's keep this in mind.

Copy link
Member

Choose a reason for hiding this comment

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

mui/mui-x#6560

const setUserLanguage = useSetUserLanguage();
const router = useRouter();
const userLanguage = useUserLanguage();

useEnhancedEffect(() => {
const { userLanguage: userLanguageUrl, canonicalAs } = pathnameToLanguage(router.asPath);

// Only consider a redirection if coming to the naked folder.
if (userLanguageUrl === 'en') {
const preferedLanguage =
LANGUAGES.find((lang) => lang === getCookie('userLanguage')) ||
acceptLanguage.get(navigator.language) ||
userLanguage;

if (
userLanguage !== preferedLanguage &&
!process.env.BUILD_ONLY_ENGLISH_LOCALE &&
!LANGUAGES_IGNORE_PAGES(router.pathname)
) {
window.location =
preferedLanguage === 'en' ? canonicalAs : `/${preferedLanguage}${canonicalAs}`;
}
}

if (userLanguage !== userLanguageUrl) {
setUserLanguage(userLanguageUrl);
}
}, [router.pathname, router.asPath, setUserLanguage, userLanguage]);

return null;
}

let reloadInterval;

// Avoid infinite loop when "Upload on reload" is set in the Chrome sw dev tools.
Expand Down Expand Up @@ -216,7 +176,6 @@ function AppWrapper(props) {
))}
</NextHead>
<UserLanguageProvider defaultUserLanguage={pageProps.userLanguage}>
<LanguageNegotiation />
<CodeCopyProvider>
<CodeVariantProvider>
<PageContext.Provider value={{ activePage, pages: productPages }}>
Expand Down
2 changes: 2 additions & 0 deletions docs/public/_redirects
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ https://v4.material-ui.com/* https://v4.mui.com/:splat 301!
/base/api/trap-focus/ /base/api/focus-trap/ 301
/material-ui/experimental-api/css-variables/ /material-ui/experimental-api/css-theme-variables/overview/ 301
/joy-ui/main-features/perfect-dark-mode/ /joy-ui/main-features/dark-mode-optimization/ 301
/pt/* /:splat 301
/zh/* /:splat 301
Copy link
Member

@cherniavskii cherniavskii Oct 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
/pt/* /:splat 301
/zh/* /:splat 301
/pt/material-ui/* /material-ui/:splat 301
/zh/material-ui/* /material-ui/:splat 301
/pt/joy-ui/* /joy-ui/:splat 301
/zh/joy-ui/* /joy-ui/:splat 301
/pt/base/* /base/:splat 301
/zh/base/* /base/:splat 301
/pt/system/* /system/:splat 301
/zh/system/* /system/:splat 301

We can do this instead to avoid #34820 (comment) and keep /x redirects independent.
This will allow us to disable translations in MUI X in our own pace.
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense 👍 Would the translation on x still work if this PR gets merged tough?

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW now that this is committed, make sure to add the redirects in mui/mui-x#6560

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense 👍 Would the translation on x still work if this PR gets merged tough?

I think so - we still have LanguageNegotiation in our docs.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @bharatkashyap should we do something similar for toolpad? Is there support for translations there as well?

Copy link
Member

Choose a reason for hiding this comment

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

No, @mnajdova, there's no need for this in Toolpad since we don't have translations at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

@bharatkashyap
I would suggest removing LanguageNegotiation in Toolpad docs then:
https://github.com/mui/mui-toolpad/blob/3dccbbe1d4cb992acee77c775c73159beb785b4d/docs/pages/_app.js#L200

Because if you go to https://mui.com/toolpad/getting-started/overview/ and change language - you'll get 404:
https://mui.com/pt/toolpad/getting-started/overview/


# 2023

Expand Down
55 changes: 1 addition & 54 deletions docs/src/modules/components/AppSettingsDrawer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,7 @@ import SettingsBrightnessIcon from '@mui/icons-material/SettingsBrightness';
import FormatTextdirectionLToRIcon from '@mui/icons-material/FormatTextdirectionLToR';
import FormatTextdirectionRToLIcon from '@mui/icons-material/FormatTextdirectionRToL';
import { useChangeTheme } from 'docs/src/modules/components/ThemeContext';
import { pathnameToLanguage } from 'docs/src/modules/utils/helpers';
import NoSsr from '@mui/material/NoSsr';
import { LANGUAGES_LABEL } from 'docs/src/modules/constants';
import { useUserLanguage, useTranslate } from 'docs/src/modules/utils/i18n';
import { useRouter } from 'next/router';
import List from '@mui/material/List';
import ListItemButton from '@mui/material/ListItemButton';

const LOCALES = { zh: 'zh-CN', pt: 'pt-BR', es: 'es-ES' };
const CROWDIN_ROOT_URL = 'https://translate.mui.com/project/material-ui-docs/';
import { useTranslate } from 'docs/src/modules/utils/i18n';

const Heading = styled(Typography)(({ theme }) => ({
margin: '20px 0 10px',
Expand Down Expand Up @@ -54,10 +45,6 @@ function AppSettingsDrawer(props) {
const [mode, setMode] = React.useState(null);
const prefersDarkMode = useMediaQuery('(prefers-color-scheme: dark)');
const preferredMode = prefersDarkMode ? 'dark' : 'light';
const userLanguage = useUserLanguage();
const crowdInLocale = LOCALES[userLanguage] || userLanguage;
const router = useRouter();
const { canonicalAs } = pathnameToLanguage(router.asPath);

React.useEffect(() => {
// syncing with homepage, can be removed once all pages are migrated to CSS variables
Expand Down Expand Up @@ -94,11 +81,6 @@ function AppSettingsDrawer(props) {
changeTheme({ direction });
};

const handleLanguageClick = (language) => () => {
document.cookie = `userLanguage=${language.code};path=/;max-age=31536000`;
onClose();
};

return (
<Drawer
anchor="right"
Expand Down Expand Up @@ -189,41 +171,6 @@ function AppSettingsDrawer(props) {
{t('settings.rtl')}
</IconToggleButton>
</ToggleButtonGroup>
<Heading gutterBottom>{t('settings.language')}</Heading>
<NoSsr defer>
<List>
{LANGUAGES_LABEL.map((language) => (
<ListItemButton
component="a"
divider
data-no-markdown-link="true"
href={language.code === 'en' ? canonicalAs : `/${language.code}${canonicalAs}`}
key={language.code}
onClick={handleLanguageClick(language)}
selected={userLanguage === language.code}
lang={language.code}
hrefLang={language.code}
>
{language.text}
</ListItemButton>
))}
<ListItemButton
component="a"
href={
userLanguage === 'en'
? `${CROWDIN_ROOT_URL}`
: `${CROWDIN_ROOT_URL}${crowdInLocale}#/staging`
}
rel="noopener nofollow"
target="_blank"
key={userLanguage}
lang={userLanguage}
hrefLang="en"
>
{t('appFrame.helpToTranslate')}
</ListItemButton>
</List>
</NoSsr>
<Heading gutterBottom>{t('settings.color')}</Heading>
<Button
component="a"
Expand Down
12 changes: 2 additions & 10 deletions docs/src/modules/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ const CODE_VARIANTS = {
};

// Valid languages to server-side render in production
const LANGUAGES = ['en', 'zh', 'pt'];
const LANGUAGES = ['en'];
Copy link
Member

Choose a reason for hiding this comment

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

Do we need these constants? They're used in next.config.js, but maybe we should remove translation logic there as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed that we want to keep the infrastructure around the translations so that we can easily add it back if we decide in the future, this is why I decided to left these constants and not do any changes in the next.config.js, but I don't mind removing the logic there as well. cc @siriwatknp what do you think?

Copy link
Member

@siriwatknp siriwatknp Oct 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Let's remove them too. There are many places that use LANGUAGES, so I think let's keep them there, otherwise it will be a lot more work. The changes are sufficient.


// Server side rendered languages
const LANGUAGES_SSR = ['en', 'zh', 'pt'];
const LANGUAGES_SSR = ['en'];

// Work in progress
const LANGUAGES_IN_PROGRESS = LANGUAGES.slice();
Expand All @@ -18,14 +18,6 @@ const LANGUAGES_LABEL = [
code: 'en',
text: 'English',
},
{
code: 'zh',
text: '中文',
},
{
code: 'pt',
text: 'Português',
},
];

const LANGUAGES_IGNORE_PAGES = (pathname) => {
Expand Down