From 0aa0f641ab110bc9a71de310fe8b9efd5f105c53 Mon Sep 17 00:00:00 2001
From: Bruce Martin <bruce@chanzuckerberg.com>
Date: Fri, 3 May 2019 11:14:30 -0700
Subject: [PATCH] improve selection interaction with clip changes (#744)

* brush interactions with underlying dataframe updates improved

* improve comment

* reset selection state upon clip
---
 .../components/brushableHistogram/index.js    | 41 +++++++++++++++----
 client/src/reducers/categoricalSelection.js   |  3 +-
 client/src/reducers/continuousSelection.js    |  3 +-
 client/src/reducers/graphSelection.js         |  1 +
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/client/src/components/brushableHistogram/index.js b/client/src/components/brushableHistogram/index.js
index f12063a0..80b303a4 100644
--- a/client/src/components/brushableHistogram/index.js
+++ b/client/src/components/brushableHistogram/index.js
@@ -38,12 +38,11 @@ class HistogramBrush extends React.Component {
     return varData.col(field);
   }
 
-  calcHistogramCache = memoize((world, field) => {
+  calcHistogramCache = memoize((col, field) => {
     /*
      recalculate expensive stuff, notably bins, summaries, etc.
     */
     const histogramCache = {};
-    const col = HistogramBrush.getColumn(world, field);
     const values = col.asArray();
     const summary = col.summarize();
     const { min: domainMin, max: domainMax } = summary;
@@ -87,23 +86,42 @@ class HistogramBrush extends React.Component {
   componentDidUpdate(prevProps) {
     const { field, world, continuousSelection } = this.props;
     const { x, y, bins, svgRef } = this._histogram;
+    let { brushXselection, brushX } = this.state;
+    let forceBrushUpdate = false;
 
-    if (world !== prevProps.world) {
-      this.renderAxesBrushBins(x, y, bins, svgRef, field);
+    /*
+    Update our axis if the underlying dataframe column has changed
+    */
+    const dfColumn = HistogramBrush.getColumn(world, field);
+    const oldDfColumn = HistogramBrush.getColumn(
+      prevProps.world,
+      prevProps.field
+    );
+    if (dfColumn !== oldDfColumn) {
+      ({ brushXselection, brushX } = this.renderAxesBrushBins(
+        x,
+        y,
+        bins,
+        svgRef,
+        field
+      ));
+      forceBrushUpdate = true;
     }
 
     /*
     if the selection has changed, ensure that the brush correctly reflects
     the underlying selection.
     */
-    if (continuousSelection !== prevProps.continuousSelection) {
+    if (
+      forceBrushUpdate ||
+      continuousSelection !== prevProps.continuousSelection
+    ) {
       const { isObs, isUserDefined, isDiffExp } = this.props;
       const myName = makeContinuousDimensionName(
         { isObs, isUserDefined, isDiffExp },
         field
       );
       const range = continuousSelection[myName];
-      const { brushXselection, brushX } = this.state;
       if (brushXselection) {
         const selection = d3.brushSelection(brushXselection.node());
         if (!range && selection) {
@@ -140,6 +158,8 @@ class HistogramBrush extends React.Component {
 
       // ignore programmatically generated events
       if (!d3.event.sourceEvent) return;
+      // ignore cascading events, which are programmatically generated
+      if (d3.event.sourceEvent.sourceEvent) return;
 
       if (d3.event.selection) {
         dispatch({
@@ -175,6 +195,8 @@ class HistogramBrush extends React.Component {
 
       // ignore programmatically generated events
       if (!d3.event.sourceEvent) return;
+      // ignore cascading events, which are programmatically generated
+      if (d3.event.sourceEvent.sourceEvent) return;
 
       if (d3.event.selection) {
         let _range;
@@ -222,7 +244,8 @@ class HistogramBrush extends React.Component {
 
   drawHistogram(svgRef) {
     const { field, world } = this.props;
-    const histogramCache = this.calcHistogramCache(world, field);
+    const col = HistogramBrush.getColumn(world, field);
+    const histogramCache = this.calcHistogramCache(col, field);
     const { x, y, bins } = histogramCache;
     this._histogram = { x, y, bins, svgRef };
   }
@@ -348,7 +371,9 @@ class HistogramBrush extends React.Component {
     svg.selectAll(".axis path").style("stroke", "rgb(230,230,230)");
     svg.selectAll(".axis line").style("stroke", "rgb(230,230,230)");
 
-    this.setState({ brushX, brushXselection });
+    const newState = { brushX, brushXselection };
+    this.setState(newState);
+    return newState;
   }
 
   render() {
diff --git a/client/src/reducers/categoricalSelection.js b/client/src/reducers/categoricalSelection.js
index 400d6547..170ce8d4 100644
--- a/client/src/reducers/categoricalSelection.js
+++ b/client/src/reducers/categoricalSelection.js
@@ -20,7 +20,8 @@ const CategoricalSelection = (
   switch (action.type) {
     case "initial data load complete (universe exists)":
     case "set World to current selection":
-    case "reset World to eq Universe": {
+    case "reset World to eq Universe":
+    case "set clip quantiles": {
       const { world } = nextSharedState;
       return ControlsHelpers.createCategoricalSelection(
         maxCategoryItems(prevSharedState),
diff --git a/client/src/reducers/continuousSelection.js b/client/src/reducers/continuousSelection.js
index d8eb9431..459aa575 100644
--- a/client/src/reducers/continuousSelection.js
+++ b/client/src/reducers/continuousSelection.js
@@ -2,7 +2,8 @@ import { makeContinuousDimensionName } from "../util/nameCreators";
 
 const ContinuousSelection = (state = {}, action) => {
   switch (action.type) {
-    case "reset World to eq Universe": {
+    case "reset World to eq Universe":
+    case "set clip quantiles": {
       return {};
     }
     case "continuous metadata histogram start":
diff --git a/client/src/reducers/graphSelection.js b/client/src/reducers/graphSelection.js
index 0e8572e3..6fde8926 100644
--- a/client/src/reducers/graphSelection.js
+++ b/client/src/reducers/graphSelection.js
@@ -6,6 +6,7 @@ const GraphSelection = (
   action
 ) => {
   switch (action.type) {
+    case "set clip quantiles":
     case "reset World to eq Universe": {
       return {
         ...state,