korbit-ai[bot] commented on code in PR #35020:
URL: https://github.com/apache/superset/pull/35020#discussion_r2323066921
##########
superset-frontend/plugins/legacy-plugin-chart-calendar/src/utils.ts:
##########
@@ -28,3 +29,67 @@ export const getFormattedUTCTime = (
const offset = date.getTimezoneOffset() * 60 * 1000;
return getTimeFormatter(timeFormat)(date.getTime() - offset);
};
+
+/**
+ * Returns a locale-aware time formatter.
+ * Falls back to Supersetβs default getTimeFormatter if locale is not
available.
+ */
+export function getLocaleTimeFormatter(format?: string) {
+ try {
+ // Use browser/OS locale automatically (e.g., Russian if
BABEL_DEFAULT_LOCALE=ru)
+ const locale = navigator.language || 'en-US';
+ const d3Locale = d3.timeFormatLocale(getD3LocaleDefinition(locale));
Review Comment:
### Missing Locale Formatter Cache <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The locale definition is recomputed on every formatter creation without
caching.
###### Why this matters
Repeatedly creating locale formatters adds unnecessary overhead, especially
when the same locale is used multiple times.
###### Suggested change β *Feature Preview*
Implement a cache for locale formatters:
```typescript
const localeFormatterCache = new Map();
export function getLocaleTimeFormatter(format?: string) {
try {
const locale = navigator.language || 'en-US';
if (!localeFormatterCache.has(locale)) {
localeFormatterCache.set(
locale,
d3.timeFormatLocale(getD3LocaleDefinition(locale))
);
}
return localeFormatterCache.get(locale).format(format || '%B');
} catch (e) {
return getTimeFormatter(format);
}
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6ba36479-e5bf-4e6c-bcc0-f50c9e966a6c/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6ba36479-e5bf-4e6c-bcc0-f50c9e966a6c?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6ba36479-e5bf-4e6c-bcc0-f50c9e966a6c?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6ba36479-e5bf-4e6c-bcc0-f50c9e966a6c?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6ba36479-e5bf-4e6c-bcc0-f50c9e966a6c)
</details>
<sub>
π¬ Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:435b377f-8294-4431-9514-8f4e71fa4961 -->
[](435b377f-8294-4431-9514-8f4e71fa4961)
##########
superset-frontend/plugins/legacy-plugin-chart-calendar/src/utils.ts:
##########
@@ -28,3 +29,67 @@
const offset = date.getTimezoneOffset() * 60 * 1000;
return getTimeFormatter(timeFormat)(date.getTime() - offset);
};
+
+/**
+ * Returns a locale-aware time formatter.
+ * Falls back to Supersetβs default getTimeFormatter if locale is not
available.
+ */
+export function getLocaleTimeFormatter(format?: string) {
+ try {
+ // Use browser/OS locale automatically (e.g., Russian if
BABEL_DEFAULT_LOCALE=ru)
+ const locale = navigator.language || 'en-US';
Review Comment:
### Incomplete Locale Matching Logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The locale detection doesn't handle the full range of locale formats that
navigator.language can return, such as zh-Hans-CN or pt-BR.
###### Why this matters
Many valid browser locales will fall through to the default English locale
because the switch statement only handles 'ru' and 'ru-RU', potentially
providing an unexpected English format instead of the closest matching locale.
###### Suggested change β *Feature Preview*
Implement a more robust locale matching system:
```typescript
function getD3LocaleDefinition(locale: string) {
// Extract primary language code
const primaryLanguage = locale.toLowerCase().split(/[-_]/)[0];
switch (primaryLanguage) {
case 'ru':
return {
// Russian locale definition
};
// Add other languages here
default:
return {
// English locale definition
};
}
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d413da9-ffcc-4875-89bf-dc85b7ab15f8/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d413da9-ffcc-4875-89bf-dc85b7ab15f8?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d413da9-ffcc-4875-89bf-dc85b7ab15f8?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d413da9-ffcc-4875-89bf-dc85b7ab15f8?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3d413da9-ffcc-4875-89bf-dc85b7ab15f8)
</details>
<sub>
π¬ Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:62943054-4580-4b6a-8d1d-343a56ebe60d -->
[](62943054-4580-4b6a-8d1d-343a56ebe60d)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
