From 31c00b6438a25e8968dcfe4b5f0caa8117a8cc51 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Thu, 18 Feb 2016 16:28:42 -0800
Subject: [PATCH] Cleanup label display logic and margin adjustment

---
 .../src/components/dashboard/dashboard.css    |  4 +-
 .../src/visualizations/lib/CardRenderer.js    | 99 ++++++++++---------
 2 files changed, 56 insertions(+), 47 deletions(-)

diff --git a/frontend/src/components/dashboard/dashboard.css b/frontend/src/components/dashboard/dashboard.css
index a9080e092a4..3d02753c975 100644
--- a/frontend/src/components/dashboard/dashboard.css
+++ b/frontend/src/components/dashboard/dashboard.css
@@ -277,12 +277,12 @@
 
 
 .mute-yl .dc-chart .axis.y,
-.mute-yl .dc-chart .y-axis-label {
+.mute-yl .dc-chart .y-axis-label.y-label {
   opacity: 0;
 }
 
 .mute-yr .dc-chart .axis.yr,
-.mute-yr .dc-chart .yr-axis-label {
+.mute-yr .dc-chart .y-axis-label.yr-label {
   opacity: 0;
 }
 
diff --git a/frontend/src/visualizations/lib/CardRenderer.js b/frontend/src/visualizations/lib/CardRenderer.js
index 1527c8156aa..b49312318f3 100644
--- a/frontend/src/visualizations/lib/CardRenderer.js
+++ b/frontend/src/visualizations/lib/CardRenderer.js
@@ -99,17 +99,16 @@ function applyChartLegend(chart, card) {
     }
 }
 
-function applyChartTimeseriesXAxis(chart, card, cols, xValues) {
+function applyChartTimeseriesXAxis(chart, series, xValues) {
     // setup an x-axis where the dimension is a timeseries
-    let settings = card.visualization_settings;
-
-    // set the axis label
+    const settings = series[0].card.visualization_settings;
+    const dimensionColumn = series[0].data.cols[0];
     if (settings.xAxis.labels_enabled) {
-        chart.xAxisLabel((settings.xAxis.title_text || null) || cols[0].display_name);
+        chart.xAxisLabel(settings.xAxis.title_text || getFriendlyName(dimensionColumn));
         chart.renderVerticalGridLines(settings.xAxis.gridLine_enabled);
 
-        if (cols[0] && cols[0].unit) {
-            chart.xAxis().tickFormat(d => formatValue(d, cols[0]));
+        if (dimensionColumn && dimensionColumn.unit) {
+            chart.xAxis().tickFormat(d => formatValue(d, dimensionColumn));
         } else {
             chart.xAxis().tickFormat(d3.time.format.multi([
                 [".%L",    (d) => d.getMilliseconds()],
@@ -124,14 +123,14 @@ function applyChartTimeseriesXAxis(chart, card, cols, xValues) {
         }
 
         // Compute a sane interval to display based on the data granularity, domain, and chart width
-        const { interval, count } = computeTimeseriesTicksInterval(xValues, cols[0], chart.width(), MIN_PIXELS_PER_TICK.x);
+        const { interval, count } = computeTimeseriesTicksInterval(xValues, dimensionColumn, chart.width(), MIN_PIXELS_PER_TICK.x);
         chart.xAxis().ticks(d3.time[interval], count);
     } else {
         chart.xAxis().ticks(0);
     }
 
     // compute the data interval
-    const { interval, count } = computeTimeseriesDataInverval(xValues, cols[0]);
+    const { interval, count } = computeTimeseriesDataInverval(xValues, dimensionColumn);
 
     // compute the domain
     let xDomain = d3.extent(xValues);
@@ -146,10 +145,11 @@ function applyChartTimeseriesXAxis(chart, card, cols, xValues) {
     chart.xUnits((start, stop) => Math.ceil(1 + moment(stop).diff(start, interval) / count));
 }
 
-function applyChartOrdinalXAxis(chart, card, cols, xValues) {
-    let settings = card.visualization_settings;
+function applyChartOrdinalXAxis(chart, series, xValues) {
+    const settings = series[0].card.visualization_settings;
+    const dimensionColumn = series[0].data.cols[0];
     if (settings.xAxis.labels_enabled) {
-        chart.xAxisLabel(settings.xAxis.title_text || cols[0].display_name);
+        chart.xAxisLabel(settings.xAxis.title_text || getFriendlyName(dimensionColumn));
         chart.renderVerticalGridLines(settings.xAxis.gridLine_enabled);
         chart.xAxis().ticks(xValues.length);
         adjustTicksIfNeeded(chart.xAxis(), chart.width(), MIN_PIXELS_PER_TICK.x);
@@ -165,7 +165,7 @@ function applyChartOrdinalXAxis(chart, card, cols, xValues) {
             let visibleKeys = xValues.filter((v, i) => i % keyInterval === 0);
             chart.xAxis().tickValues(visibleKeys);
         }
-        chart.xAxis().tickFormat(d => formatValue(d, cols[0]));
+        chart.xAxis().tickFormat(d => formatValue(d, dimensionColumn));
     } else {
         chart.xAxis().ticks(0);
         chart.xAxis().tickFormat('');
@@ -175,18 +175,28 @@ function applyChartOrdinalXAxis(chart, card, cols, xValues) {
         .xUnits(dc.units.ordinal);
 }
 
-function applyChartYAxis(chart, card, cols) {
-    let settings = card.visualization_settings;
+function applyChartYAxis(chart, series, yAxisSplit) {
+    let settings = series[0].card.visualization_settings;
     if (settings.yAxis.labels_enabled) {
-        chart.yAxisLabel(settings.yAxis.title_text || getFriendlyName(cols[1]));
         chart.renderHorizontalGridLines(true);
         chart.elasticY(true);
 
-        // Very small charts (i.e., Dashboard Cards) tend to render with an excessive number of ticks
-        // set some limits on the ticks per pixel and adjust if needed
+        // left
+        if (settings.yAxis.title_text) {
+            chart.yAxisLabel(settings.yAxis.title_text);
+        } else if (yAxisSplit[0].length === 1) {
+            chart.yAxisLabel(getFriendlyName(series[yAxisSplit[0][0]].data.cols[1]));
+        }
         adjustTicksIfNeeded(chart.yAxis(), chart.height(), MIN_PIXELS_PER_TICK.y);
-        if (chart.rightYAxis) {
-            adjustTicksIfNeeded(chart.rightYAxis(), chart.height(), MIN_PIXELS_PER_TICK.y);
+
+        // right
+        if (yAxisSplit.length > 1) {
+            if (yAxisSplit[1].length === 1) {
+                chart.rightYAxisLabel(getFriendlyName(series[yAxisSplit[1][0]].data.cols[1]));
+            }
+            if (chart.rightYAxis) {
+                adjustTicksIfNeeded(chart.rightYAxis(), chart.height(), MIN_PIXELS_PER_TICK.y);
+            }
         }
     } else {
         chart.yAxis().ticks(0);
@@ -394,9 +404,18 @@ function lineAndBarOnRender(chart, card) {
         }
     });
 
-    // adjust the margins to fit the Y-axis tick label sizes, and rerender
-    chart.margins().left = chart.select(".axis.y").node().getBBox().width + 30;
-    chart.margins().bottom = chart.select(".axis.x").node().getBBox().height + 30;
+    function adjustMargin(margin, direction, axisSelector, labelSelector) {
+        let axis = chart.select(axisSelector).node();
+        let label = chart.select(labelSelector).node();
+        let axisSize = axis ? axis.getBoundingClientRect()[direction] : 0;
+        let labelSize = label ? label.getBoundingClientRect()[direction] : 0;
+        chart.margins()[margin] = axisSize + labelSize + 15;
+    }
+
+    // adjust the margins to fit the X and Y axis tick and label sizes, and rerender
+    adjustMargin("bottom", "height", ".axis.x",  ".x-axis-label");
+    adjustMargin("left",   "width",  ".axis.y",  ".y-axis-label.y-label");
+    adjustMargin("right",  "width",  ".axis.yr", ".y-axis-label.yr-label");
 
     chart.render();
 }
@@ -438,27 +457,17 @@ export let CardRenderer = {
     },
 
     lineAreaBar(element, chartType, { series, onHoverChange, onRender }) {
-        let { card, data: result } = series[0];
+        const colors = getCardColors(series[0].card);
 
-        const colors = getCardColors(card);
-
-        let isTimeseries = dimensionIsTimeseries(result);
-        let isStacked = chartType === "area";
-        let isLinear = false;
+        const isTimeseries = dimensionIsTimeseries(series[0].data);
+        const isStacked = chartType === "area";
+        const isLinear = false;
 
         // validation.  we require at least 2 rows for line charting
-        if (result.cols.length < 2) {
+        if (series[0].data.cols.length < 2) {
             return;
         }
 
-        // pre-process data
-        let data = result.rows.map((row) => {
-            // IMPORTANT: clone the data if you are going to modify it in any way
-            let tuple = row.slice(0);
-            tuple[0] = (isTimeseries) ? new Date(row[0]) : row[0];
-            return tuple;
-        });
-
         // build crossfilter dataset + dimension + base group
         let dataset = crossfilter();
         series.map((s, index) =>
@@ -488,11 +497,11 @@ export let CardRenderer = {
                 chart.stack(groups[i]);
             }
 
-            applyChartLineBarSettings(chart, card, chartType, isLinear, isTimeseries);
+            applyChartLineBarSettings(chart, series[0].card, chartType, isLinear, isTimeseries);
 
             chart.ordinalColors(colors);
         } else {
-            chart = initializeChart(card, element, "compositeChart")
+            chart = initializeChart(series[0].card, element, "compositeChart")
 
             let subCharts = series.map(s =>
                 dc[getDcjsChartType(series[0].card.display)](chart)
@@ -505,7 +514,7 @@ export let CardRenderer = {
                     .colors(colors[index % colors.length])
                     .useRightYAxis(yAxisSplit.length > 1 && yAxisSplit[1].includes(index))
 
-                applyChartLineBarSettings(subChart, card, chartType, isLinear, isTimeseries);
+                applyChartLineBarSettings(subChart, series[0].card, chartType, isLinear, isTimeseries);
             });
 
             chart
@@ -541,14 +550,14 @@ export let CardRenderer = {
         // x-axis settings
         // TODO: we should support a linear (numeric) x-axis option
         if (isTimeseries) {
-            applyChartTimeseriesXAxis(chart, card, result.cols, xValues);
+            applyChartTimeseriesXAxis(chart, series, xValues);
         } else {
-            applyChartOrdinalXAxis(chart, card, result.cols, xValues);
+            applyChartOrdinalXAxis(chart, series, xValues);
         }
 
         // y-axis settings
         // TODO: if we are multi-series this could be split axis
-        applyChartYAxis(chart, card, result.cols, data);
+        applyChartYAxis(chart, series, yAxisSplit);
 
         applyChartTooltips(chart, (e, d, seriesIndex) => {
             if (onHoverChange) {
@@ -569,7 +578,7 @@ export let CardRenderer = {
         chart.render();
 
         // apply any on-rendering functions
-        lineAndBarOnRender(chart, card);
+        lineAndBarOnRender(chart, series[0].card);
 
         onRender && onRender({ yAxisSplit });
     },
-- 
GitLab