From 4c4b6c41a1ee0c2f4172208389b616d666b0ea1d Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Mon, 19 Nov 2018 14:01:20 -0800
Subject: [PATCH] Minor improvements to Histogram viz (#6391)

* Minor improvements to Histogram viz

* prevent legend overflow (spill) by simply hiding it
* legend title only includes metric name when necessary
* control validator asking forcing at least one numeric column to be
selected

* Removing print()
---
 .../src/explore/controlPanels/Histogram.js    |  2 ++
 .../visualizations/Histogram/Histogram.css    |  3 +++
 .../visualizations/Histogram/Histogram.jsx    |  1 +
 superset/viz.py                               | 20 ++++++++++++-------
 4 files changed, 19 insertions(+), 7 deletions(-)
 create mode 100644 superset/assets/src/visualizations/Histogram/Histogram.css

diff --git a/superset/assets/src/explore/controlPanels/Histogram.js b/superset/assets/src/explore/controlPanels/Histogram.js
index 52896149e..186dda246 100644
--- a/superset/assets/src/explore/controlPanels/Histogram.js
+++ b/superset/assets/src/explore/controlPanels/Histogram.js
@@ -1,4 +1,5 @@
 import { t } from '@superset-ui/translation';
+import { nonEmpty } from '../validators';
 
 export default {
   controlPanelSections: [
@@ -29,6 +30,7 @@ export default {
       label: t('Numeric Columns'),
       description: t('Select the numeric columns to draw the histogram'),
       multi: true,
+      validators: [nonEmpty],
     },
     link_length: {
       label: t('No of Bins'),
diff --git a/superset/assets/src/visualizations/Histogram/Histogram.css b/superset/assets/src/visualizations/Histogram/Histogram.css
new file mode 100644
index 000000000..28b7d70e8
--- /dev/null
+++ b/superset/assets/src/visualizations/Histogram/Histogram.css
@@ -0,0 +1,3 @@
+.histogram {
+  overflow: hidden;
+}
diff --git a/superset/assets/src/visualizations/Histogram/Histogram.jsx b/superset/assets/src/visualizations/Histogram/Histogram.jsx
index 8414830a2..2b5451990 100644
--- a/superset/assets/src/visualizations/Histogram/Histogram.jsx
+++ b/superset/assets/src/visualizations/Histogram/Histogram.jsx
@@ -6,6 +6,7 @@ import { LegendOrdinal } from '@vx/legend';
 import { scaleOrdinal } from '@vx/scale';
 import { CategoricalColorNamespace } from '@superset-ui/color';
 import WithLegend from '../WithLegend';
+import './Histogram.css';
 
 const propTypes = {
   className: PropTypes.string,
diff --git a/superset/viz.py b/superset/viz.py
index 068a70dcf..e3e891ac0 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -1467,6 +1467,16 @@ class HistogramViz(BaseViz):
         d['groupby'] = []
         return d
 
+    def labelify(self, keys, column):
+        if isinstance(keys, str):
+            keys = (keys,)
+        # removing undesirable characters
+        labels = [re.sub(r'\W+', r'_', k) for k in keys]
+        if len(self.columns) > 1 or not self.groupby:
+            # Only show numeric column in label if there are many
+            labels = [column] + labels
+        return '__'.join(labels)
+
     def get_data(self, df):
         """Returns the chart data"""
         chart_data = []
@@ -1475,14 +1485,10 @@ class HistogramViz(BaseViz):
         else:
             groups = [((), df)]
         for keys, data in groups:
-            if isinstance(keys, str):
-                keys = (keys,)
-            # removing undesirable characters
-            keys = [re.sub(r'\W+', r'_', k) for k in keys]
             chart_data.extend([{
-                'key': '__'.join([c] + keys),
-                'values': data[c].tolist()}
-                for c in self.columns])
+                'key': self.labelify(keys, column),
+                'values': data[column].tolist()}
+                for column in self.columns])
         return chart_data