-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
feat: allow setting calendar for i18n date formatting #6430
Conversation
Let me think about it first... 🤔 We probably want to allow something like |
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6430--docusaurus-2.netlify.app/ |
Size Change: +261 B (0%) Total Size: 806 kB
ℹ️ View Unchanged
|
I don't know, that API would look weird to me and maybe too specific? What are we trying to solve in the first place? I don't remember precisely Are there any 'en-GB' users that do not want to format dates as If some user wants to use locale key const i18n = {
defaultLocale: "en",
locales: ["en"],
localeConfigs: {
en: {
localeCode: "en-GB"
}
}
}; |
Yes, maybe we want to merge this with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably add this implementation for now, looks good enough 👍
@@ -24,6 +24,8 @@ export function getDefaultLocaleConfig(locale: string): I18nLocaleConfig { | |||
label: getDefaultLocaleLabel(locale), | |||
direction: getLangDir(locale), | |||
htmlLang: locale, | |||
// If the locale name includes -u-ca-xxx the calendar will be defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add a test case for this? Using something like ar-u-ca-islamicc
?
and a test case for an invalid locale, ensuring it falls back and fail-safe instead of crashing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use new Intl.DisplayNames
to construct the label, which is gonna crash on terribly malformed locale names anyways. new Intl.Locale
is more resilient than new Intl.DisplayNames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This currently works fine: yarn start:website --locale "My-weird-locale"
But anything more fancy will crash: yarn start:website --locale "My weird locale"
But ok we can implement that later, considering we don't have anything setup to accept properly an invalid locale name
Motivation
Close #5440
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
None...yet