From 47d9fd705a61679323efdd0231242d4ad125dc3f Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 22 Aug 2018 16:44:54 -0700 Subject: [PATCH 1/3] extract slice and formData --- .../assets/src/visualizations/pivot_table.js | 97 ++++++++++++++----- 1 file changed, 74 insertions(+), 23 deletions(-) diff --git a/superset/assets/src/visualizations/pivot_table.js b/superset/assets/src/visualizations/pivot_table.js index fded4616b350a..e7ad7d9a25962 100644 --- a/superset/assets/src/visualizations/pivot_table.js +++ b/superset/assets/src/visualizations/pivot_table.js @@ -1,51 +1,77 @@ import dt from 'datatables.net-bs'; import 'datatables.net-bs/css/dataTables.bootstrap.css'; import $ from 'jquery'; - +import PropTypes from 'prop-types'; import { d3format, fixDataTableBodyHeight } from '../modules/utils'; import './pivot_table.css'; dt(window, $); -module.exports = function (slice, payload) { - const container = slice.container; - const fd = slice.formData; - const height = container.height(); - let cols = payload.data.columns; - if (Array.isArray(cols[0])) { - cols = cols.map(col => col[0]); - } +const propTypes = { + data: PropTypes.shape({ + html: PropTypes.string, + columns: PropTypes.arrayOf(PropTypes.oneOfType([ + PropTypes.string, + PropTypes.arrayOf(PropTypes.string), + ])), + }), + height: PropTypes.number, + columnFormats: PropTypes.object, + groupBy: PropTypes.arrayOf(PropTypes.string), + numberFormat: PropTypes.string, + verboseMap: PropTypes.object, +}; + +function PivotTable(element, props) { + PropTypes.checkPropTypes(propTypes, props, 'prop', 'PivotTable'); + + const { + data, + height, + columnFormats, + groupBy, + numberFormat, + verboseMap, + } = props; + + const { html, columns } = data; + const theContainer = element; + const $theContainer = $(element); // payload data is a string of html with a single table element - container.html(payload.data.html); + theContainer.innerHTML = html; + + const cols = Array.isArray(columns[0]) + ? columns.map(col => col[0]) + : columns; // jQuery hack to set verbose names in headers const replaceCell = function () { const s = $(this)[0].textContent; - $(this)[0].textContent = slice.datasource.verbose_map[s] || s; + $(this)[0].textContent = verboseMap[s] || s; }; - slice.container.find('thead tr:first th').each(replaceCell); - slice.container.find('thead tr th:first-child').each(replaceCell); + $theContainer.find('thead tr:first th').each(replaceCell); + $theContainer.find('thead tr th:first-child').each(replaceCell); // jQuery hack to format number - slice.container.find('tbody tr').each(function () { + $theContainer.find('tbody tr').each(function () { $(this).find('td').each(function (i) { const metric = cols[i]; - const format = slice.datasource.column_formats[metric] || fd.number_format || '.3s'; + const format = columnFormats[metric] || numberFormat || '.3s'; const tdText = $(this)[0].textContent; - if (!isNaN(tdText) && tdText !== '') { + if (!Number.isNaN(tdText) && tdText !== '') { $(this)[0].textContent = d3format(format, tdText); } }); }); - if (fd.groupby.length === 1) { + if (groupBy.length === 1) { // When there is only 1 group by column, // we use the DataTable plugin to make the header fixed. // The plugin takes care of the scrolling so we don't need // overflow: 'auto' on the table. - container.css('overflow', 'hidden'); - const table = container.find('table').DataTable({ + theContainer.style.overflow = 'hidden'; + const table = $theContainer.find('table').DataTable({ paging: false, searching: false, bInfo: false, @@ -54,12 +80,37 @@ module.exports = function (slice, payload) { scrollX: true, }); table.column('-1').order('desc').draw(); - fixDataTableBodyHeight(container.find('.dataTables_wrapper'), height); + fixDataTableBodyHeight($theContainer.find('.dataTables_wrapper'), height); } else { // When there is more than 1 group by column we just render the table, without using // the DataTable plugin, so we need to handle the scrolling ourselves. // In this case the header is not fixed. - container.css('overflow', 'auto'); - container.css('height', `${height + 10}px`); + theContainer.style.overflow = 'auto'; + theContainer.style.height = `${height + 10}px`; } -}; +} + +function adaptor(slice, payload) { + const { selector, formData, datasource } = slice; + const { + groupby: groupBy, + number_format: numberFormat, + } = formData; + const { + column_formats: columnFormats, + verbose_map: verboseMap, + } = datasource; + const element = document.querySelector(selector); + + return PivotTable(element, { + data: payload.data, + height: slice.height(), + columnFormats, + groupBy, + numberFormat, + verboseMap, + }); +} + +export default adaptor; + From 7821ed00c71fbfe88c992fcafdf8517f96dedf95 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 22 Aug 2018 16:46:59 -0700 Subject: [PATCH 2/3] rename variables --- .../assets/src/visualizations/pivot_table.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/superset/assets/src/visualizations/pivot_table.js b/superset/assets/src/visualizations/pivot_table.js index e7ad7d9a25962..4f6def0761535 100644 --- a/superset/assets/src/visualizations/pivot_table.js +++ b/superset/assets/src/visualizations/pivot_table.js @@ -35,11 +35,11 @@ function PivotTable(element, props) { } = props; const { html, columns } = data; - const theContainer = element; - const $theContainer = $(element); + const container = element; + const $container = $(element); // payload data is a string of html with a single table element - theContainer.innerHTML = html; + container.innerHTML = html; const cols = Array.isArray(columns[0]) ? columns.map(col => col[0]) @@ -50,11 +50,11 @@ function PivotTable(element, props) { const s = $(this)[0].textContent; $(this)[0].textContent = verboseMap[s] || s; }; - $theContainer.find('thead tr:first th').each(replaceCell); - $theContainer.find('thead tr th:first-child').each(replaceCell); + $container.find('thead tr:first th').each(replaceCell); + $container.find('thead tr th:first-child').each(replaceCell); // jQuery hack to format number - $theContainer.find('tbody tr').each(function () { + $container.find('tbody tr').each(function () { $(this).find('td').each(function (i) { const metric = cols[i]; const format = columnFormats[metric] || numberFormat || '.3s'; @@ -70,8 +70,8 @@ function PivotTable(element, props) { // we use the DataTable plugin to make the header fixed. // The plugin takes care of the scrolling so we don't need // overflow: 'auto' on the table. - theContainer.style.overflow = 'hidden'; - const table = $theContainer.find('table').DataTable({ + container.style.overflow = 'hidden'; + const table = $container.find('table').DataTable({ paging: false, searching: false, bInfo: false, @@ -80,13 +80,13 @@ function PivotTable(element, props) { scrollX: true, }); table.column('-1').order('desc').draw(); - fixDataTableBodyHeight($theContainer.find('.dataTables_wrapper'), height); + fixDataTableBodyHeight($container.find('.dataTables_wrapper'), height); } else { // When there is more than 1 group by column we just render the table, without using // the DataTable plugin, so we need to handle the scrolling ourselves. // In this case the header is not fixed. - theContainer.style.overflow = 'auto'; - theContainer.style.height = `${height + 10}px`; + container.style.overflow = 'auto'; + container.style.height = `${height + 10}px`; } } From 7a86b5324bf811ea7b309cf4452ed39184168f84 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 29 Aug 2018 15:41:28 -0700 Subject: [PATCH 3/3] update proptypes and add comments --- superset/assets/src/visualizations/pivot_table.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset/assets/src/visualizations/pivot_table.js b/superset/assets/src/visualizations/pivot_table.js index 4f6def0761535..cd93867cbfdb4 100644 --- a/superset/assets/src/visualizations/pivot_table.js +++ b/superset/assets/src/visualizations/pivot_table.js @@ -9,6 +9,7 @@ dt(window, $); const propTypes = { data: PropTypes.shape({ + // TODO: replace this with raw data in SIP-6 html: PropTypes.string, columns: PropTypes.arrayOf(PropTypes.oneOfType([ PropTypes.string, @@ -16,10 +17,10 @@ const propTypes = { ])), }), height: PropTypes.number, - columnFormats: PropTypes.object, + columnFormats: PropTypes.objectOf(PropTypes.string), groupBy: PropTypes.arrayOf(PropTypes.string), numberFormat: PropTypes.string, - verboseMap: PropTypes.object, + verboseMap: PropTypes.objectOf(PropTypes.string), }; function PivotTable(element, props) {