You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by cc...@apache.org on 2018/08/31 22:42:48 UTC
[incubator-superset] branch master updated: [SIP-5] Refactor table
(#5707)
This is an automated email from the ASF dual-hosted git repository.
ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 8a4b1b7 [SIP-5] Refactor table (#5707)
8a4b1b7 is described below
commit 8a4b1b7c25a8234ac1641add38645a08fb338dd1
Author: Krist Wongsuphasawat <kr...@gmail.com>
AuthorDate: Fri Aug 31 15:42:44 2018 -0700
[SIP-5] Refactor table (#5707)
* update indent
* extract formData and data.
* take filter
* remove dependencies
* remove removeFilter
* add comment
* remove columnFormats and verboseMap from props. clarify a few more props
* fix linting issue
* minor syntax
* syntax fix
* Move check to adaptor
* update unit test
* remove code related to .widget
* rename variables for clarity
* move Option fix to browser.js
---
superset/assets/spec/helpers/browser.js | 4 +
.../spec/javascripts/visualizations/table_spec.jsx | 10 +-
superset/assets/src/chart/Chart.jsx | 8 -
superset/assets/src/visualizations/table.css | 28 +--
superset/assets/src/visualizations/table.js | 260 ++++++++++++++-------
5 files changed, 184 insertions(+), 126 deletions(-)
diff --git a/superset/assets/spec/helpers/browser.js b/superset/assets/spec/helpers/browser.js
index 80ce315..3f824b9 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 e34472b..f87aae6 100644
--- a/superset/assets/spec/javascripts/visualizations/table_spec.jsx
+++ b/superset/assets/spec/javascripts/visualizations/table_spec.jsx
@@ -1,10 +1,7 @@
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';
describe('table viz', () => {
@@ -18,10 +15,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/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
index c593cee..b93b275 100644
--- a/superset/assets/src/chart/Chart.jsx
+++ b/superset/assets/src/chart/Chart.jsx
@@ -3,7 +3,6 @@ import React from 'react';
import PropTypes from 'prop-types';
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';
@@ -167,13 +166,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.css b/superset/assets/src/visualizations/table.css
index 9af0c0e..5d1e29a 100644
--- a/superset/assets/src/visualizations/table.css
+++ b/superset/assets/src/visualizations/table.css
@@ -1,39 +1,13 @@
-.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;
+ top: 0px;
}
.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 d1490ec..1e09b83 100644
--- a/superset/assets/src/visualizations/table.js
+++ b/superset/assets/src/visualizations/table.js
@@ -1,39 +1,94 @@
import d3 from 'd3';
+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';
-
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 = {
+ // Each object is { field1: value1, field2: value2 }
+ data: PropTypes.arrayOf(PropTypes.object),
+ height: PropTypes.number,
+ alignPositiveNegative: PropTypes.bool,
+ colorPositiveNegative: PropTypes.bool,
+ columns: PropTypes.arrayOf(PropTypes.shape({
+ key: PropTypes.string,
+ label: PropTypes.string,
+ format: PropTypes.string,
+ })),
+ filters: PropTypes.object,
+ includeSearch: PropTypes.bool,
+ metrics: PropTypes.arrayOf(PropTypes.oneOfType([
+ PropTypes.string,
+ PropTypes.object,
+ ])),
+ onAddFilter: PropTypes.func,
+ onRemoveFilter: PropTypes.func,
+ orderDesc: PropTypes.bool,
+ pageLength: PropTypes.oneOfType([
+ PropTypes.number,
+ PropTypes.string,
+ ]),
+ percentMetrics: PropTypes.arrayOf(PropTypes.oneOfType([
+ PropTypes.string,
+ PropTypes.object,
+ ])),
+ tableFilter: PropTypes.bool,
+ tableTimestampFormat: PropTypes.string,
+ timeseriesLimitMetric: PropTypes.oneOfType([
+ PropTypes.string,
+ PropTypes.object,
+ ]),
+};
+
+const formatValue = d3.format('0,000');
+function NOOP() {}
- const data = payload.data;
- const fd = slice.formData;
+function TableVis(element, props) {
+ PropTypes.checkPropTypes(propTypes, props, 'prop', 'TableVis');
+
+ const {
+ data,
+ height,
+ alignPositiveNegative = false,
+ colorPositiveNegative = false,
+ columns,
+ filters = {},
+ includeSearch = false,
+ metrics: rawMetrics,
+ onAddFilter = NOOP,
+ onRemoveFilter = NOOP,
+ orderDesc,
+ pageLength,
+ percentMetrics,
+ tableFilter,
+ tableTimestampFormat,
+ timeseriesLimitMetric,
+ } = props;
- 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 $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(data[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 < data.length; i += 1) {
+ arr.push(data[i][c]);
}
return arr;
}
const maxes = {};
const mins = {};
for (let i = 0; i < metrics.length; i += 1) {
- if (fd.align_pn) {
+ if (alignPositiveNegative) {
maxes[metrics[i]] = d3.max(col(metrics[i]).map(Math.abs));
} else {
maxes[metrics[i]] = d3.max(col(metrics[i]));
@@ -41,9 +96,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,53 +106,36 @@ 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) => {
- 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(function (d) {
- return d;
- });
+ .text(d => d);
- const filters = slice.getFilters();
table.append('tbody')
.selectAll('tr')
- .data(data.records)
+ .data(data)
.enter()
.append('tr')
.selectAll('td')
- .data(row => data.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) {
- html = slice.d3format(c, val);
- }
- 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,
@@ -107,8 +145,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 = (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
@@ -133,15 +171,8 @@ function tableVis(slice, payload) {
return null;
})
.classed('text-right', d => d.isMetric)
- .attr('title', (d) => {
- if (!isNaN(d.val)) {
- return fC(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 &&
@@ -149,45 +180,39 @@ 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]);
+ 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]);
}
}
})
- .style('cursor', function (d) {
- return (!d.isMetric) ? 'pointer' : '';
- })
+ .style('cursor', d => (!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,
aaSorting: [],
- searching: fd.include_search,
+ searching: includeSearch,
bInfo: false,
- scrollY: height + 'px',
+ 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;
- const limitMetric = Array.isArray(fd.timeseries_limit_metric)
- ? fd.timeseries_limit_metric[0]
- : fd.timeseries_limit_metric;
+ const limitMetric = Array.isArray(timeseriesLimitMetric)
+ ? timeseriesLimitMetric[0]
+ : timeseriesLimitMetric;
if (limitMetric) {
// Sort by as specified
sortBy = limitMetric.label || limitMetric;
@@ -196,14 +221,81 @@ function tableVis(slice, payload) {
sortBy = metrics[0];
}
if (sortBy) {
- datatable.column(data.columns.indexOf(sortBy)).order(fd.order_desc ? '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);
+ 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();
}
-module.exports = tableVis;
+TableVis.propTypes = propTypes;
+
+function adaptor(slice, payload) {
+ const { selector, formData, datasource } = slice;
+ const {
+ align_pn: alignPositiveNegative,
+ color_pn: colorPositiveNegative,
+ 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 { 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 && columnFormats[key],
+ };
+ });
+
+ const element = document.querySelector(selector);
+
+ return TableVis(element, {
+ data: records,
+ height: slice.height(),
+ alignPositiveNegative,
+ colorPositiveNegative,
+ columns: processedColumns,
+ filters: slice.getFilters(),
+ includeSearch,
+ metrics,
+ onAddFilter(...args) { slice.addFilter(...args); },
+ orderDesc,
+ pageLength: pageLength && parseInt(pageLength, 10),
+ percentMetrics,
+ // Aug 22, 2018
+ // 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,
+ });
+}
+
+export default adaptor;