pomegranited commented on code in PR #31751:
URL: https://github.com/apache/superset/pull/31751#discussion_r1917784418


##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -123,10 +127,22 @@ function Echart(
     getEchartInstance: () => chartRef.current,
   }));
 
+  const localeObj = useSelector(
+    (state: ExplorePageState) => state?.common?.locale ?? 'en',
+  );
+  try {
+    const lang = require(
+      `echarts/lib/i18n/lang/${localeObj.toUpperCase()}`,
+    ).default;
+    echarts.registerLocale(localeObj.toUpperCase(), lang);
+  } catch (e) {
+    console.warn(`Locale ${localeObj} not supported in ECharts`);
+  }
+
   useEffect(() => {
     if (!divRef.current) return;
     if (!chartRef.current) {
-      chartRef.current = init(divRef.current);
+      chartRef.current = init(divRef.current, undefined, { locale: localeObj 
});

Review Comment:
   This `localeObj` also needs to be uppercased, so it might be clearer to just 
uppercase that string from the start, and use it everywhere?
   
   Here's my full diff -- I've tested it and it works well:
   
   ```diff
   --- 
a/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
   +++ 
b/superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx
   @@ -25,6 +25,11 @@ import React, {
      useLayoutEffect,
      useCallback,
    } from 'react';
   +
   +import { useSelector } from 'react-redux';
   +import { ExplorePageState } from 'src/explore/types';
   +import { registerLocale } from 'echarts/core';
   +
    import { styled } from '@superset-ui/core';
    import { ECharts, init } from 'echarts';
    import { EchartsHandler, EchartsProps, EchartsStylesProps } from '../types';
   @@ -62,10 +67,20 @@ function Echart(
        getEchartInstance: () => chartRef.current,
      }));
   
   +  const locale = useSelector(
   +    (state: ExplorePageState) => state?.common?.locale ?? 'en',
   +  ).toUpperCase();
   +  try {
   +    const lang = require(`echarts/lib/i18n/lang${locale}`).default;
   +    registerLocale(locale, lang);
   +  } catch (e) {
   +    console.warn(`Locale ${locale} not supported in ECharts`, e);
   +  }
   +
      useEffect(() => {
        if (!divRef.current) return;
        if (!chartRef.current) {
   -      chartRef.current = init(divRef.current);
   +      chartRef.current = init(divRef.current, null, { locale });
        }
   
        Object.entries(eventHandlers || {}).forEach(([name, handler]) => {
   @@ -79,7 +94,7 @@ function Echart(
        });
   
        chartRef.current.setOption(echartOptions, true);
   -  }, [echartOptions, eventHandlers, zrEventHandlers]);
   +  }, [echartOptions, eventHandlers, zrEventHandlers, locale]);
   
      // highlighting
      useEffect(() => {
   ```
   
   
https://github.com/user-attachments/assets/e508a49c-fbff-481c-803d-f37760dfee91
   
   
   
   



##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -123,10 +127,22 @@ function Echart(
     getEchartInstance: () => chartRef.current,
   }));
 
+  const localeObj = useSelector(
+    (state: ExplorePageState) => state?.common?.locale ?? 'en',
+  );
+  try {
+    const lang = require(
+      `echarts/lib/i18n/lang/${localeObj.toUpperCase()}`,

Review Comment:
   Nit small typo here, e.g 
https://github.com/apache/echarts/blob/master/i18n/langRU.js
   
   ```suggestion
         `echarts/lib/i18n/lang${localeObj.toUpperCase()}`,
   ```



##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -123,10 +127,22 @@ function Echart(
     getEchartInstance: () => chartRef.current,
   }));
 
+  const localeObj = useSelector(
+    (state: ExplorePageState) => state?.common?.locale ?? 'en',
+  );
+  try {
+    const lang = require(
+      `echarts/lib/i18n/lang/${localeObj.toUpperCase()}`,
+    ).default;
+    echarts.registerLocale(localeObj.toUpperCase(), lang);

Review Comment:
   nit: this is already imported above, so can just be:
   
   ```suggestion
       registerLocale(localeObj.toUpperCase(), lang);
   ```



-- 
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]

Reply via email to