korbit-ai[bot] commented on code in PR #31765:
URL: https://github.com/apache/superset/pull/31765#discussion_r1908521611
##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -126,7 +126,163 @@ function Echart(
useEffect(() => {
if (!divRef.current) return;
if (!chartRef.current) {
- chartRef.current = init(divRef.current);
+ chartRef.current = init(divRef.current, null, {
+ locale: {
+ time: {
+ month: [
+ t('January'),
+ t('February'),
+ t('March'),
+ t('April'),
+ t('May'),
+ t('June'),
+ t('July'),
+ t('August'),
+ t('September'),
+ t('October'),
+ t('November'),
+ t('December'),
+ ],
+ monthAbbr: [
+ t('Jan'),
+ t('Feb'),
+ t('Mar'),
+ t('Apr'),
+ t('May'),
+ t('Jun'),
+ t('Jul'),
+ t('Aug'),
+ t('Sep'),
+ t('Oct'),
+ t('Nov'),
+ t('Dec'),
+ ],
+ dayOfWeek: [
+ t('Sunday'),
+ t('Monday'),
+ t('Tuesday'),
+ t('Wednesday'),
+ t('Thursday'),
+ t('Friday'),
+ t('Saturday'),
+ ],
+ dayOfWeekAbbr: [
+ t('Sun'),
+ t('Mon'),
+ t('Tue'),
+ t('Wed'),
+ t('Thu'),
+ t('Fri'),
+ t('Sat'),
+ ],
+ },
+ legend: {
+ selector: {
+ all: t('All'),
+ inverse: t('Inv'),
+ },
+ },
+ toolbox: {
+ brush: {
+ title: {
+ rect: t('Box Select'),
+ polygon: t('Lasso Select'),
+ lineX: t('Horizontally Select'),
+ lineY: t('Vertically Select'),
+ keep: t('Keep Selections'),
+ clear: t('Clear Selections'),
+ },
+ },
+ dataView: {
+ title: t('Data View'),
+ lang: [t('Data View'), t('Close'), 'Refresh'],
+ },
Review Comment:
### Missing Translation Wrapper <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The 'Refresh' string in the dataView.lang array is not wrapped in a
translation function t(), unlike other UI strings.
###### Why this matters
This will cause inconsistency in localization where the 'Refresh' text will
always appear in English regardless of the user's selected language.
###### Suggested change
Wrap 'Refresh' string with the translation function t():
```typescript
dataView: {
title: t('Data View'),
lang: [t('Data View'), t('Close'), t('Refresh')],
},
```
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:3c13b7a7-c254-422c-adcf-cfcba8539c0a -->
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/formatters.ts:
##########
@@ -76,24 +77,40 @@ export const getYAxisFormatter = (
export function getTooltipTimeFormatter(
format?: string,
+ timeGrain?: TimeGranularity,
): TimeFormatter | StringConstructor {
+ if (
+ timeGrain === TimeGranularity.QUARTER ||
+ timeGrain === TimeGranularity.MONTH ||
+ timeGrain === TimeGranularity.YEAR
+ ) {
+ return getTimeFormatter(undefined, timeGrain);
+ }
if (format === SMART_DATE_ID) {
- return getSmartDateDetailedFormatter();
+ return getTimeFormatter(SMART_DATE_DETAILED_ID, timeGrain);
}
Review Comment:
### Incorrect Override of Smart Date Format <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
When SMART_DATE_ID format is specified, the code overrides it with
SMART_DATE_DETAILED_ID, which contradicts the user's explicit format choice.
###### Why this matters
Users selecting SMART_DATE_ID format will unexpectedly receive a different
formatting style than requested, potentially causing confusion and
inconsistency in the application's behavior.
###### Suggested change
Respect the user's format choice by modifying the code to:
```typescript
if (format === SMART_DATE_ID) {
return getTimeFormatter(SMART_DATE_ID, timeGrain);
}
```
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:fc3c285b-d1ab-4f73-b7c2-9b2121abb449 -->
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
