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

[l10n] Add Arabic (ar) locale #15693

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[l10n] Add Arabic (ar) locale #15693

wants to merge 5 commits into from

Conversation

0xenn
Copy link

@0xenn 0xenn commented Dec 2, 2024

Fixes #15650

@mui-bot
Copy link

mui-bot commented Dec 2, 2024

Localization writing tips ✍️

Seems you are updating localization 🌍 files.

Thank you for contributing to the localization! 🎉 To make your PR perfect, here is a list of elements to check: ✔️

  • Verify if the PR title respects the release format. Here are two examples (depending if you update or add a locale file)

    [l10n] Improve Swedish (sv-SE) locale
    [l10n] Add Danish (da-DK) locale

  • Update the documentation of supported locales by running pnpm l10n
  • Verify that you have added an export line in src/locales/index.ts for the new locale.
  • Run pnpm docs:api which should add your new translation to the list of exported interfaces.
  • Clean files with pnpm prettier.

Deploy preview: https://deploy-preview-15693--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against f2bf8b5

@@ -36,4 +36,5 @@ export * from './urPK';
export * from './viVN';
export * from './zhCN';
export * from './zhHK';
export * from './ar';

Choose a reason for hiding this comment

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

Based on other languages, it is recommended to specify the country of the language, such as arMR or arEG

Copy link
Author

@0xenn 0xenn Dec 2, 2024

Choose a reason for hiding this comment

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

@ET-TOUNANI As arabic is spoken in many countries they have many dialects just like eu. So i have taken that consideration and named it. Kindly suggest! ☺️

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @0xenn! 🙏
Could you please run the suggested scripts to fix CI errors?

Given that @mui/material has localization for different Arabic dialects, should we also consider having it separate?
I'm not an expert in Arabic language, so, can't comment on the best approach.
But if it is anything similar to how Norwegian (Bokmål and Nynorsk) differ, then we should probably split it per dialect. 🤔

For languages, where a common form for certain thing is acceptable, I wouldn't mind landing on a generic locale (like having en instead of enUS and endGB, etc.)

@LukasTy LukasTy added l10n localization component: pickers This is the name of the generic UI component, not the React module! labels Dec 2, 2024
@LukasTy LukasTy changed the title [pickers] Added Support for Arabic (ar) Locale in @mui/x-date-pickers [l10n] Add Arabic (ar) locale Dec 2, 2024
@0xenn
Copy link
Author

0xenn commented Dec 2, 2024

@LukasTy Done. Thanks for your feedback.😊

@LukasTy
Copy link
Member

LukasTy commented Dec 2, 2024

Done. Thanks for your feedback.😊

What do you think about the localization itself?
Do you think that a single Arabic localization can work for every country speaking Arabic?
As I mentioned, @mui/material has different localization for different countries.
If you strongly feel that this is not needed, then we'd need to add an entry for

ar: 'Arabic'

in the localeNames file.
After this you would need to re-run pnpm l10n, which would update the json file with data for each localization.
And, preferably place the export in the alphabetic order.

@0xenn
Copy link
Author

0xenn commented Dec 2, 2024

@LukasTy as mentioned by you that @mui/material has as different localization for different countries. In this case also i think it would be much better to have that distinction because there might be slight differences between dialects You can have a look on language code and let me know. Even i don't have that much idea 😅.

@LukasTy
Copy link
Member

LukasTy commented Dec 2, 2024

In this case also i think it would be much better to have that distinction because there might be slight differences between dialects You can have a look on language code and let me know. Even i don't have that much idea 😅.

The main issue is consistency.
@mui/material has localization per dialect, so does the @mui/x-data-grid.
If we were to introduce a generic ar locale, that might confuse users when products from the same company have different solutions for the same language. 🙈

WDYT @mui/xgrid and @mui/material-ui? Does it make sense to segment Arabic locales precisely? 🤔

@0xenn
Copy link
Author

0xenn commented Dec 2, 2024

🤣.Understood then it's better to have localization per dialect. In that case which country do i need to specify as i have used generic arabic.

@LukasTy
Copy link
Member

LukasTy commented Dec 2, 2024

Understood then it's better to have localization per dialect.

If it's ok for you, let's wait for any feedback from the tagged teams before proceeding with any changes. 😉

In that case which country do i need to specify as i have used generic arabic.

Whichever you need or can provide. 👍 Preferably—the most popular/common.
P.S. If a given language is not in the following list, you'll need to add an entry for it.

'ar-EG': 'Arabic (Egypt)',
'ar-SA': 'Arabic (Saudi Arabia)',
'ar-SD': 'Arabic (Sudan)',

@0xenn
Copy link
Author

0xenn commented Dec 2, 2024

If it's ok for you, let's wait for any feedback from the tagged teams before proceeding with any changes. 😉

Sure No problem. Kindly notify it

Whichever you need or can provide. 👍 Preferably—the most popular/common. P.S. If a given language is not in the following list, you'll need to add an entry for it.

Ok. 😁

@DiegoAndai
Copy link
Member

Hey @LukasTy, I asked the Material UI team, and we agree that we should try to keep consistency and follow https://mui.com/material-ui/guides/localization/#supported-locales, which would be best.

@LukasTy
Copy link
Member

LukasTy commented Dec 2, 2024

I asked the Material UI team, and we agree that we should try to keep consistency and follow https://mui.com/material-ui/guides/localization/#supported-locales, which would be best.

Thanks for the reply @DiegoAndai. 🙏
We recently noticed mui/material-ui#31847.
Does that change the decision?
Maybe this PR could be the catalyst for finalizing that issue. 🤔
The "optimizer" gene" in me is all in for simplifying where possible.

One could say that we could even rename enUS to en 🙈

@DiegoAndai
Copy link
Member

We recently noticed mui/material-ui#31847.
Does that change the decision?

I'm not aware of that issue, but seems like there's no decision there either.

@0xenn
Copy link
Author

0xenn commented Dec 3, 2024

Whichever you need or can provide. 👍 Preferably—the most popular/common. P.S. If a given language is not in the following list, you'll need to add an entry for it.

'ar-EG': 'Arabic (Egypt)',
'ar-SA': 'Arabic (Saudi Arabia)',
'ar-SD': 'Arabic (Sudan)',

So that means for now we can go with these three according to https://mui.com/material-ui/guides/localization/#supported-locales right.

@MBilalShafi
Copy link
Member

WDYT @mui/xgrid and @mui/material-ui? Does it make sense to segment Arabic locales precisely? 🤔

I'm not a native Arabic, however, I speak a language that uses the same alphabet system and many words are common with Arabic. As I understand, although different dialects exist they are very similar in terms of communication and a standard locale could cover more than 99% of the communication.

Thus, the proposal in mui/material-ui#31847 makes sense to me, I'd be in favor of doing it for X line of products in v8, and possibly deprecating the country-specific ones aiming to remove them in v9. The core line of products could follow it then.

@LukasTy
Copy link
Member

LukasTy commented Dec 3, 2024

Thank you for your input @MBilalShafi. 🙏
This further strengthens the position of optimizing locales. After all, it's just translations that users can override if something doesn't fit their use case. 🤷

Besides, we already have a couple (eu and mk) country independent locales. 👍

@0xenn could you update the PR with the mentioned suggestion (#15693 (comment)) of adding an entry in the localeNames file? 🤔

@LukasTy LukasTy added needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Dec 3, 2024
@0xenn
Copy link
Author

0xenn commented Dec 3, 2024

@LukasTy Done. Thanks ☺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! l10n localization needs cherry-pick The PR should be cherry-picked to master after merge v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pickers] Add Support for Arabic (ar) Locale in @mui/x-date-pickers
6 participants