From 49dc864d4e3fb1a97c986a24f0f15f224953cf81 Mon Sep 17 00:00:00 2001
From: Tom Robinson <tlrobinson@gmail.com>
Date: Fri, 19 May 2017 15:24:37 -0700
Subject: [PATCH] Cleanup click interactions with brushing, fix scatter drill
 through

---
 .../components/OnClickOutsideWrapper.jsx      |   4 +-
 .../visualizations/lib/LineAreaBarRenderer.js | 128 ++++++++++--------
 .../visualizations/lib/graph/brush.js         |   4 +-
 3 files changed, 72 insertions(+), 64 deletions(-)

diff --git a/frontend/src/metabase/components/OnClickOutsideWrapper.jsx b/frontend/src/metabase/components/OnClickOutsideWrapper.jsx
index d8c8d7bf01e..700ff3fa880 100644
--- a/frontend/src/metabase/components/OnClickOutsideWrapper.jsx
+++ b/frontend/src/metabase/components/OnClickOutsideWrapper.jsx
@@ -30,14 +30,14 @@ export default class OnClickOutsideWrapper extends Component {
                 document.addEventListener("keydown", this._handleKeyPress, false);
             }
             if (this.props.dismissOnClickOutside) {
-                window.addEventListener("click", this._handleClick, true);
+                window.addEventListener("mousedown", this._handleClick, true);
             }
         }, 0);
     }
 
     componentWillUnmount() {
         document.removeEventListener("keydown", this._handleKeyPress, false);
-        window.removeEventListener("click", this._handleClick, true);
+        window.removeEventListener("mousedown", this._handleClick, true);
         clearTimeout(this._timeout);
 
         // remove from the stack after a delay, if it is removed through some other
diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
index af7a705ac3d..caf7ab01f25 100644
--- a/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
+++ b/frontend/src/metabase/visualizations/lib/LineAreaBarRenderer.js
@@ -385,72 +385,80 @@ function applyChartTooltips(chart, series, isStacked, isScalarSeries, onHoverCha
         }
 
         if (onVisualizationClick) {
-            chart.selectAll(".bar, .dot, .area, .bubble")
-                .style({ "cursor": "pointer" })
-                .on("click", function(d) {
-                    const seriesIndex = determineSeriesIndexFromElement(this, isStacked);
-                    const card = series[seriesIndex].card;
-                    const isSingleSeriesBar = this.classList.contains("bar") && series.length === 1;
-
-                    let clicked: ?ClickObject;
-                    if (Array.isArray(d.key)) { // scatter
-                        clicked = {
-                            value: d.key[2],
-                            column: cols[2],
-                            dimensions: [
-                                { value: d.key[0], column: cols[0] },
-                                { value: d.key[1], column: cols[1] }
-                            ],
-                            origin: d.key._origin
-                        }
-                    } else if (isScalarSeries) {
-                        // special case for multi-series scalar series, which should be treated as scalars
-                        clicked = {
-                            value: d.data.value,
-                            column: series[seriesIndex].data.cols[1]
-                        };
-                    } else if (d.data) { // line, area, bar
-                        if (!isSingleSeriesBar) {
-                            cols = series[seriesIndex].data.cols;
-                        }
-                        clicked = {
-                            value: d.data.value,
-                            column: cols[1],
-                            dimensions: [
-                                { value: d.data.key, column: cols[0] }
-                            ]
-                        }
-                    } else {
-                        clicked = {
-                            dimensions: []
-                        };
+            const onClick = function(d) {
+                const seriesIndex = determineSeriesIndexFromElement(this, isStacked);
+                const card = series[seriesIndex].card;
+                const isSingleSeriesBar = this.classList.contains("bar") && series.length === 1;
+
+                let clicked: ?ClickObject;
+                if (Array.isArray(d.key)) { // scatter
+                    clicked = {
+                        value: d.key[2],
+                        column: cols[2],
+                        dimensions: [
+                            { value: d.key[0], column: cols[0] },
+                            { value: d.key[1], column: cols[1] }
+                        ].filter(({ column }) =>
+                            // don't include aggregations since we can't filter on them
+                            column.source !== "aggregation"
+                        ),
+                        origin: d.key._origin
                     }
-
-                    // handle multiseries
-                    if (clicked && series.length > 1) {
-                        if (card._breakoutColumn) {
-                            // $FlowFixMe
-                            clicked.dimensions.push({
-                                value: card._breakoutValue,
-                                column: card._breakoutColumn
-                            });
-                        }
+                } else if (isScalarSeries) {
+                    // special case for multi-series scalar series, which should be treated as scalars
+                    clicked = {
+                        value: d.data.value,
+                        column: series[seriesIndex].data.cols[1]
+                    };
+                } else if (d.data) { // line, area, bar
+                    if (!isSingleSeriesBar) {
+                        cols = series[seriesIndex].data.cols;
                     }
-
-                    if (card._seriesIndex != null) {
-                        // $FlowFixMe
-                        clicked.seriesIndex = card._seriesIndex;
+                    clicked = {
+                        value: d.data.value,
+                        column: cols[1],
+                        dimensions: [
+                            { value: d.data.key, column: cols[0] }
+                        ]
                     }
+                } else {
+                    clicked = {
+                        dimensions: []
+                    };
+                }
 
-                    if (clicked) {
-                        const isLine = this.classList.contains("dot");
-                        onVisualizationClick({
-                            ...clicked,
-                            element: isLine ? this : null,
-                            event: isLine ? null : d3.event,
+                // handle multiseries
+                if (clicked && series.length > 1) {
+                    if (card._breakoutColumn) {
+                        // $FlowFixMe
+                        clicked.dimensions.push({
+                            value: card._breakoutValue,
+                            column: card._breakoutColumn
                         });
                     }
-                });
+                }
+
+                if (card._seriesIndex != null) {
+                    // $FlowFixMe
+                    clicked.seriesIndex = card._seriesIndex;
+                }
+
+                if (clicked) {
+                    const isLine = this.classList.contains("dot");
+                    onVisualizationClick({
+                        ...clicked,
+                        element: isLine ? this : null,
+                        event: isLine ? null : d3.event,
+                    });
+                }
+            }
+
+            chart.selectAll(".dot, .area, .bubble")
+                .style({ "cursor": "pointer" })
+                .on("click", onClick);
+            chart.selectAll(".bar")
+                .style({ "cursor": "pointer" })
+                .on("mousedown", onClick);
         }
     });
 }
diff --git a/frontend/src/metabase/visualizations/lib/graph/brush.js b/frontend/src/metabase/visualizations/lib/graph/brush.js
index 02b222fd4e2..68133fc1b3f 100644
--- a/frontend/src/metabase/visualizations/lib/graph/brush.js
+++ b/frontend/src/metabase/visualizations/lib/graph/brush.js
@@ -68,8 +68,8 @@ export function initBrush(parent, child, onBrushChange, onBrushEnd) {
         if (e.keyCode === KEYCODE_ESCAPE) {
             // set the "cancelled" flag
             cancelled = true;
-            // hide the brush
-            parent.select(".brush").style("opacity", 0);
+            // dispatch a mouseup to end brushing early
+            window.dispatchEvent(new MouseEvent("mouseup"));
         }
     };
 
-- 
GitLab