From eb9d3044d12a27753f978e05d5f9ecef9f65dc71 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:00:18 -0700
Subject: [PATCH 01/15] update indent

---
 superset/assets/src/visualizations/table.css | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/superset/assets/src/visualizations/table.css b/superset/assets/src/visualizations/table.css
index 9af0c0e5f5f65..1b3b3aa382434 100644
--- a/superset/assets/src/visualizations/table.css
+++ b/superset/assets/src/visualizations/table.css
@@ -1,5 +1,5 @@
 .slice-grid .widget.table .slice_container {
-    overflow: auto !important;
+  overflow: auto !important;
 }
 
 .slice_container.table table.table {
@@ -9,21 +9,22 @@
 }
 
 .widget.table td.filtered {
-    background-color: #005a63;
-    color: white;
+  background-color: #005a63;
+  color: white;
 }
 
 .widget.table tr>th {
-    padding: 1px 5px !important;
-    font-size: small !important;
+  padding: 1px 5px !important;
+  font-size: small !important;
 }
 
 .widget.table tr>td {
-    padding: 1px 5px !important;
-    font-size: small !important;
+  padding: 1px 5px !important;
+  font-size: small !important;
 }
+
 table.table thead th.sorting:after, table.table thead th.sorting_asc:after, table.table thead th.sorting_desc:after {
-    top: 0px;
+  top: 0px;
 }
 
 .like-pre {
@@ -34,6 +35,7 @@ table.table thead th.sorting:after, table.table thead th.sorting_asc:after, tabl
   width: auto;
   max-width: unset;
 }
+
 .widget.table thead tr {
   height: 25px;
 }

From 1d1404e80d0ba91c41cc2ad00e940f698d1a1135 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:33:24 -0700
Subject: [PATCH 02/15] extract formData and data.

---
 superset/assets/src/chart/Chart.jsx         |   7 -
 superset/assets/src/visualizations/table.js | 169 +++++++++++++++-----
 2 files changed, 125 insertions(+), 51 deletions(-)

diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index 032f1d672ef49..93470335ddf64 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -168,13 +168,6 @@ class Chart extends React.PureComponent {
     );
   }
 
-  d3format(col, number) {
-    const { datasource } = this.props;
-    const format = (datasource.column_formats && datasource.column_formats[col]) || '0.3s';
-
-    return d3format(format, number);
-  }
-
   error(e) {
     this.props.actions.chartRenderingFailed(e, this.props.chartId);
   }
diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index a715d19acbf6d..d35bc16418161 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -1,39 +1,82 @@
 import d3 from 'd3';
+import PropTypes from 'prop-types';
+import $ from 'jquery';
 import dt from 'datatables.net-bs';
 import 'datatables.net-bs/css/dataTables.bootstrap.css';
 import dompurify from 'dompurify';
-
 import { fixDataTableBodyHeight, d3TimeFormatPreset } from '../modules/utils';
 import './table.css';
 
-const $ = require('jquery');
-
 dt(window, $);
 
-function tableVis(slice, payload) {
-  const container = $(slice.selector);
-  const fC = d3.format('0,000');
+const propTypes = {
+  data: PropTypes.shape({
+    records: PropTypes.arrayOf(PropTypes.object),
+    columns: PropTypes.arrayOf(PropTypes.string),
+  }),
+  height: PropTypes.number,
+  alignPn: PropTypes.bool,
+  colorPn: PropTypes.bool,
+  columnFormats: PropTypes.object,
+  includeSearch: PropTypes.bool,
+  metrics: PropTypes.arrayOf(PropTypes.string),
+  orderDesc: PropTypes.bool,
+  pageLength: PropTypes.oneOfType([
+    PropTypes.number,
+    PropTypes.string,
+  ]),
+  percentMetrics: PropTypes.array,
+  tableFilter: PropTypes.bool,
+  tableTimestampFormat: PropTypes.string,
+  timeseriesLimitMetric: PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.object,
+  ]),
+  verboseMap: PropTypes.object,
+};
+
+const fC = d3.format('0,000');
+
+function TableVis(element, props) {
+  PropTypes.checkPropTypes(propTypes, props, 'prop', 'TableVis');
+
+  const {
+    data,
+    height,
+    alignPn,
+    colorPn,
+    includeSearch,
+    metrics: rawMetrics,
+    orderDesc,
+    pageLength,
+    percentMetrics,
+    tableFilter,
+    tableTimestampFormat,
+    timeseriesLimitMetric,
+    verboseMap,
+  } = props;
+
+  const { columns, records } = data;
 
-  const data = payload.data;
-  const fd = slice.formData;
+  const $container = $(element);
 
-  let metrics = (fd.metrics || []).map(m => m.label || m);
-  // Add percent metrics
-  metrics = metrics.concat((fd.percent_metrics || []).map(m => '%' + m));
-  // Removing metrics (aggregates) that are strings
-  metrics = metrics.filter(m => !isNaN(data.records[0][m]));
+  const metrics = (rawMetrics || []).map(m => m.label || m)
+    // Add percent metrics
+    .concat((percentMetrics || []).map(m => '%' + m))
+    // Removing metrics (aggregates) that are strings
+    .filter(m => !Number.isNaN(records[0][m]));
 
   function col(c) {
     const arr = [];
-    for (let i = 0; i < data.records.length; i += 1) {
-      arr.push(data.records[i][c]);
+    for (let i = 0; i < records.length; i += 1) {
+      arr.push(records[i][c]);
     }
     return arr;
   }
   const maxes = {};
   const mins = {};
   for (let i = 0; i < metrics.length; i += 1) {
-    if (fd.align_pn) {
+    if (alignPn) {
       maxes[metrics[i]] = d3.max(col(metrics[i]).map(Math.abs));
     } else {
       maxes[metrics[i]] = d3.max(col(metrics[i]));
@@ -41,9 +84,9 @@ function tableVis(slice, payload) {
     }
   }
 
-  const tsFormatter = d3TimeFormatPreset(fd.table_timestamp_format);
+  const tsFormatter = d3TimeFormatPreset(tableTimestampFormat);
 
-  const div = d3.select(slice.selector);
+  const div = d3.select(element);
   div.html('');
   const table = div.append('table')
     .classed(
@@ -51,8 +94,7 @@ function tableVis(slice, payload) {
       'table-condensed table-hover dataTable no-footer', true)
     .attr('width', '100%');
 
-  const verboseMap = slice.datasource.verbose_map;
-  const cols = data.columns.map((c) => {
+  const cols = columns.map((c) => {
     if (verboseMap[c]) {
       return verboseMap[c];
     }
@@ -76,11 +118,11 @@ function tableVis(slice, payload) {
   const filters = slice.getFilters();
   table.append('tbody')
     .selectAll('tr')
-    .data(data.records)
+    .data(records)
     .enter()
     .append('tr')
     .selectAll('td')
-    .data(row => data.columns.map((c) => {
+    .data(row => columns.map((c) => {
       const val = row[c];
       let html;
       const isMetric = metrics.indexOf(c) >= 0;
@@ -91,7 +133,8 @@ function tableVis(slice, payload) {
         html = `<span class="like-pre">${dompurify.sanitize(val)}</span>`;
       }
       if (isMetric) {
-        html = slice.d3format(c, val);
+        const format = (columnFormats && columnFormats[c]) || '0.3s';
+        html = format(val);
       }
       if (c[0] === '%') {
         html = d3.format('.3p')(val);
@@ -107,8 +150,8 @@ function tableVis(slice, payload) {
     .append('td')
     .style('background-image', function (d) {
       if (d.isMetric) {
-        const r = (fd.color_pn && d.val < 0) ? 150 : 0;
-        if (fd.align_pn) {
+        const r = (colorPn && d.val < 0) ? 150 : 0;
+        if (alignPn) {
           const perc = Math.abs(Math.round((d.val / maxes[d.col]) * 100));
           // The 0.01 to 0.001 is a workaround for what appears to be a
           // CSS rendering bug on flat, transparent colors
@@ -149,7 +192,7 @@ function tableVis(slice, payload) {
       filters[d.col].indexOf(d.val) >= 0,
     )
     .on('click', function (d) {
-      if (!d.isMetric && fd.table_filter) {
+      if (!d.isMetric && tableFilter) {
         const td = d3.select(this);
         if (td.classed('filtered')) {
           slice.removeFilter(d.col, [d.val]);
@@ -164,43 +207,81 @@ function tableVis(slice, payload) {
       return (!d.isMetric) ? 'pointer' : '';
     })
     .html(d => d.html ? d.html : d.val);
-  const height = slice.height();
-  let paging = false;
-  let pageLength;
-  if (fd.page_length && fd.page_length > 0) {
-    paging = true;
-    pageLength = parseInt(fd.page_length, 10);
-  }
-  const datatable = container.find('.dataTable').DataTable({
+
+  const paging = pageLength && pageLength > 0;
+
+  const datatable = $container.find('.dataTable').DataTable({
     paging,
-    pageLength,
+    pageLength: paging ? parseInt(pageLength, 10) : undefined,
     aaSorting: [],
-    searching: fd.include_search,
+    searching: includeSearch,
     bInfo: false,
     scrollY: height + 'px',
     scrollCollapse: true,
     scrollX: true,
   });
-  fixDataTableBodyHeight(
-      container.find('.dataTables_wrapper'), height);
+
+  fixDataTableBodyHeight($container.find('.dataTables_wrapper'), height);
   // Sorting table by main column
   let sortBy;
-  if (fd.timeseries_limit_metric) {
+  if (timeseriesLimitMetric) {
     // Sort by as specified
-    sortBy = fd.timeseries_limit_metric.label || fd.timeseries_limit_metric;
+    sortBy = timeseriesLimitMetric.label || timeseriesLimitMetric;
   } else if (metrics.length > 0) {
     // If not specified, use the first metric from the list
     sortBy = metrics[0];
   }
   if (sortBy) {
-    datatable.column(data.columns.indexOf(sortBy)).order(fd.order_desc ? 'desc' : 'asc');
+    datatable.column(columns.indexOf(sortBy)).order(orderDesc ? 'desc' : 'asc');
   }
   if (sortBy && metrics.indexOf(sortBy) < 0) {
     // Hiding the sortBy column if not in the metrics list
-    datatable.column(data.columns.indexOf(sortBy)).visible(false);
+    datatable.column(columns.indexOf(sortBy)).visible(false);
   }
   datatable.draw();
-  container.parents('.widget').find('.tooltip').remove();
+  $container.parents('.widget').find('.tooltip').remove();
+}
+
+TableVis.propTypes = propTypes;
+
+function adaptor(slice, payload) {
+  const { selector, formData, datasource } = slice;
+  const {
+    align_pn: alignPn,
+    color_pn: colorPn,
+    include_search: includeSearch,
+    metrics,
+    order_desc: orderDesc,
+    page_length: pageLength,
+    percent_metrics: percentMetrics,
+    table_filter: tableFilter,
+    table_timestamp_format: tableTimestampFormat,
+    timeseries_limit_metric: timeseriesLimitMetric,
+  } = formData;
+  const {
+    verbose_map: verboseMap,
+    column_formats: columnFormats,
+  } = datasource;
+  const element = document.querySelector(selector);
+
+  console.log('slice.getFilters', slice.getFilters());
+
+  return TableVis(element, {
+    data: payload.data,
+    height: slice.height(),
+    alignPn,
+    colorPn,
+    columnFormats,
+    includeSearch,
+    metrics,
+    orderDesc,
+    pageLength,
+    percentMetrics,
+    tableFilter,
+    tableTimestampFormat,
+    timeseriesLimitMetric,
+    verboseMap,
+  });
 }
 
-module.exports = tableVis;
+export default adaptor;

From 114ac67c8a73985c1d2b15f9177538b988ed40b8 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:39:40 -0700
Subject: [PATCH 03/15] take filter

---
 superset/assets/src/visualizations/table.js | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index d35bc16418161..fdc1fabc72dbb 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -18,6 +18,7 @@ const propTypes = {
   alignPn: PropTypes.bool,
   colorPn: PropTypes.bool,
   columnFormats: PropTypes.object,
+  filters: PropTypes.object,
   includeSearch: PropTypes.bool,
   metrics: PropTypes.arrayOf(PropTypes.string),
   orderDesc: PropTypes.bool,
@@ -45,6 +46,8 @@ function TableVis(element, props) {
     height,
     alignPn,
     colorPn,
+    columnFormats,
+    filters,
     includeSearch,
     metrics: rawMetrics,
     orderDesc,
@@ -115,7 +118,6 @@ function TableVis(element, props) {
       return d;
     });
 
-  const filters = slice.getFilters();
   table.append('tbody')
     .selectAll('tr')
     .data(records)
@@ -134,9 +136,8 @@ function TableVis(element, props) {
       }
       if (isMetric) {
         const format = (columnFormats && columnFormats[c]) || '0.3s';
-        html = format(val);
-      }
-      if (c[0] === '%') {
+        html = d3.format(format)(val);
+      } else if (c[0] === '%') {
         html = d3.format('.3p')(val);
       }
       return {
@@ -264,14 +265,13 @@ function adaptor(slice, payload) {
   } = datasource;
   const element = document.querySelector(selector);
 
-  console.log('slice.getFilters', slice.getFilters());
-
   return TableVis(element, {
     data: payload.data,
     height: slice.height(),
     alignPn,
     colorPn,
     columnFormats,
+    filters: slice.getFilters(),
     includeSearch,
     metrics,
     orderDesc,

From f2ef891f2c1d19b1cf58a55343bfa2286685823a Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:44:14 -0700
Subject: [PATCH 04/15] remove dependencies

---
 superset/assets/src/chart/Chart.jsx         | 1 -
 superset/assets/src/visualizations/table.js | 8 ++++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index 93470335ddf64..091cbaf71a291 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -4,7 +4,6 @@ import PropTypes from 'prop-types';
 import Mustache from 'mustache';
 import { Tooltip } from 'react-bootstrap';
 
-import { d3format } from '../modules/utils';
 import ChartBody from './ChartBody';
 import Loading from '../components/Loading';
 import { Logger, LOG_ACTIONS_RENDER_CHART } from '../logger';
diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index fdc1fabc72dbb..684e5c7208529 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -114,9 +114,7 @@ function TableVis(element, props) {
     .data(cols)
     .enter()
     .append('th')
-    .text(function (d) {
-      return d;
-    });
+    .text(d => d);
 
   table.append('tbody')
     .selectAll('tr')
@@ -178,7 +176,7 @@ function TableVis(element, props) {
     })
     .classed('text-right', d => d.isMetric)
     .attr('title', (d) => {
-      if (!isNaN(d.val)) {
+      if (!Number.isNaN(d.val)) {
         return fC(d.val);
       }
       return null;
@@ -265,6 +263,8 @@ function adaptor(slice, payload) {
   } = datasource;
   const element = document.querySelector(selector);
 
+  console.log('tableFilter', tableFilter);
+
   return TableVis(element, {
     data: payload.data,
     height: slice.height(),

From 90b1ad8250e7e1ed060b3620c1ae582be9483fc0 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:52:36 -0700
Subject: [PATCH 05/15] remove removeFilter

---
 superset/assets/src/visualizations/table.js | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 684e5c7208529..37f4fb6cf2a38 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -21,6 +21,8 @@ const propTypes = {
   filters: PropTypes.object,
   includeSearch: PropTypes.bool,
   metrics: PropTypes.arrayOf(PropTypes.string),
+  onAddFilter: PropTypes.func,
+  onRemoveFilter: PropTypes.func,
   orderDesc: PropTypes.bool,
   pageLength: PropTypes.oneOfType([
     PropTypes.number,
@@ -37,6 +39,7 @@ const propTypes = {
 };
 
 const fC = d3.format('0,000');
+function NOOP() {}
 
 function TableVis(element, props) {
   PropTypes.checkPropTypes(propTypes, props, 'prop', 'TableVis');
@@ -50,6 +53,8 @@ function TableVis(element, props) {
     filters,
     includeSearch,
     metrics: rawMetrics,
+    onAddFilter = NOOP,
+    onRemoveFilter = NOOP,
     orderDesc,
     pageLength,
     percentMetrics,
@@ -194,11 +199,11 @@ function TableVis(element, props) {
       if (!d.isMetric && tableFilter) {
         const td = d3.select(this);
         if (td.classed('filtered')) {
-          slice.removeFilter(d.col, [d.val]);
+          onRemoveFilter(d.col, [d.val]);
           d3.select(this).classed('filtered', false);
         } else {
           d3.select(this).classed('filtered', true);
-          slice.addFilter(d.col, [d.val]);
+          onAddFilter(d.col, [d.val]);
         }
       }
     })
@@ -263,8 +268,6 @@ function adaptor(slice, payload) {
   } = datasource;
   const element = document.querySelector(selector);
 
-  console.log('tableFilter', tableFilter);
-
   return TableVis(element, {
     data: payload.data,
     height: slice.height(),
@@ -274,9 +277,13 @@ function adaptor(slice, payload) {
     filters: slice.getFilters(),
     includeSearch,
     metrics,
+    onAddFilter(...args) { slice.addFilter(...args); },
     orderDesc,
     pageLength,
     percentMetrics,
+    // Aug 22, 2018
+    // Perhaps this field can be removed as there is
+    // no code left in repo to set tableFilter to true.
     tableFilter,
     tableTimestampFormat,
     timeseriesLimitMetric,

From 9421da8907b7b5d5cb618dc3e53093684ff3ad01 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 22 Aug 2018 17:53:59 -0700
Subject: [PATCH 06/15] add comment

---
 superset/assets/src/visualizations/table.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 37f4fb6cf2a38..02be8bf3c2c80 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -282,8 +282,9 @@ function adaptor(slice, payload) {
     pageLength,
     percentMetrics,
     // Aug 22, 2018
-    // Perhaps this field can be removed as there is
+    // Perhaps this `tableFilter` field can be removed as there is
     // no code left in repo to set tableFilter to true.
+    // which make `onAddFilter` will never be called as well.
     tableFilter,
     tableTimestampFormat,
     timeseriesLimitMetric,

From dc0b635bff21b0b1f48c5ec5c054a01e661b27eb Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Wed, 29 Aug 2018 16:19:50 -0700
Subject: [PATCH 07/15] remove columnFormats and verboseMap from props. clarify
 a few more props

---
 superset/assets/src/visualizations/table.js | 105 +++++++++++---------
 1 file changed, 59 insertions(+), 46 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index e23c10e4d4ee6..37c9736445cad 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -10,17 +10,22 @@ import './table.css';
 dt(window, $);
 
 const propTypes = {
-  data: PropTypes.shape({
-    records: PropTypes.arrayOf(PropTypes.object),
-    columns: PropTypes.arrayOf(PropTypes.string),
-  }),
+  // Each object is { field1: value1, field2: value2 }
+  data: PropTypes.arrayOf(PropTypes.object),
   height: PropTypes.number,
   alignPn: PropTypes.bool,
   colorPn: PropTypes.bool,
-  columnFormats: PropTypes.object,
+  columns: PropTypes.arrayOf(PropTypes.shape({
+    key: PropTypes.string,
+    label: PropTypes.string,
+    format: PropTypes.string,
+  })),
   filters: PropTypes.object,
   includeSearch: PropTypes.bool,
-  metrics: PropTypes.arrayOf(PropTypes.string),
+  metrics: PropTypes.arrayOf(PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.object,
+  ])),
   onAddFilter: PropTypes.func,
   onRemoveFilter: PropTypes.func,
   orderDesc: PropTypes.bool,
@@ -28,17 +33,19 @@ const propTypes = {
     PropTypes.number,
     PropTypes.string,
   ]),
-  percentMetrics: PropTypes.array,
+  percentMetrics: PropTypes.arrayOf(PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.object,
+  ])),
   tableFilter: PropTypes.bool,
   tableTimestampFormat: PropTypes.string,
   timeseriesLimitMetric: PropTypes.oneOfType([
     PropTypes.string,
     PropTypes.object,
   ]),
-  verboseMap: PropTypes.object,
 };
 
-const fC = d3.format('0,000');
+const formatValue = d3.format('0,000');
 function NOOP() {}
 
 function TableVis(element, props) {
@@ -49,7 +56,7 @@ function TableVis(element, props) {
     height,
     alignPn,
     colorPn,
-    columnFormats,
+    columns,
     filters,
     includeSearch,
     metrics: rawMetrics,
@@ -64,20 +71,18 @@ function TableVis(element, props) {
     verboseMap,
   } = props;
 
-  const { columns, records } = data;
-
   const $container = $(element);
 
   const metrics = (rawMetrics || []).map(m => m.label || m)
     // Add percent metrics
     .concat((percentMetrics || []).map(m => '%' + m))
     // Removing metrics (aggregates) that are strings
-    .filter(m => !Number.isNaN(records[0][m]));
+    .filter(m => !Number.isNaN(data[0][m]));
 
   function col(c) {
     const arr = [];
-    for (let i = 0; i < records.length; i += 1) {
-      arr.push(records[i][c]);
+    for (let i = 0; i < data.length; i += 1) {
+      arr.push(data[i][c]);
     }
     return arr;
   }
@@ -102,49 +107,36 @@ function TableVis(element, props) {
       'table-condensed table-hover dataTable no-footer', true)
     .attr('width', '100%');
 
-  const cols = columns.map((c) => {
-    if (verboseMap[c]) {
-      return verboseMap[c];
-    }
-    // Handle verbose names for percents
-    if (c[0] === '%') {
-      const cName = c.substring(1);
-      return '% ' + (verboseMap[cName] || cName);
-    }
-    return c;
-  });
-
   table.append('thead').append('tr')
     .selectAll('th')
-    .data(cols)
+    .data(columns.map(c => c.label))
     .enter()
     .append('th')
     .text(d => d);
 
   table.append('tbody')
     .selectAll('tr')
-    .data(records)
+    .data(data)
     .enter()
     .append('tr')
     .selectAll('td')
-    .data(row => columns.map((c) => {
-      const val = row[c];
+    .data(row => columns.map(({ key, format }) => {
+      const val = row[key];
       let html;
-      const isMetric = metrics.indexOf(c) >= 0;
-      if (c === '__timestamp') {
+      const isMetric = metrics.indexOf(key) >= 0;
+      if (key === '__timestamp') {
         html = tsFormatter(val);
       }
       if (typeof (val) === 'string') {
         html = `<span class="like-pre">${dompurify.sanitize(val)}</span>`;
       }
       if (isMetric) {
-        const format = (columnFormats && columnFormats[c]) || '0.3s';
-        html = d3.format(format)(val);
-      } else if (c[0] === '%') {
+        html = d3.format(format || '0.3s')(val);
+      } else if (key[0] === '%') {
         html = d3.format('.3p')(val);
       }
       return {
-        col: c,
+        col: key,
         val,
         html,
         isMetric,
@@ -182,7 +174,7 @@ function TableVis(element, props) {
     .classed('text-right', d => d.isMetric)
     .attr('title', (d) => {
       if (!Number.isNaN(d.val)) {
-        return fC(d.val);
+        return formatValue(d.val);
       }
       return null;
     })
@@ -239,11 +231,13 @@ function TableVis(element, props) {
     sortBy = metrics[0];
   }
   if (sortBy) {
-    datatable.column(columns.indexOf(sortBy)).order(orderDesc ? 'desc' : 'asc');
-  }
-  if (sortBy && metrics.indexOf(sortBy) < 0) {
-    // Hiding the sortBy column if not in the metrics list
-    datatable.column(columns.indexOf(sortBy)).visible(false);
+    const keys = columns.map(c => c.key);
+    const index = keys.indexOf(sortBy);
+    datatable.column(index).order(orderDesc ? 'desc' : 'asc');
+    if (metrics.indexOf(sortBy) < 0) {
+      // Hiding the sortBy column if not in the metrics list
+      datatable.column(index).visible(false);
+    }
   }
   datatable.draw();
   $container.parents('.widget').find('.tooltip').remove();
@@ -269,14 +263,34 @@ function adaptor(slice, payload) {
     verbose_map: verboseMap,
     column_formats: columnFormats,
   } = datasource;
+  const { records, columns } = payload.data;
+
+  const processedColumns = columns.map(key => {
+    let label = verboseMap[key];
+    // Handle verbose names for percents
+    if (!label) {
+      if (key[0] === '%') {
+        const cleanedKey = key.substring(1);
+        label = '% ' + (verboseMap[cleanedKey] || cleanedKey);
+      } else {
+        label = key;
+      }
+    }
+    return {
+      key,
+      label,
+      format: columnFormats[key],
+    };
+  });
+
   const element = document.querySelector(selector);
 
   return TableVis(element, {
-    data: payload.data,
+    data: records,
     height: slice.height(),
     alignPn,
     colorPn,
-    columnFormats,
+    columns: processedColumns,
     filters: slice.getFilters(),
     includeSearch,
     metrics,
@@ -291,7 +305,6 @@ function adaptor(slice, payload) {
     tableFilter,
     tableTimestampFormat,
     timeseriesLimitMetric,
-    verboseMap,
   });
 }
 

From cd1d82d839409cdd46876aed20446b334082edfe Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 30 Aug 2018 10:44:53 -0700
Subject: [PATCH 08/15] fix linting issue

---
 superset/assets/src/visualizations/table.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 37c9736445cad..0e495855927bc 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -68,7 +68,6 @@ function TableVis(element, props) {
     tableFilter,
     tableTimestampFormat,
     timeseriesLimitMetric,
-    verboseMap,
   } = props;
 
   const $container = $(element);
@@ -265,7 +264,7 @@ function adaptor(slice, payload) {
   } = datasource;
   const { records, columns } = payload.data;
 
-  const processedColumns = columns.map(key => {
+  const processedColumns = columns.map((key) => {
     let label = verboseMap[key];
     // Handle verbose names for percents
     if (!label) {

From cfba6ffb63446731a6bc14b43e28a9200520ccd6 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 30 Aug 2018 12:20:06 -0700
Subject: [PATCH 09/15] minor syntax

---
 superset/assets/src/visualizations/table.js | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 0e495855927bc..12e329ff8c910 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -171,15 +171,8 @@ function TableVis(element, props) {
       return null;
     })
     .classed('text-right', d => d.isMetric)
-    .attr('title', (d) => {
-      if (!Number.isNaN(d.val)) {
-        return formatValue(d.val);
-      }
-      return null;
-    })
-    .attr('data-sort', function (d) {
-      return (d.isMetric) ? d.val : null;
-    })
+    .attr('title', d => (!Number.isNaN(d.val) ? formatValue(d.val) : null))
+    .attr('data-sort', d => (d.isMetric) ? d.val : null)
     // Check if the dashboard currently has a filter for each row
     .classed('filtered', d =>
       filters &&
@@ -198,9 +191,7 @@ function TableVis(element, props) {
         }
       }
     })
-    .style('cursor', function (d) {
-      return (!d.isMetric) ? 'pointer' : '';
-    })
+    .style('cursor', d => (!d.isMetric) ? 'pointer' : '')
     .html(d => d.html ? d.html : d.val);
 
   const paging = pageLength && pageLength > 0;
@@ -211,7 +202,7 @@ function TableVis(element, props) {
     aaSorting: [],
     searching: includeSearch,
     bInfo: false,
-    scrollY: height + 'px',
+    scrollY: `${height}px`,
     scrollCollapse: true,
     scrollX: true,
   });
@@ -278,7 +269,7 @@ function adaptor(slice, payload) {
     return {
       key,
       label,
-      format: columnFormats[key],
+      format: columnFormats && columnFormats[key],
     };
   });
 

From 029875630895c9fad3bc3c074d02bf6e695eae75 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 30 Aug 2018 12:23:47 -0700
Subject: [PATCH 10/15] syntax fix

---
 superset/assets/src/visualizations/table.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 12e329ff8c910..277323149d120 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -198,7 +198,7 @@ function TableVis(element, props) {
 
   const datatable = $container.find('.dataTable').DataTable({
     paging,
-    pageLength: paging ? parseInt(pageLength, 10) : undefined,
+    pageLength,
     aaSorting: [],
     searching: includeSearch,
     bInfo: false,

From f23c597c8b8f6bbb2b913dc72f93758f4c418385 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 30 Aug 2018 14:47:25 -0700
Subject: [PATCH 11/15] Move check to adaptor

---
 superset/assets/src/visualizations/table.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 277323149d120..a851af755c60f 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -1,6 +1,6 @@
 import d3 from 'd3';
-import PropTypes from 'prop-types';
 import $ from 'jquery';
+import PropTypes from 'prop-types';
 import dt from 'datatables.net-bs';
 import 'datatables.net-bs/css/dataTables.bootstrap.css';
 import dompurify from 'dompurify';
@@ -286,7 +286,7 @@ function adaptor(slice, payload) {
     metrics,
     onAddFilter(...args) { slice.addFilter(...args); },
     orderDesc,
-    pageLength,
+    pageLength: pageLength && parseInt(pageLength, 10),
     percentMetrics,
     // Aug 22, 2018
     // Perhaps this `tableFilter` field can be removed as there is

From 1d034cf977410b69b62cd3d06ec6ae6566171c60 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Thu, 30 Aug 2018 15:04:16 -0700
Subject: [PATCH 12/15] update unit test

---
 .../spec/javascripts/visualizations/table_spec.jsx | 14 +++++++-------
 superset/assets/src/visualizations/table.js        |  9 +++++----
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/superset/assets/spec/javascripts/visualizations/table_spec.jsx b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
index e34472b84b898..0e0e4ec92ea84 100644
--- a/superset/assets/spec/javascripts/visualizations/table_spec.jsx
+++ b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
@@ -1,12 +1,13 @@
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 import $ from 'jquery';
-
 import '../../helpers/browser';
-import { d3format } from '../../../src/modules/utils';
-
 import tableVis from '../../../src/visualizations/table';
 
+// Fix `Option is not defined`
+// https://stackoverflow.com/questions/39501589/jsdom-option-is-not-defined-when-running-my-mocha-test
+global.Option = window.Option;
+
 describe('table viz', () => {
   const div = '<div id="slice-container"><div class="dataTables_wrapper"></div></div>';
   const baseSlice = {
@@ -18,10 +19,9 @@ describe('table viz', () => {
     datasource: {
       verbose_map: {},
     },
-    getFilters: () => {},
-    d3format,
-    removeFilter: null,
-    addFilter: null,
+    getFilters: () => ({}),
+    removeFilter() {},
+    addFilter() {},
     height: () => 0,
   };
   const basePayload = {
diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index a851af755c60f..3dd97e61b98fd 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -54,11 +54,11 @@ function TableVis(element, props) {
   const {
     data,
     height,
-    alignPn,
-    colorPn,
+    alignPn = false,
+    colorPn = false,
     columns,
-    filters,
-    includeSearch,
+    filters = {},
+    includeSearch = false,
     metrics: rawMetrics,
     onAddFilter = NOOP,
     onRemoveFilter = NOOP,
@@ -253,6 +253,7 @@ function adaptor(slice, payload) {
     verbose_map: verboseMap,
     column_formats: columnFormats,
   } = datasource;
+
   const { records, columns } = payload.data;
 
   const processedColumns = columns.map((key) => {

From 66c56671426281510f9af6f8180470a0290e25a7 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Fri, 31 Aug 2018 14:47:06 -0700
Subject: [PATCH 13/15] remove code related to .widget

---
 superset/assets/src/visualizations/table.css | 28 --------------------
 superset/assets/src/visualizations/table.js  |  1 -
 2 files changed, 29 deletions(-)

diff --git a/superset/assets/src/visualizations/table.css b/superset/assets/src/visualizations/table.css
index 1b3b3aa382434..5d1e29a2ac13f 100644
--- a/superset/assets/src/visualizations/table.css
+++ b/superset/assets/src/visualizations/table.css
@@ -1,28 +1,9 @@
-.slice-grid .widget.table .slice_container {
-  overflow: auto !important;
-}
-
 .slice_container.table table.table {
   margin: 0px !important;
   background: transparent;
   background-color: white;
 }
 
-.widget.table td.filtered {
-  background-color: #005a63;
-  color: white;
-}
-
-.widget.table tr>th {
-  padding: 1px 5px !important;
-  font-size: small !important;
-}
-
-.widget.table tr>td {
-  padding: 1px 5px !important;
-  font-size: small !important;
-}
-
 table.table thead th.sorting:after, table.table thead th.sorting_asc:after, table.table thead th.sorting_desc:after {
   top: 0px;
 }
@@ -30,12 +11,3 @@ table.table thead th.sorting:after, table.table thead th.sorting_asc:after, tabl
 .like-pre {
   white-space: pre-wrap;
 }
-
-.widget.table {
-  width: auto;
-  max-width: unset;
-}
-
-.widget.table thead tr {
-  height: 25px;
-}
diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 3dd97e61b98fd..236a968c9d1d4 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -230,7 +230,6 @@ function TableVis(element, props) {
     }
   }
   datatable.draw();
-  $container.parents('.widget').find('.tooltip').remove();
 }
 
 TableVis.propTypes = propTypes;

From 25cd3b530f6b7e125c6730c27ee90e61fa2620fc Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Fri, 31 Aug 2018 14:48:42 -0700
Subject: [PATCH 14/15] rename variables for clarity

---
 superset/assets/src/visualizations/table.js | 22 ++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js
index 236a968c9d1d4..1e09b834022b3 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -13,8 +13,8 @@ const propTypes = {
   // Each object is { field1: value1, field2: value2 }
   data: PropTypes.arrayOf(PropTypes.object),
   height: PropTypes.number,
-  alignPn: PropTypes.bool,
-  colorPn: PropTypes.bool,
+  alignPositiveNegative: PropTypes.bool,
+  colorPositiveNegative: PropTypes.bool,
   columns: PropTypes.arrayOf(PropTypes.shape({
     key: PropTypes.string,
     label: PropTypes.string,
@@ -54,8 +54,8 @@ function TableVis(element, props) {
   const {
     data,
     height,
-    alignPn = false,
-    colorPn = false,
+    alignPositiveNegative = false,
+    colorPositiveNegative = false,
     columns,
     filters = {},
     includeSearch = false,
@@ -88,7 +88,7 @@ function TableVis(element, props) {
   const maxes = {};
   const mins = {};
   for (let i = 0; i < metrics.length; i += 1) {
-    if (alignPn) {
+    if (alignPositiveNegative) {
       maxes[metrics[i]] = d3.max(col(metrics[i]).map(Math.abs));
     } else {
       maxes[metrics[i]] = d3.max(col(metrics[i]));
@@ -145,8 +145,8 @@ function TableVis(element, props) {
     .append('td')
     .style('background-image', function (d) {
       if (d.isMetric) {
-        const r = (colorPn && d.val < 0) ? 150 : 0;
-        if (alignPn) {
+        const r = (colorPositiveNegative && d.val < 0) ? 150 : 0;
+        if (alignPositiveNegative) {
           const perc = Math.abs(Math.round((d.val / maxes[d.col]) * 100));
           // The 0.01 to 0.001 is a workaround for what appears to be a
           // CSS rendering bug on flat, transparent colors
@@ -237,8 +237,8 @@ TableVis.propTypes = propTypes;
 function adaptor(slice, payload) {
   const { selector, formData, datasource } = slice;
   const {
-    align_pn: alignPn,
-    color_pn: colorPn,
+    align_pn: alignPositiveNegative,
+    color_pn: colorPositiveNegative,
     include_search: includeSearch,
     metrics,
     order_desc: orderDesc,
@@ -278,8 +278,8 @@ function adaptor(slice, payload) {
   return TableVis(element, {
     data: records,
     height: slice.height(),
-    alignPn,
-    colorPn,
+    alignPositiveNegative,
+    colorPositiveNegative,
     columns: processedColumns,
     filters: slice.getFilters(),
     includeSearch,

From 0e7ce7f371181de2f4fa4d65217ce00c556f4fa1 Mon Sep 17 00:00:00 2001
From: Krist Wongsuphasawat <krist.wongz@gmail.com>
Date: Fri, 31 Aug 2018 14:50:46 -0700
Subject: [PATCH 15/15] move Option fix to browser.js

---
 superset/assets/spec/helpers/browser.js                       | 4 ++++
 .../assets/spec/javascripts/visualizations/table_spec.jsx     | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/superset/assets/spec/helpers/browser.js b/superset/assets/spec/helpers/browser.js
index 80ce31539feae..3f824b9fc3a65 100644
--- a/superset/assets/spec/helpers/browser.js
+++ b/superset/assets/spec/helpers/browser.js
@@ -30,6 +30,10 @@ global.navigator = {
   appName: 'Netscape',
 };
 
+// Fix `Option is not defined`
+// https://stackoverflow.com/questions/39501589/jsdom-option-is-not-defined-when-running-my-mocha-test
+global.Option = window.Option;
+
 // Configuration copied from https://github.com/sinonjs/sinon/issues/657
 // allowing for sinon.fakeServer to work
 
diff --git a/superset/assets/spec/javascripts/visualizations/table_spec.jsx b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
index 0e0e4ec92ea84..f87aae6634924 100644
--- a/superset/assets/spec/javascripts/visualizations/table_spec.jsx
+++ b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
@@ -4,10 +4,6 @@ import $ from 'jquery';
 import '../../helpers/browser';
 import tableVis from '../../../src/visualizations/table';
 
-// Fix `Option is not defined`
-// https://stackoverflow.com/questions/39501589/jsdom-option-is-not-defined-when-running-my-mocha-test
-global.Option = window.Option;
-
 describe('table viz', () => {
   const div = '<div id="slice-container"><div class="dataTables_wrapper"></div></div>';
   const baseSlice = {