[GitHub] fabianmenges commented on a change in pull request #3594: [Bugfix]: Fixing bug in label generation for multiple groupbys

[gi...@git.apache.org]

fabianmenges commented on a change in pull request #3594: [Bugfix]: Fixing bug in label generation for multiple groupbys
URL: https://github.com/apache/incubator-superset/pull/3594#discussion_r142729045
 
 

 ##########
 File path: superset/assets/visualizations/nvd3_vis.js
 ##########
 @@ -77,17 +77,16 @@ function getMaxLabelSize(container, axisClass) {
 /* eslint-disable camelcase */
 function formatLabel(column, verbose_map) {
   let label;
-  if (verbose_map) {
-    if (Array.isArray(column) && column.length) {
-      label = verbose_map[column[0]];
-      if (column.length > 1) {
-        label += `, ${column.slice(1).join(', ')}`;
-      }
-    } else {
-      label = verbose_map[column];
+  if (Array.isArray(column) && column.length) {
+    label = verbose_map[column[0]] || column[0];
 
 Review comment:
   Yeah, so my change for renaming on the backend is still around somewhere #3437.
   
   Anyways your change #3563 as well as the original code also assumed it in the first position. 
   There are some advantages (that we currently don't use) to do it in the front end. 
   E.g. we could have a checkbox that would you switch between them or you could have custom verbose maps per slice instead of per datasource. And updating the charts would not require requests to the application.
   
   So if we document and define that metrics always have to come before groupys we should be fine doing it in the UI.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services