Fix pulse table cell spacing, clean up objects passed to pulse formatting code from Clojure

frontend/src/metabase-shared/color_selector.js

@@ -8,16 +8,22 @@ global.console = {
   error: print,
 };
 
-global.makeCellBackgroundGetter = function(data, settings) {
-  data = JSON.parse(data);
-  settings = JSON.parse(settings);
+global.makeCellBackgroundGetter = function(
+  rowsJavaList,
+  colsJSON,
+  settingsJSON,
+) {
+  const rows = rowsJavaList;
+  const cols = JSON.parse(colsJSON);
+  const settings = JSON.parse(settingsJSON);
   try {
-    const getter = makeCellBackgroundGetter(data, settings);
+    const getter = makeCellBackgroundGetter(rows, cols, settings);
     return (value, rowIndex, colName) => {
       const color = getter(value, rowIndex, colName);
       if (color) {
         return roundColor(color);
       }
+      return null;
     };
   } catch (e) {
     print("ERROR", e);

frontend/src/metabase/visualizations/lib/table_format.js

 import { alpha, getColorScale } from "metabase/lib/colors";
 import _ from "underscore";
-import d3 from "d3";
 
 const CELL_ALPHA = 0.65;
 const ROW_ALPHA = 0.2;
 const GRADIENT_ALPHA = 0.75;
 
-export function makeCellBackgroundGetter(data, settings) {
-  const { rows, cols } = data;
+export function makeCellBackgroundGetter(rows, cols, settings) {
   const formats = settings["table.column_formatting"];
   const pivot = settings["table.pivot"];
   let formatters = {};
   let rowFormatters = [];
   try {
-    const columnExtents = computeColumnExtents(formats, data);
+    const columnExtents = computeColumnExtents(formats, rows, cols);
     formatters = compileFormatters(formats, columnExtents);
     rowFormatters = compileRowFormatters(formats, columnExtents);
   } catch (e) {
@@ -107,14 +105,32 @@ function compileFormatter(
   }
 }
 
-function computeColumnExtents(formats, data) {
+// NOTE: implement `extent` like this rather than using d3.extent since rows may
+// be a Java `List` rather than a JavaScript Array when used in Pulse formatting
+function extent(rows, colIndex) {
+  let min = Infinity;
+  let max = -Infinity;
+  const length = rows.length;
+  for (let i = 0; i < length; i++) {
+    const value = rows[i][colIndex];
+    if (value < min) {
+      min = value;
+    }
+    if (value > max) {
+      max = value;
+    }
+  }
+  return [min, max];
+}
+
+function computeColumnExtents(formats, rows, cols) {
   return _.chain(formats)
     .map(format => format.columns)
     .flatten()
     .uniq()
     .map(columnName => {
-      const colIndex = _.findIndex(data.cols, col => col.name === columnName);
-      return [columnName, d3.extent(data.rows, row => row[colIndex])];
+      const colIndex = _.findIndex(cols, col => col.name === columnName);
+      return [columnName, extent(rows, colIndex)];
     })
     .object()
     .value();

frontend/src/metabase/visualizations/visualizations/Table.jsx

@@ -109,8 +109,8 @@ export default class Table extends Component {
       readDependencies: ["table.pivot"],
     },
     "table._cell_background_getter": {
-      getValue([{ data }], settings) {
-        return makeCellBackgroundGetter(data, settings);
+      getValue([{ data: { rows, cols } }], settings) {
+        return makeCellBackgroundGetter(rows, cols, settings);
       },
       readDependencies: ["table.column_formatting", "table.pivot"],
     },

resources/frontend_shared/color_selector.js

@@ -14322,16 +14322,18 @@ global.console = {
   error: print
 };
-global.makeCellBackgroundGetter = function (data, settings) {
-  data = JSON.parse(data);
-  settings = JSON.parse(settings);
+global.makeCellBackgroundGetter = function (rowsJavaList, colsJSON, settingsJSON) {
+  var rows = rowsJavaList;
+  var cols = JSON.parse(colsJSON);
+  var settings = JSON.parse(settingsJSON);
   try {
-    var getter = (0, _table_format.makeCellBackgroundGetter)(data, settings);
+    var getter = (0, _table_format.makeCellBackgroundGetter)(rows, cols, settings);
     return function (value, rowIndex, colName) {
       var color = getter(value, rowIndex, colName);
       if (color) {
         return roundColor(color);
       }
+      return null;
     };
   } catch (e) {
     print("ERROR", e);
@@ -19673,10 +19675,6 @@ var _underscore = __webpack_require__(342);
 var _underscore2 = _interopRequireDefault(_underscore);
-var _d = __webpack_require__(125);
-
-var _d2 = _interopRequireDefault(_d);
-
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 function _toConsumableArray(arr) { if (Array.isArray(arr)) { for (var i = 0, arr2 = Array(arr.length); i < arr.length; i++) { arr2[i] = arr[i]; } return arr2; } else { return Array.from(arr); } }
@@ -19685,16 +19683,13 @@ var CELL_ALPHA = 0.65;
 var ROW_ALPHA = 0.2;
 var GRADIENT_ALPHA = 0.75;
-function makeCellBackgroundGetter(data, settings) {
-  var rows = data.rows,
-      cols = data.cols;
-
+function makeCellBackgroundGetter(rows, cols, settings) {
   var formats = settings["table.column_formatting"];
   var pivot = settings["table.pivot"];
   var formatters = {};
   var rowFormatters = [];
   try {
-    var columnExtents = computeColumnExtents(formats, data);
+    var columnExtents = computeColumnExtents(formats, rows, cols);
     formatters = compileFormatters(formats, columnExtents);
     rowFormatters = compileRowFormatters(formats, columnExtents);
   } catch (e) {
@@ -19841,16 +19836,32 @@ function compileFormatter(format, columnName, columnExtents) {
   }
 }
-function computeColumnExtents(formats, data) {
+// NOTE: implement `extent` like this rather than using d3.extent since rows may
+// be a Java `List` rather than a JavaScript Array when used in Pulse formatting
+function extent(rows, colIndex) {
+  var min = Infinity;
+  var max = -Infinity;
+  var length = rows.length;
+  for (var i = 0; i < length; i++) {
+    var value = rows[i][colIndex];
+    if (value < min) {
+      min = value;
+    }
+    if (value > max) {
+      max = value;
+    }
+  }
+  return [min, max];
+}
+
+function computeColumnExtents(formats, rows, cols) {
   return _underscore2.default.chain(formats).map(function (format) {
     return format.columns;
   }).flatten().uniq().map(function (columnName) {
-    var colIndex = _underscore2.default.findIndex(data.cols, function (col) {
+    var colIndex = _underscore2.default.findIndex(cols, function (col) {
       return col.name === columnName;
     });
-    return [columnName, _d2.default.extent(data.rows, function (row) {
-      return row[colIndex];
-    })];
+    return [columnName, extent(rows, colIndex)];
   }).object().value();
 }

src/metabase/pulse/color.clj

 (ns metabase.pulse.color
   "Namespaces that uses the Nashorn javascript engine to invoke some shared javascript code that we use to determine
   the background color of pulse table cells"
-  (:require [clojure.walk :as walk]
+  (:require [cheshire.core :as json]
             [metabase.query-processor.util :as qputil]
             [puppetlabs.i18n.core :refer [trs]]
             [schema.core :as s])
@@ -38,33 +38,6 @@
    :rows [[s/Any]]
    s/Any s/Any})
 
-(defn- map->js-map [m]
-  (let [^ScriptEngine engine       @js-engine
-        ^ScriptObjectMirror js-map (.eval engine "var map = {}; map")]
-    (doseq [[k v] m]
-      (.put js-map (name k) v))
-    js-map))
-
-(defn- coll->js-array [coll]
-  (let [^ScriptEngine engine         @js-engine
-        ^ScriptObjectMirror js-array (.eval engine "var array = []; array")]
-    (doseq [[idx item] (partition-all 2 (interleave (range) coll))]
-      (.put js-array (str idx) item))
-    js-array))
-
-(defn- convert-to-js-data
-  "When passing in function arguments from Clojure, it doesn't create proper JS maps and arrays but rather just uses
-  the Java objects. Not all JS functions work on these Java objects so this function walks the Clojure data structure
-  and dumps the data into a JS data structure. This ensures we get a real map/array that supports all of the regular
-  JS functions"
-  [x]
-  (->> x
-       (qputil/postwalk-pred map? map->js-map)
-       ;; This is for converting vectors/lists to a JS data type. This is handled separately from the map conversion
-       ;; (in a separate pass) because clojure.walk will also pass key value pairs in as a vector. Converting maps
-       ;; first, then converting vectors/lists prevents that issue
-       (qputil/postwalk-pred coll? coll->js-array)))
-
 (s/defn make-color-selector
   "Returns a curried javascript function (object) that can be used with `get-background-color` for delegating to JS
   code to pick out the correct color for a given cell in a pulse table. The logic for picking a color is somewhat
@@ -75,8 +48,8 @@
   (let [^Invocable engine @js-engine
         ;; Ideally we'd convert everything to JS data before invoking the function below, but converting rows would be
         ;; expensive. The JS code is written to deal with `rows` in it's native Nashorn format but since `cols` and
-        ;; `viz-settings` are small, convert those so that they're easier to work with once in JS code
-        js-fn-args (object-array [rows (convert-to-js-data cols) (convert-to-js-data viz-settings)])]
+        ;; `viz-settings` are small, pass those as JSON so that they can be deserialized to pure JS objects once in JS code
+        js-fn-args (object-array [rows (json/generate-string cols) (json/generate-string viz-settings)])]
     (.invokeFunction engine "makeCellBackgroundGetter" js-fn-args)))
 
 (defn get-background-color

src/metabase/pulse/render.clj

@@ -373,7 +373,9 @@
   create."
   [color-selector column-names header+rows]
   (let [{bar-width :bar-width :as header} (first header+rows)]
-    [:table {:style (style {:max-width (str "100%"), :white-space :nowrap, :padding-bottom :8px, :border-collapse :collapse})}
+    [:table {:style (style {:max-width (str "100%"), :white-space :nowrap, :padding-bottom :8px, :border-collapse :collapse})
+             :cellpadding "0"
+             :cellspacing "0"}
      [:thead
       [:tr
        (for [header-cell (:row header)]