You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/08/31 22:42:46 UTC
[GitHub] williaster closed pull request #5707: [SIP-5] Refactor table
williaster closed pull request #5707: [SIP-5] Refactor table
URL: https://github.com/apache/incubator-superset/pull/5707
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/spec/helpers/browser.js b/superset/assets/spec/helpers/browser.js
index 80ce31539f..3f824b9fc3 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 e34472b84b..f87aae6634 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 c593cee3b2..b93b2758bb 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 9af0c0e5f5..5d1e29a2ac 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 d1490ecc43..1e09b83402 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;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org