[DataGrid] Include MUI core component localizations in `localeText` #1913

DanailH · 2021-06-16T10:18:41Z

I had a thought about how to solve the problem mentioned in the 2 tickets above and I have a possible solution without introducing any duplication and without adding regressions to the way the grid localization currently works.

I added a Breaking Change label because I needed to change some of the types but even with this we still support v5, people can use the localization with the theme and in addition, they can just provide all the localizations through the localeText prop.

The only duplication is that I needed to list the components for which there are localizations in the core (but without any actual values, only their names).

Preview: https://deploy-preview-1913--material-ui-x.netlify.app/components/data-grid/localization/

ToDo:

Update the Localization docs

dtassone · 2021-06-16T10:20:53Z

packages/grid/_modules_/grid/constants/localeTextConstants.ts

+  MuiAlert: {},
+  MuiBreadcrumbs: {},
+  MuiRating: {},
+  MuiAutocomplete: {},


We don't natively use all these components. Why should we add them all?

One question I would have is, let's say we want to implement Ant Design, as a design specification. How does these change fit?

We don't, correct but for some of them, it does make sense to have them. Technically we only use the MuiTablePagination so we can only keep that one and add the rest on demand if we happen to use them in the future. The benefit is that that way all the localizations are kept together.

@oliviertassinari regarding the Ant Design - I didn't understand the problem - is it if we use a different pagination component?

Actually, these are only here because the way that the getLocaleText API works is that it throws an error if the key doesn't exist. If that was not the case we woudn't need to keep these here at all.

I would keep only MuiTablePagination. If we add all of them it might create doubts for who is doing translations.

is it if we use a different pagination component?

Different design specs can be supported in different ways. The most likely path (>80%) we will take is that we will expose the same MuiPagination structure, but styled differently. The only thing that we would need to do is to reinject the new components. So we might be able to use the same translation keys, as we would display the same content. (I was initially worried about the coupling of the translation keys, all good)

oliviertassinari

It also seems that a large part of the issue was with the lack of documentation on his aspect (the core components). So adding something about it might help. I mean, we likely want to explain that developers have to translate the core components used. And how it works.

DanailH · 2021-06-16T12:12:48Z

It also seems that a large part of the issue was with the lack of documentation on his aspect (the core components). So adding something about it might help. I mean, we likely want to explain that developers have to translate the core components used. And how it works.

I know what you mean. The thing is that the solution that @m4theushw described here #1854 (comment) is missing from the docs (or at least from the Localization docs). Probably it makes sense to add a section for it.

oliviertassinari · 2021-06-16T12:23:20Z

the solution that @m4theushw described here #1854 (comment)

@DanailH Not necessarily, what I had in mind is to make sure we have the documentation counterpart of the solution we deploy. Solutions don't have to be source changes only, it could also heavily depends on the documentation.

We have a couple of different options, ranging from:

Hide the notion of core components to the developers, have each translation key flat in the locale, no need to update the docs
A documentation only fix, explain how the problem can be solved with the componentsProps prop.
The hybrid, what we have started to do here. We are partially coupled with the core components, but still get a single place where developers can find all the props that can be translated, updating the docs still seems important.

DanailH · 2021-06-16T12:37:32Z

@oliviertassinari Understood. I tried to go with 1. but listing the props flat is a problem since we have to respect the typings and more importantly because we support both v4 and v5.

Option 2. is the easiest and would require the least effort.
Option 3. - if we can agree on the implementation of course I will update the docs with what has changed and add examples.

m4theushw · 2021-06-16T13:02:48Z

packages/grid/_modules_/grid/utils/getGridLocalization.ts

          },
        },
-        ...coreTranslations?.components,


We need to revert this line and its v4 counterpart below. If another component from the core is used, it will not get translations.

@m4theushw It seems that you envision a different solution. This change seems to be OK/aligned with the intent to have all the translations happening inside MuiDataGrid.localeText, even for the core components.

I have no problem with appending the core translations into MuiDataGrid.localeText. My point is that if another component from the core (e.g. Alert) is used outside the DataGrid it will not get translations. That's because in the docs we recommend to use the locale from the data-grid package to create the theme. Open this CodeSandbox and note that the standalone TablePagination is not translated.

I see your worry - I will update the docs to match this implementation. That's why it's marked as a breaking change because you will no longer import all translations from the X package, you would need a mixed solution depending on your case.

Open this CodeSandbox and note that the standalone TablePagination is not translated.

The table component not translated sounds like the expected behavior, translations do no longer leak.

docs/src/pages/components/data-grid/localization/localization.md

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

packages/grid/_modules_/grid/models/api/gridLocaleTextApi.ts

docs/src/pages/components/data-grid/localization/localization.md

m4theushw · 2021-06-18T13:57:08Z

docs/src/pages/components/data-grid/localization/localization.md

+const theme = createMuiTheme(
+  {
+    palette: {
+      primary: { main: '#1976d2' },
+    },
+  },
+  bgBG,
+  coreBgBG,
+);


oliviertassinari · 2021-07-10T23:01:59Z

packages/grid/_modules_/grid/models/api/gridLocaleTextApi.ts

+import { Localization as CoreLocalization } from '@material-ui/core/locale';
+
+// @ts-expect-error We have different structure in v5
+type MuiComponentsLocalization = CoreLocalization['props'] | CoreLocalization['components'];
+
 /**
 * Set the types of the texts in the grid.
 */
-export interface GridLocaleText {
+export interface GridLocaleText extends Partial<MuiComponentsLocalization> {


This type is not correct, we extend any:

This should fail: https://codesandbox.io/s/material-demo-forked-d8djr?file=/demo.tsx, it doesn't. Fixed in [DataGrid] Fix localeText type #2117.

The tests of [docs] Fix DataTable.tsx demo in v4 material-ui#27196 shouldn't fail, they do. Fixed in [DataGrid] Fix localeText type #2117

It allows regressions like [DataGrid] Add missing localeText types #2118

Include core component translations without duplicating

ec6b49b

DanailH added bug 🐛 Something doesn't work breaking change component: data grid This is the name of the generic UI component, not the React module! labels Jun 16, 2021

DanailH requested review from dtassone, oliviertassinari and m4theushw June 16, 2021 10:18

DanailH self-assigned this Jun 16, 2021

dtassone reviewed Jun 16, 2021

View reviewed changes

Update docs

d425b3a

oliviertassinari reviewed Jun 16, 2021

View reviewed changes

m4theushw reviewed Jun 16, 2021

View reviewed changes

DanailH added 2 commits June 17, 2021 12:32

only add MuiTablePagination translations

8d9e5fc

Update docs

8f42808

dtassone reviewed Jun 17, 2021

View reviewed changes

docs/src/pages/components/data-grid/localization/localization.md Outdated Show resolved Hide resolved

fix docs

fd998f9

DanailH requested review from dtassone, m4theushw and oliviertassinari June 18, 2021 08:05

dtassone approved these changes Jun 18, 2021

View reviewed changes

m4theushw reviewed Jun 18, 2021

View reviewed changes

docs/src/pages/components/data-grid/localization/localization.md Outdated Show resolved Hide resolved

docs/src/pages/components/data-grid/localization/localization.md Outdated Show resolved Hide resolved

docs/src/pages/components/data-grid/localization/localization.md Show resolved Hide resolved

DanailH and others added 2 commits June 18, 2021 16:22

Update docs/src/pages/components/data-grid/localization/localization.md

16955a3

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

Update docs/src/pages/components/data-grid/localization/localization.md

186fd72

Co-authored-by: Matheus Wichman <matheushw@outlook.com>

DanailH requested a review from m4theushw June 18, 2021 13:22

oliviertassinari reviewed Jun 18, 2021

View reviewed changes

packages/grid/_modules_/grid/models/api/gridLocaleTextApi.ts Outdated Show resolved Hide resolved

docs/src/pages/components/data-grid/localization/localization.md Show resolved Hide resolved

DanailH added 2 commits June 18, 2021 16:50

Update docs

3de6ae8

Pull latest

9cd4f15

m4theushw approved these changes Jun 18, 2021

View reviewed changes

DanailH merged commit 04b1149 into mui:master Jun 18, 2021

oliviertassinari reviewed Jul 10, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Include MUI core component localizations in `localeText` #1913

[DataGrid] Include MUI core component localizations in `localeText` #1913

DanailH commented Jun 16, 2021 •

edited by oliviertassinari

dtassone Jun 16, 2021

oliviertassinari Jun 16, 2021 •

edited

DanailH Jun 16, 2021

m4theushw Jun 16, 2021

oliviertassinari Jun 16, 2021 •

edited

oliviertassinari left a comment •

edited

DanailH commented Jun 16, 2021 •

edited

oliviertassinari commented Jun 16, 2021 •

edited

DanailH commented Jun 16, 2021

m4theushw Jun 16, 2021

oliviertassinari Jun 16, 2021 •

edited

m4theushw Jun 16, 2021

DanailH Jun 17, 2021

oliviertassinari Jun 18, 2021 •

edited

m4theushw Jun 18, 2021

oliviertassinari Jul 10, 2021 •

edited

[DataGrid] Include MUI core component localizations in localeText #1913

[DataGrid] Include MUI core component localizations in localeText #1913

Conversation

DanailH commented Jun 16, 2021 • edited by oliviertassinari

dtassone Jun 16, 2021

Choose a reason for hiding this comment

oliviertassinari Jun 16, 2021 • edited

Choose a reason for hiding this comment

DanailH Jun 16, 2021

Choose a reason for hiding this comment

m4theushw Jun 16, 2021

Choose a reason for hiding this comment

oliviertassinari Jun 16, 2021 • edited

Choose a reason for hiding this comment

oliviertassinari left a comment • edited

Choose a reason for hiding this comment

DanailH commented Jun 16, 2021 • edited

oliviertassinari commented Jun 16, 2021 • edited

DanailH commented Jun 16, 2021

m4theushw Jun 16, 2021

Choose a reason for hiding this comment

oliviertassinari Jun 16, 2021 • edited

Choose a reason for hiding this comment

m4theushw Jun 16, 2021

Choose a reason for hiding this comment

DanailH Jun 17, 2021

Choose a reason for hiding this comment

oliviertassinari Jun 18, 2021 • edited

Choose a reason for hiding this comment

m4theushw Jun 18, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 10, 2021 • edited

Choose a reason for hiding this comment

[DataGrid] Include MUI core component localizations in `localeText` #1913

[DataGrid] Include MUI core component localizations in `localeText` #1913

DanailH commented Jun 16, 2021 •

edited by oliviertassinari

oliviertassinari Jun 16, 2021 •

edited

oliviertassinari Jun 16, 2021 •

edited

oliviertassinari left a comment •

edited

DanailH commented Jun 16, 2021 •

edited

oliviertassinari commented Jun 16, 2021 •

edited

oliviertassinari Jun 16, 2021 •

edited

oliviertassinari Jun 18, 2021 •

edited

oliviertassinari Jul 10, 2021 •

edited