[SIP-5] Refactor table #5707
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,58 +1,108 @@ | ||
| 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'; | ||
|
|
||
| dt(window, $); | ||
|
|
||
| const propTypes = { | ||
| data: PropTypes.shape({ | ||
| records: PropTypes.arrayOf(PropTypes.object), | ||
| columns: PropTypes.arrayOf(PropTypes.string), | ||
| }), | ||
| height: PropTypes.number, | ||
| alignPn: PropTypes.bool, | ||
|
There was a problem hiding this comment. "Pn" means positive/negative? could we update that variable name to be more descriptive to indicate that? |
||
| colorPn: PropTypes.bool, | ||
| columnFormats: PropTypes.object, | ||
| 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, | ||
| 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 NOOP() {} | ||
|
|
||
| function TableVis(element, props) { | ||
| PropTypes.checkPropTypes(propTypes, props, 'prop', 'TableVis'); | ||
|
|
||
| const { | ||
| data, | ||
| height, | ||
| alignPn, | ||
| colorPn, | ||
| columnFormats, | ||
| filters, | ||
| includeSearch, | ||
| metrics: rawMetrics, | ||
| onAddFilter = NOOP, | ||
|
There was a problem hiding this comment. Why do we need a No Op on these prop functions? There was a problem hiding this comment. Set |
||
| onRemoveFilter = NOOP, | ||
| orderDesc, | ||
| pageLength, | ||
| percentMetrics, | ||
| tableFilter, | ||
| tableTimestampFormat, | ||
| timeseriesLimitMetric, | ||
| 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])); | ||
|
|
||
| function col(c) { | ||
| const arr = []; | ||
| 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 (alignPn) { | ||
| maxes[metrics[i]] = d3.max(col(metrics[i]).map(Math.abs)); | ||
| } else { | ||
| maxes[metrics[i]] = d3.max(col(metrics[i])); | ||
| mins[metrics[i]] = d3.min(col(metrics[i])); | ||
| } | ||
| } | ||
|
|
||
| const tsFormatter = d3TimeFormatPreset(tableTimestampFormat); | ||
|
|
||
| const div = d3.select(element); | ||
| div.html(''); | ||
| const table = div.append('table') | ||
| .classed( | ||
| 'dataframe dataframe table table-striped ' + | ||
| 'table-condensed table-hover dataTable no-footer', true) | ||
| .attr('width', '100%'); | ||
|
|
||
| const cols = columns.map((c) => { | ||
| if (verboseMap[c]) { | ||
| return verboseMap[c]; | ||
| } | ||
|
|
@@ -69,18 +119,15 @@ function tableVis(slice, payload) { | |
| .data(cols) | ||
| .enter() | ||
| .append('th') | ||
| .text(d => d); | ||
|
|
||
| table.append('tbody') | ||
| .selectAll('tr') | ||
| .data(records) | ||
| .enter() | ||
| .append('tr') | ||
| .selectAll('td') | ||
| .data(row => columns.map((c) => { | ||
| const val = row[c]; | ||
| let html; | ||
| const isMetric = metrics.indexOf(c) >= 0; | ||
|
|
@@ -91,9 +138,9 @@ function tableVis(slice, payload) { | |
| 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('.3p')(val); | ||
| } | ||
| return { | ||
|
|
@@ -107,8 +154,8 @@ function tableVis(slice, payload) { | |
| .append('td') | ||
| .style('background-image', function (d) { | ||
| if (d.isMetric) { | ||
| 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 | ||
|
|
@@ -134,7 +181,7 @@ function tableVis(slice, payload) { | |
| }) | ||
| .classed('text-right', d => d.isMetric) | ||
| .attr('title', (d) => { | ||
| if (!Number.isNaN(d.val)) { | ||
| return fC(d.val); | ||
| } | ||
| return null; | ||
|
|
@@ -149,58 +196,100 @@ function tableVis(slice, payload) { | |
| filters[d.col].indexOf(d.val) >= 0, | ||
| ) | ||
| .on('click', function (d) { | ||
| if (!d.isMetric && tableFilter) { | ||
| const td = d3.select(this); | ||
| if (td.classed('filtered')) { | ||
| onRemoveFilter(d.col, [d.val]); | ||
| d3.select(this).classed('filtered', false); | ||
| } else { | ||
| d3.select(this).classed('filtered', true); | ||
| onAddFilter(d.col, [d.val]); | ||
| } | ||
| } | ||
| }) | ||
| .style('cursor', function (d) { | ||
| return (!d.isMetric) ? 'pointer' : ''; | ||
| }) | ||
| .html(d => d.html ? d.html : d.val); | ||
|
|
||
| const paging = pageLength && pageLength > 0; | ||
|
|
||
| const datatable = $container.find('.dataTable').DataTable({ | ||
| paging, | ||
| pageLength: paging ? parseInt(pageLength, 10) : undefined, | ||
| aaSorting: [], | ||
| searching: includeSearch, | ||
| bInfo: false, | ||
| scrollY: height + 'px', | ||
| scrollCollapse: true, | ||
| scrollX: true, | ||
| }); | ||
|
|
||
| fixDataTableBodyHeight($container.find('.dataTables_wrapper'), height); | ||
| // Sorting table by main column | ||
| let sortBy; | ||
| if (timeseriesLimitMetric) { | ||
| // Sort by as specified | ||
| 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(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); | ||
| } | ||
| datatable.draw(); | ||
| $container.parents('.widget').find('.tooltip').remove(); | ||
|
There was a problem hiding this comment. I would guess that this tried to remove tooltips from other dashboard charts 🤔 |
||
| } | ||
|
|
||
| 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); | ||
|
|
||
| return TableVis(element, { | ||
| data: payload.data, | ||
| height: slice.height(), | ||
| alignPn, | ||
| colorPn, | ||
| columnFormats, | ||
| filters: slice.getFilters(), | ||
| includeSearch, | ||
| metrics, | ||
| onAddFilter(...args) { slice.addFilter(...args); }, | ||
| orderDesc, | ||
| pageLength, | ||
| percentMetrics, | ||
| // Aug 22, 2018 | ||
| // Perhaps this `tableFilter` field can be removed as there is | ||
|
There was a problem hiding this comment. that's interesting. I think this table used to be able to function as a filter on dashboards but perhaps that broke at some point. @mistercrunch any thoughts? |
||
| // no code left in repo to set tableFilter to true. | ||
| // which make `onAddFilter` will never be called as well. | ||
| tableFilter, | ||
| tableTimestampFormat, | ||
| timeseriesLimitMetric, | ||
| verboseMap, | ||
| }); | ||
| } | ||
|
|
||
| export default adaptor; | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly good news: anything with
.widgetI think can be removed. this is what grid cells on the old dashboard used to be called.