Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

perf: faster legacy table chart #385

Merged
merged 12 commits into from
Mar 5, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
*/
import { t } from '@superset-ui/translation';
import React, { useEffect, createRef } from 'react';
import { getNumberFormatter, NumberFormats } from '@superset-ui/number-format';
import { formatNumber, getNumberFormatter, NumberFormats } from '@superset-ui/number-format';
import { getTimeFormatter } from '@superset-ui/time-format';
import { DataTableProps } from './transformProps';
import { filterXSS } from 'xss';

// initialize datatables.net
import $ from 'jquery';
Expand All @@ -35,19 +36,9 @@ if (!dt.$) {
dt(window, $);
}

const RE_HTML_TAG = /<.*>/; // a dead simple regexp to find html tag
ktmud marked this conversation as resolved.
Show resolved Hide resolved
const formatPercent = getNumberFormatter(NumberFormats.PERCENT_3_POINT);

const adjustTableHeight = ($root: any, height: number) => {
const headHeight = $root.find('.dataTables_scrollHead').height();
const filterHeight = $root.find('.dataTables_filter').height() || 0;
const pageLengthHeight = $root.find('.dataTables_length').height() || 0;
let paginationHeight = $root.find('.dataTables_paginate').height() || 0;
const controlsHeight = pageLengthHeight > filterHeight ? pageLengthHeight : filterHeight;
$root
.find('.dataTables_scrollBody')
.css('max-height', height - headHeight - controlsHeight - paginationHeight);
};

// const NOOP = () => { };

export default function ReactDataTable(props: DataTableProps) {
Expand Down Expand Up @@ -80,17 +71,14 @@ export default function ReactDataTable(props: DataTableProps) {

const maxes: { [key: string]: number } = {};
const mins: { [key: string]: number } = {};
const isMetric = (key: string) => metrics.includes(key);
// a fater way of determine whether a key is a metric
ktmud marked this conversation as resolved.
Show resolved Hide resolved
// might be called by each cell
const isMetric = (key: string) => maxes.hasOwnProperty(key);

// Specify options for each data table columns
// Ref: https://datatables.net/reference/option/columns
const columnOptions = columns.map(({ key, label }) => {
// collect min/max for rendering bars
columns.forEach(({ key }) => {
const vals = data.map(row => row[key]);
const keyIsMetric = isMetric(key);
let className = keyIsMetric ? 'dt-metric' : '';

// collect min/max for for rendering bars
if (keyIsMetric) {
if (metrics.includes(key)) {
const nums = vals as number[];
if (alignPositiveNegative) {
maxes[key] = Math.max(...nums.map(Math.abs));
Expand All @@ -99,21 +87,15 @@ export default function ReactDataTable(props: DataTableProps) {
mins[key] = Math.min(...nums);
}
}

return {
key,
className,
title: label,
};
});

const viewportHeight = Math.min(height, window.innerHeight);
const pageLengthChoices = [10, 25, 40, 50, 75, 100, 150, 200];
ktmud marked this conversation as resolved.
Show resolved Hide resolved
const hasPagination = pageLength > 0;
const options = {
aaSorting: [], // initial sorting order, reset to [] to use backend ordering
autoWidth: false,
columns: columnOptions,
paging: pageLength > 0,
paging: hasPagination,
pagingType: 'first_last_numbers',
pageLength,
lengthMenu: [
Expand All @@ -135,14 +117,29 @@ export default function ReactDataTable(props: DataTableProps) {
scrollX: true,
drawCallback,
};

const rootElem = createRef<HTMLDivElement>();

useEffect(() => {
const $container = $(rootElem.current as HTMLElement);
const dataTable = $container.find('.dataTable').DataTable(options);
adjustTableHeight($container.find('.dataTables_wrapper'), viewportHeight);
const $root = $(rootElem.current as HTMLElement);
const dataTable = $root.find('table').DataTable(options);

// adjust table height
const scrollHeadHeight = 34;
const paginationHeight = hasPagination ? 35 : 0;
const searchBarHeight = hasPagination || includeSearch ? 35 : 0;
const scrollBodyHeight = viewportHeight - scrollHeadHeight - paginationHeight - searchBarHeight;
$root.find('.dataTables_scrollBody').css('max-height', scrollBodyHeight);

return () => {
dataTable.destroy();
// there may be weird lifecycle issue, so we put destroy in try/catch
try {
dataTable.destroy();
// reset height
$root.find('.dataTables_scrollBody').css('max-height', '');
} catch (error) {
// pass
}
};
});

Expand All @@ -151,8 +148,11 @@ export default function ReactDataTable(props: DataTableProps) {
* and adjust the pagination size (which is not configurable via DataTables API).
*/
function drawCallback(this: DataTables.JQueryDataTables) {
const root = rootElem.current as HTMLElement;
// force smaller pagination, because datatables-bs hard-corded pagination styles
$('.pagination', rootElem.current as HTMLElement).addClass('pagination-sm');
$('.pagination', root).addClass('pagination-sm');
// display tr rows on current page
$('tr', root).css('display', '');
}

/**
Expand All @@ -162,11 +162,10 @@ export default function ReactDataTable(props: DataTableProps) {
if (key === '__timestamp') {
return formatTimestamp(val);
} else if (typeof val === 'string') {
// It's fine to return raw value react will handles HTML escaping
return val;
return filterXSS(val, { stripIgnoreTag: true });
ktmud marked this conversation as resolved.
Show resolved Hide resolved
} else if (isMetric(key)) {
// default format '' will return human readable numbers (e.g. 50M, 33k)
return getNumberFormatter(format || '')(val as number);
return formatNumber(format || '', val as number);
} else if (key[0] === '%') {
return formatPercent(val);
}
Expand Down Expand Up @@ -203,29 +202,39 @@ export default function ReactDataTable(props: DataTableProps) {

return (
<div className="superset-legacy-chart-table" ref={rootElem}>
<table className="table table-striped table-condensed table-hover dataTable">
<table className="table table-striped table-condensed table-hover">
<thead>
<tr>
{columns.map(col => (
<th key={col.key}>{col.label}</th>
// by default all columns will have sorting
<th className="sorting" key={col.key} title={col.label}>
{col.label}
</th>
))}
</tr>
</thead>
<tbody>
{data.map((record, i) => (
<tr key={i}>
// hide rows after first page makes the initial render faster (less layout computation)
<tr key={i} style={{ display: pageLength > 0 && i >= pageLength ? 'none' : undefined }}>
ktmud marked this conversation as resolved.
Show resolved Hide resolved
{columns.map(({ key, format }) => {
const val = record[key];
const keyIsMetric = isMetric(key);
const text = cellText(key, format, val);
const isHtml = !keyIsMetric && RE_HTML_TAG.test(text);
return (
<td
key={key}
data-sort={val}
className={isMetric(key) ? 'text-right' : ''}
className={keyIsMetric ? 'text-right' : ''}
style={{
backgroundImage: typeof val === 'number' ? cellBar(key, val) : undefined,
}}
title={val as string}
// only set innerHTML for actual html content, this saves time
dangerouslySetInnerHTML={isHtml ? { __html: text } : undefined}
>
{cellText(key, format, val)}
{isHtml ? null : text}
</td>
);
})}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
*/
.superset-legacy-chart-table {
margin: 0px auto;
ktmud marked this conversation as resolved.
Show resolved Hide resolved
background: transparent;
background-color: white;
}
.superset-legacy-chart-table table {
width: 100%;
}
.superset-legacy-chart-table .dt-metric {
text-align: right;
}
.superset-legacy-chart-table div.dataTables_wrapper div.dataTables_paginate {
line-height: 0;
}
.superset-legacy-chart-table div.dataTables_wrapper div.dataTables_paginate ul.pagination {
margin-top: 0.3em;
margin-top: 0.5em;
}

.superset-legacy-chart-table thead th.sorting:after,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
/// <reference path="../types/external.d.ts" />
ktmud marked this conversation as resolved.
Show resolved Hide resolved
import { t } from '@superset-ui/translation';
import { ChartMetadata, ChartPlugin } from '@superset-ui/chart';
import transformProps from './transformProps';
Expand Down
10 changes: 9 additions & 1 deletion packages/superset-ui-plugins-demo/.storybook/storybook.css
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ body,
font-weight: 200;
color: #484848;
}

#root > div {
padding: 8px;
}
ktmud marked this conversation as resolved.
Show resolved Hide resolved

#root > div.superset-body {
background: #f5f5f5;
padding: 16px;
min-height: 100%;
}
#root > div.superset-body .panel {
margin-bottom: 0;
}
Original file line number Diff line number Diff line change
@@ -1,29 +1,41 @@
/* eslint-disable no-magic-numbers */
import React from 'react';
import React, { useState } from 'react';
import { SuperChart } from '@superset-ui/chart';
import { Props as SuperChartProps } from '@superset-ui/chart/lib/components/SuperChart';
import data from './data';
import birthNames from './birth_names.json';
ktmud marked this conversation as resolved.
Show resolved Hide resolved

// must have explicit import for Storyboard build watch to work
// import TableChartPlugin from '../../../../superset-ui-legacy-plugin-chart-table/src';
// new TableChartPlugin().configure({ key: 'dev_table' }).register();

import 'bootstrap/dist/css/bootstrap.min.css';

function paginated(props: SuperChartProps, pageSize = 100) {
function paginated(props: SuperChartProps, pageSize = 50) {
if (props.formData) {
props.formData.page_length = pageSize;
props.formData = {
...props.formData,
page_length: pageSize,
};
}
if (props.queryData?.form_data) {
props.queryData.form_data.page_length = pageSize;
props.queryData.form_data = {
...props.queryData.form_data,
page_length: pageSize,
};
}
return props;
return {
...props,
};
}

/**
* Duplicate query data records until reaching specified size
* Load sample data for testing
* @param props the original props passed to SuperChart
* @param pageSize number of records perpage
* @param targetSize the target total number of records
*/
function duplicated(props: SuperChartProps, pageSize = 50, targetSize = 5042) {
function loadData(props: SuperChartProps, pageSize = 50, targetSize = 3042) {
if (!props.queryData) return props;
const data = props.queryData && props.queryData.data;
if (data.records.length > 0) {
Expand All @@ -32,7 +44,8 @@ function duplicated(props: SuperChartProps, pageSize = 50, targetSize = 5042) {
data.records = records.concat(records).slice(0, targetSize);
}
}
return paginated(props);
props.height = window.innerHeight - 130;
return paginated(props, pageSize);
}

export default [
Expand Down Expand Up @@ -70,9 +83,37 @@ export default [
},
{
renderStory() {
return <SuperChart {...duplicated(birthNames)} chartType="table" />;
const [chartProps, setChartProps] = useState(loadData(birthNames));
const updatePageSize = (size: number) => {
setChartProps(paginated(chartProps, size));
};
return (
<div className="superset-body">
<div className="panel">
<div className="panel-heading form-inline">
Initial page size:{' '}
<div className="btn-group">
{[10, 25, 40, 50, 100, -1].map(pageSize => {
return (
<button
key={pageSize}
className="btn btn-default"
onClick={() => updatePageSize(pageSize)}
>
{pageSize > 0 ? pageSize : 'All'}
</button>
);
})}
</div>
</div>
<div className="panel-body">
<SuperChart {...chartProps} chartType="table" />
</div>
</div>
</div>
);
},
storyName: 'Big Chart',
storyName: 'Big Table',
storyPath: 'legacy-|plugin-chart-table|TableChartPlugin',
},
];
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"disableErrorBoundary": true,
"id": "chart-id-60",
"className": "table",
"className": "superset-chart-table",
"chartType": "table",
"width": 932,
"height": 1135,
Expand Down Expand Up @@ -444,16 +444,16 @@
"%pct_boys": 0.0007479431563201197
},
{
"name": "Christopher",
"name": "<a href=\"http://www.google.com\" target=\"_blank\">Christopher (test link)</a>",
"state": "other",
"gender": "boy",
"total": 803544,
"SUM(sum_boys)": 803544,
"SUM(sum_girls)": 0,
"SUM(sum_girls)": -300000,
"%pct_boys": 0.0007479431563201197
},
{
"name": "James",
"name": "<a href=\"javascript:alert(1);\" target=\"_blank\">James (test javascript link)</a>",
"state": "other",
"gender": "boy",
"total": 749616,
Expand Down