From 9e7ecfa279b6897c6fa7a1e354d5caaf9906a9ab Mon Sep 17 00:00:00 2001
From: Severiano Badajoz <sbadajoz@chanzuckerberg.com>
Date: Tue, 23 Jun 2020 11:59:39 -0700
Subject: [PATCH] check if bin creation returned null before rendering charts
 (#1576)

* check if bin creation returned null before rendering charts

* refactor chart rendering into functions (#1577)

* little fixes from PR

* reintroduce fix to check for null values

* change getAllByClass to return element

* slice instead

* new stackedbar test

* feedback-1573-test (#1579)

* feedback-1573-test

* enable whole test set

* revert tests

Co-authored-by: Timmy Huang <tihuan@users.noreply.github.com>
---
 .../src/components/categorical/value/index.js | 159 ++++++++++++------
 1 file changed, 110 insertions(+), 49 deletions(-)

diff --git a/client/src/components/categorical/value/index.js b/client/src/components/categorical/value/index.js
index 39644210..1a669d42 100644
--- a/client/src/components/categorical/value/index.js
+++ b/client/src/components/categorical/value/index.js
@@ -22,11 +22,14 @@ import { labelPrompt, isLabelErroneous } from "../labelUtil";
 import MiniHistogram from "../../miniHistogram";
 import MiniStackedBar from "../../miniStackedBar";
 
+const VALUE_HEIGHT = 11;
+const CHART_WIDTH = 100;
+
 /* this is defined outside of the class so we can use it in connect() */
 function _currentLabelAsString(ownProps) {
   const { categorySummary, categoryIndex } = ownProps;
   // when called as a function, the String() constructor performs type conversion,
-  // and returns a primtive string.
+  // and returns a primitive string.
   return String(categorySummary.categoryValues[categoryIndex]);
 }
 
@@ -76,6 +79,13 @@ class CategoryValue extends React.Component {
     }
   }
 
+  // If coloring by and this isn't the colorAccessor and it isn't being edited
+  get shouldRenderStackedBarOrHistogram() {
+    const { colorAccessor, isColorBy, annotations } = this.props;
+
+    return colorAccessor && !isColorBy && !annotations.isEditingLabelName;
+  }
+
   getLabel() {
     const { categoryIndex, categorySummary } = this.props;
     const label = categorySummary.categoryValues[categoryIndex];
@@ -297,7 +307,7 @@ class CategoryValue extends React.Component {
     height
   ) => {
     /*
-      Knowing that colorScale is based off continuous data, 
+      Knowing that colorScale is based off continuous data,
       createHistogramBins fetches the continuous data in relation to the cells relevant to the category value.
       It then separates that data into 50 bins for drawing the mini-histogram
     */
@@ -339,8 +349,8 @@ class CategoryValue extends React.Component {
     categoryValue,
     width
   ) => {
-    /* 
-      Knowing that the color scale is based off of categorical data, 
+    /*
+      Knowing that the color scale is based off of categorical data,
       createOccupancyStack obtains a map showing the number if cells per colored value
       Using the colorScale a stack of colored bars is drawn representing the map
      */
@@ -408,6 +418,99 @@ class CategoryValue extends React.Component {
     return false;
   }
 
+  renderMiniStackedBar = (categoryValue) => {
+    const {
+      categoricalSelection,
+      colorAccessor,
+      colorScale,
+      metadataField,
+      world,
+    } = this.props;
+
+    const isColorBy = metadataField === colorAccessor;
+
+    if (
+      !this.shouldRenderStackedBarOrHistogram ||
+      !categoricalSelection[colorAccessor] ||
+      isColorBy
+    ) {
+      return null;
+    }
+
+    const { domainValues, scale, domain, occupancy } =
+      this.createStackedGraphBins(
+        world,
+        metadataField,
+        colorAccessor,
+        categoryValue,
+        CHART_WIDTH
+      ) ?? {};
+
+    if (!domainValues || !scale || !domain || !occupancy) {
+      return null;
+    }
+
+    return (
+      <MiniStackedBar
+        /* eslint-disable react/jsx-props-no-spreading -- Disable unneeded on next release of eslint-config-airbnb */
+        {...{
+          colorScale,
+          domainValues,
+          scale,
+          domain,
+          occupancy,
+        }}
+        /* eslint-enable react/jsx-props-no-spreading -- enable */
+        height={VALUE_HEIGHT}
+        width={CHART_WIDTH}
+      />
+    );
+  };
+
+  renderMiniHistogram = (categoryValue) => {
+    const {
+      categoricalSelection,
+      colorAccessor,
+      colorScale,
+      world,
+      metadataField,
+    } = this.props;
+
+    if (
+      !this.shouldRenderStackedBarOrHistogram ||
+      categoricalSelection[colorAccessor]
+    ) {
+      return null;
+    }
+
+    const { xScale, yScale, bins } =
+      this.createHistogramBins(
+        world,
+        metadataField,
+        colorAccessor,
+        categoryValue,
+        CHART_WIDTH,
+        VALUE_HEIGHT
+      ) ?? {};
+
+    return (
+      <MiniHistogram
+        /* eslint-disable react/jsx-props-no-spreading -- Disable unneeded on next release of eslint-config-airbnb */
+        {...{
+          colorScale,
+          xScale,
+          yScale,
+          bins,
+        }}
+        /* eslint-enable react/jsx-props-no-spreading -- enable */
+        obsOrVarContinuousFieldDisplayName={colorAccessor}
+        domainLabel={categoryValue}
+        height={VALUE_HEIGHT}
+        width={CHART_WIDTH}
+      />
+    );
+  };
+
   render() {
     const {
       categoricalSelection,
@@ -424,7 +527,6 @@ class CategoryValue extends React.Component {
       // our lint doesn't like jsx spread, we are version pinned to prevent api change on their part
       flippedProps,
       isDilated,
-      world,
       categorySummary,
     } = this.props;
     const ontologyEnabled = ontology?.enabled ?? false;
@@ -455,12 +557,12 @@ class CategoryValue extends React.Component {
 
     const valueToggleLabel = `value-toggle-checkbox-${displayString}`;
 
-    const VALUE_HEIGHT = 11;
     const LEFT_MARGIN = 60;
     const CHECKBOX = 26;
     const CELL_NUMBER = 50;
     const ANNO_MENU = 26;
     const LABEL_MARGIN = 16;
+    const CHART_MARGIN = 24;
 
     const otherElementsWidth =
       LEFT_MARGIN +
@@ -469,9 +571,6 @@ class CategoryValue extends React.Component {
       LABEL_MARGIN +
       (isUserAnno ? ANNO_MENU : 0);
 
-    const CHART_WIDTH = 100;
-    const CHART_MARGIN = 24;
-
     const labelWidth =
       colorAccessor && !isColorBy
         ? globals.leftSidebarWidth -
@@ -596,46 +695,8 @@ class CategoryValue extends React.Component {
             ) : null}
           </div>
           <span style={{ flexShrink: 0 }}>
-            {colorAccessor && !isColorBy && !annotations.isEditingLabelName ? (
-              categoricalSelection[colorAccessor] ? (
-                <MiniStackedBar
-                  /* eslint-disable react/jsx-props-no-spreading -- Disable unneeded on next release of eslint-config-airbnb */
-                  {...{
-                    colorScale,
-                    ...this.createStackedGraphBins(
-                      world,
-                      metadataField,
-                      colorAccessor,
-                      value,
-                      CHART_WIDTH
-                    ),
-                  }}
-                  /* eslint-enable react/jsx-props-no-spreading -- enable */
-                  height={VALUE_HEIGHT}
-                  width={CHART_WIDTH}
-                />
-              ) : (
-                <MiniHistogram
-                  /* eslint-disable react/jsx-props-no-spreading -- Disable unneeded on next release of eslint-config-airbnb */
-                  {...{
-                    colorScale,
-                    ...this.createHistogramBins(
-                      world,
-                      metadataField,
-                      colorAccessor,
-                      value,
-                      CHART_WIDTH,
-                      VALUE_HEIGHT
-                    ),
-                  }}
-                  /* eslint-enable react/jsx-props-no-spreading -- enable */
-                  obsOrVarContinuousFieldDisplayName={colorAccessor}
-                  domainLabel={value}
-                  height={VALUE_HEIGHT}
-                  width={CHART_WIDTH}
-                />
-              )
-            ) : null}
+            {this.renderMiniStackedBar(value)}
+            {this.renderMiniHistogram(value)}
           </span>
         </div>
         <div>