You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "artemonsh (via GitHub)" <gi...@apache.org> on 2023/01/26 08:45:52 UTC

[GitHub] [superset] artemonsh commented on a diff in pull request #22772: chore: Localization of several charts and elements pt. 2

artemonsh commented on code in PR #22772:
URL: https://github.com/apache/superset/pull/22772#discussion_r1087557415


##########
superset-frontend/plugins/legacy-plugin-chart-calendar/src/Calendar.js:
##########
@@ -88,7 +88,8 @@ function Calendar(element, props) {
   Object.keys(metricsData).forEach(metric => {
     const calContainer = div.append('div');
     if (showMetricName) {
-      calContainer.text(`Metric: ${verboseMap[metric] || metric}`);
+      // eslint-disable-next-line prefer-template
+      calContainer.text(t('Metric') + `: ${verboseMap[metric] || metric}`);

Review Comment:
   Unfortunately, words inside template literals are not being localized properly. In fact, they don't even appear in the file `messages.pot` and could not be ever translated. I created an issue some time ago regarding this https://github.com/apache/superset/issues/21784. I think that creating constant variables `***_TEXT = t('...')` and then passing them inside template literal is one of the best solutions.
   
   For verification go run `scripts/babel_update.sh` locally to generate `messages.pot` file and reveal that the message `Drill to detail by` ([link to file](https://github.com/apache/superset/blob/master/superset-frontend/src/components/Chart/DrillDetail/DrillDetailMenuItems.tsx#L207)) is present in the `messages.pot` but not on the 207 line, where template literal is used! 
   ![image](https://user-images.githubusercontent.com/53895552/214790779-5f512273-eeb1-466c-9974-3e6c9e99486b.png)
   
   It is not obvious for some cases, though. For instance, in the reviewed code above the word `Metric` is localized in other files without using template literals, so it is present in `messages.pot` file and hence can be localized even if it is placed inside template literal. But for more uncommon words/messages it does not work as they are presented only once in the code with template literals only. I hope I was clear and managed to describe the problem we are facing. 
   
   Actually, I started changing template literals in the [last PR](https://github.com/apache/superset/pull/22150) (search for `// eslint-disable-next-line prefer-template`) :yum:
   
   



-- 
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-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org