You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by er...@apache.org on 2020/10/03 04:24:05 UTC

[incubator-superset] 01/01: Revert "refactor: Remove usages of reactable from TimeTable (#11046)"

This is an automated email from the ASF dual-hosted git repository.

erikrit pushed a commit to branch revert-11046-reactable/timetable
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit 1474fd77c4c468acbec572f7cfb6dea2b222f6e6
Author: Erik Ritter <er...@airbnb.com>
AuthorDate: Fri Oct 2 21:23:35 2020 -0700

    Revert "refactor: Remove usages of reactable from TimeTable (#11046)"
    
    This reverts commit f01b8a30f7af781b06d06e12fd6d895002333a02.
---
 .../src/components/ListView/ListView.tsx           |  60 ++----
 .../src/components/ListView/TableCollection.tsx    |  12 +-
 superset-frontend/src/components/ListView/utils.ts |   4 +-
 .../src/visualizations/TimeTable/TimeTable.jsx     | 206 ++++++++++-----------
 4 files changed, 126 insertions(+), 156 deletions(-)

diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx
index 72948d5..e0e19c5 100644
--- a/superset-frontend/src/components/ListView/ListView.tsx
+++ b/superset-frontend/src/components/ListView/ListView.tsx
@@ -17,7 +17,6 @@
  * under the License.
  */
 import { t, styled } from '@superset-ui/core';
-import { css } from '@emotion/core';
 import React, { useState } from 'react';
 import { Alert } from 'react-bootstrap';
 import { Empty } from 'src/common/components';
@@ -38,11 +37,7 @@ import {
 } from './types';
 import { ListViewError, useListViewState } from './utils';
 
-interface ListViewStylesProps {
-  fullHeight: boolean;
-}
-
-const ListViewStyles = styled.div<ListViewStylesProps>`
+const ListViewStyles = styled.div`
   text-align: center;
 
   .superset-list-view {
@@ -53,12 +48,8 @@ const ListViewStyles = styled.div<ListViewStylesProps>`
     padding-bottom: 48px;
 
     .body {
-      ${({ fullHeight }) =>
-        !fullHeight &&
-        css`
-          overflow: scroll;
-          max-height: 64vh;
-        `};
+      overflow: scroll;
+      max-height: 64vh;
     }
   }
 
@@ -211,9 +202,6 @@ export interface ListViewProps<T extends object = any> {
   renderCard?: (row: T & { loading: boolean }) => React.ReactNode;
   cardSortSelectOptions?: Array<CardSortSelectOption>;
   defaultViewMode?: ViewModeType;
-  sticky?: boolean;
-  fullHeight?: boolean;
-  manualSortBy?: boolean;
 }
 
 function ListView<T extends object = any>({
@@ -233,9 +221,6 @@ function ListView<T extends object = any>({
   renderCard,
   cardSortSelectOptions,
   defaultViewMode = 'card',
-  sticky = true,
-  fullHeight = false,
-  manualSortBy = true,
 }: ListViewProps<T>) {
   const {
     getTableProps,
@@ -259,10 +244,8 @@ function ListView<T extends object = any>({
     initialPageSize,
     initialSort,
     initialFilters: filters,
-    manualSortBy,
   });
   const filterable = Boolean(filters.length);
-  const withPagination = Boolean(count);
   if (filterable) {
     const columnAccessors = columns.reduce(
       (acc, col) => ({ ...acc, [col.accessor || col.id]: true }),
@@ -283,7 +266,7 @@ function ListView<T extends object = any>({
   );
 
   return (
-    <ListViewStyles fullHeight={fullHeight}>
+    <ListViewStyles>
       <div className={`superset-list-view ${className}`}>
         <div className="header">
           {cardViewEnabled && (
@@ -367,7 +350,6 @@ function ListView<T extends object = any>({
               rows={rows}
               columns={columns}
               loading={loading}
-              sticky={sticky}
             />
           )}
           {!loading && rows.length === 0 && (
@@ -378,25 +360,23 @@ function ListView<T extends object = any>({
         </div>
       </div>
 
-      {withPagination && (
-        <div className="pagination-container">
-          <Pagination
-            totalPages={pageCount || 0}
-            currentPage={pageCount ? pageIndex + 1 : 0}
-            onChange={(p: number) => gotoPage(p - 1)}
-            hideFirstAndLastPageLinks
-          />
-          <div className="row-count-container">
-            {!loading &&
-              t(
-                '%s-%s of %s',
-                pageSize * pageIndex + (rows.length && 1),
-                pageSize * pageIndex + rows.length,
-                count,
-              )}
-          </div>
+      <div className="pagination-container">
+        <Pagination
+          totalPages={pageCount || 0}
+          currentPage={pageCount ? pageIndex + 1 : 0}
+          onChange={(p: number) => gotoPage(p - 1)}
+          hideFirstAndLastPageLinks
+        />
+        <div className="row-count-container">
+          {!loading &&
+            t(
+              '%s-%s of %s',
+              pageSize * pageIndex + (rows.length && 1),
+              pageSize * pageIndex + rows.length,
+              count,
+            )}
         </div>
-      )}
+      </div>
     </ListViewStyles>
   );
 }
diff --git a/superset-frontend/src/components/ListView/TableCollection.tsx b/superset-frontend/src/components/ListView/TableCollection.tsx
index a68bcf4..4dbbc86 100644
--- a/superset-frontend/src/components/ListView/TableCollection.tsx
+++ b/superset-frontend/src/components/ListView/TableCollection.tsx
@@ -30,19 +30,14 @@ interface TableCollectionProps {
   rows: TableInstance['rows'];
   columns: TableInstance['column'][];
   loading: boolean;
-  sticky: boolean;
 }
 
-interface TableProps {
-  sticky: boolean;
-}
-
-const Table = styled.table<TableProps>`
+const Table = styled.table`
   border-collapse: separate;
 
   th {
     background: ${({ theme }) => theme.colors.grayscale.light5};
-    position: ${({ sticky }) => sticky && 'sticky'};
+    position: sticky;
     top: 0;
 
     &:first-of-type {
@@ -204,10 +199,9 @@ export default function TableCollection({
   columns,
   rows,
   loading,
-  sticky,
 }: TableCollectionProps) {
   return (
-    <Table {...getTableProps()} sticky={sticky} className="table table-hover">
+    <Table {...getTableProps()} className="table table-hover">
       <thead>
         {headerGroups.map(headerGroup => (
           <tr {...headerGroup.getHeaderGroupProps()}>
diff --git a/superset-frontend/src/components/ListView/utils.ts b/superset-frontend/src/components/ListView/utils.ts
index 92605d2..c8ecc95 100644
--- a/superset-frontend/src/components/ListView/utils.ts
+++ b/superset-frontend/src/components/ListView/utils.ts
@@ -110,7 +110,6 @@ interface UseListViewConfig {
     Header: (conf: any) => React.ReactNode;
     Cell: (conf: any) => React.ReactNode;
   };
-  manualSortBy: boolean;
 }
 
 export function useListViewState({
@@ -123,7 +122,6 @@ export function useListViewState({
   initialSort = [],
   bulkSelectMode = false,
   bulkSelectColumnConfig,
-  manualSortBy,
 }: UseListViewConfig) {
   const [query, setQuery] = useQueryParams({
     filters: JsonParam,
@@ -179,9 +177,9 @@ export function useListViewState({
       initialState,
       manualFilters: true,
       manualPagination: true,
+      manualSortBy: true,
       autoResetFilters: false,
       pageCount: Math.ceil(count / initialPageSize),
-      manualSortBy,
     },
     useFilters,
     useSortBy,
diff --git a/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx b/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx
index d27a1e8..c384cb0 100644
--- a/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx
+++ b/superset-frontend/src/visualizations/TimeTable/TimeTable.jsx
@@ -20,14 +20,14 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import Mustache from 'mustache';
 import { scaleLinear } from 'd3-scale';
+import { Table, Thead, Th, Tr, Td } from 'reactable-arc';
 import { formatNumber, formatTime } from '@superset-ui/core';
 import {
   InfoTooltipWithTrigger,
   MetricOption,
 } from '@superset-ui/chart-controls';
 import moment from 'moment';
-import ListView from 'src/components/ListView';
-import { memoize } from 'lodash-es';
+
 import FormattedNumber from './FormattedNumber';
 import SparklineCell from './SparklineCell';
 import './TimeTable.less';
@@ -90,68 +90,6 @@ const defaultProps = {
 };
 
 class TimeTable extends React.PureComponent {
-  memoizedColumns = memoize(() => [
-    { accessor: 'metric', Header: 'Metric' },
-    ...this.props.columnConfigs.map((columnConfig, i) => ({
-      accessor: columnConfig.key,
-      cellProps: columnConfig.colType === 'spark' && { style: { width: '1%' } },
-      Header: () => (
-        <>
-          {columnConfig.label}{' '}
-          {columnConfig.tooltip && (
-            <InfoTooltipWithTrigger
-              tooltip={columnConfig.tooltip}
-              label={`tt-col-${i}`}
-              placement="top"
-            />
-          )}
-        </>
-      ),
-      sortType: (rowA, rowB, columnId) => {
-        const rowAVal = rowA.values[columnId].props['data-value'];
-        const rowBVal = rowB.values[columnId].props['data-value'];
-        return rowAVal - rowBVal;
-      },
-    })),
-  ]);
-
-  memoizedRows = memoize(() => {
-    const entries = Object.keys(this.props.data)
-      .sort()
-      .map(time => ({ ...this.props.data[time], time }));
-    const reversedEntries = entries.concat().reverse();
-
-    return this.props.rows.map(row => {
-      const valueField = row.label || row.metric_name;
-      const cellValues = this.props.columnConfigs.reduce(
-        (acc, columnConfig) => {
-          if (columnConfig.colType === 'spark') {
-            return {
-              ...acc,
-              [columnConfig.key]: this.renderSparklineCell(
-                valueField,
-                columnConfig,
-                entries,
-              ),
-            };
-          }
-          return {
-            ...acc,
-            [columnConfig.key]: this.renderValueCell(
-              valueField,
-              columnConfig,
-              reversedEntries,
-            ),
-          };
-        },
-        {},
-      );
-      return { ...row, ...cellValues, metric: this.renderLeftCell(row) };
-    });
-  });
-
-  initialSort = [{ id: 'metric', desc: false }];
-
   renderLeftCell(row) {
     const { rowType, url } = this.props;
     const context = { metric: row };
@@ -169,9 +107,10 @@ class TimeTable extends React.PureComponent {
       return column.label;
     }
 
+    const metric = row;
     return (
       <MetricOption
-        metric={row}
+        metric={metric}
         url={fullUrl}
         showFormula={false}
         openInNewWindow
@@ -197,27 +136,32 @@ class TimeTable extends React.PureComponent {
     }
 
     return (
-      <SparklineCell
-        width={parseInt(column.width, 10) || 300}
-        height={parseInt(column.height, 10) || 50}
-        data={sparkData}
-        data-value={sparkData[sparkData.length - 1]}
-        ariaLabel={`spark-${valueField}`}
-        numberFormat={column.d3format}
-        yAxisBounds={column.yAxisBounds}
-        showYAxis={column.showYAxis}
-        renderTooltip={({ index }) => (
-          <div>
-            <strong>{formatNumber(column.d3format, sparkData[index])}</strong>
+      <Td
+        column={column.key}
+        key={column.key}
+        value={sparkData[sparkData.length - 1]}
+      >
+        <SparklineCell
+          width={parseInt(column.width, 10) || 300}
+          height={parseInt(column.height, 10) || 50}
+          data={sparkData}
+          ariaLabel={`spark-${valueField}`}
+          numberFormat={column.d3format}
+          yAxisBounds={column.yAxisBounds}
+          showYAxis={column.showYAxis}
+          renderTooltip={({ index }) => (
             <div>
-              {formatTime(
-                column.dateFormat,
-                moment.utc(entries[index].time).toDate(),
-              )}
+              <strong>{formatNumber(column.d3format, sparkData[index])}</strong>
+              <div>
+                {formatTime(
+                  column.dateFormat,
+                  moment.utc(entries[index].time).toDate(),
+                )}
+              </div>
             </div>
-          </div>
-        )}
-      />
+          )}
+        />
+      </Td>
     );
   }
 
@@ -260,9 +204,10 @@ class TimeTable extends React.PureComponent {
     const color = colorFromBounds(v, column.bounds);
 
     return (
-      <span
+      <Td
+        column={column.key}
         key={column.key}
-        data-value={v}
+        value={v}
         style={
           color && {
             boxShadow: `inset 0px -2.5px 0px 0px ${color}`,
@@ -271,34 +216,87 @@ class TimeTable extends React.PureComponent {
         }
       >
         {errorMsg ? (
-          { errorMsg }
+          <div>{errorMsg}</div>
         ) : (
-          <span style={{ color }}>
+          <div style={{ color }}>
             <FormattedNumber num={v} format={column.d3format} />
-          </span>
+          </div>
+        )}
+      </Td>
+    );
+  }
+
+  renderRow(row, entries, reversedEntries) {
+    const { columnConfigs } = this.props;
+    const valueField = row.label || row.metric_name;
+    const leftCell = this.renderLeftCell(row);
+
+    return (
+      <Tr key={leftCell}>
+        <Td column="metric" data={leftCell}>
+          {leftCell}
+        </Td>
+        {columnConfigs.map(c =>
+          c.colType === 'spark'
+            ? this.renderSparklineCell(valueField, c, entries)
+            : this.renderValueCell(valueField, c, reversedEntries),
         )}
-      </span>
+      </Tr>
     );
   }
 
   render() {
-    const { className, height } = this.props;
+    const {
+      className,
+      height,
+      data,
+      columnConfigs,
+      rowType,
+      rows,
+    } = this.props;
+
+    const entries = Object.keys(data)
+      .sort()
+      .map(time => ({ ...data[time], time }));
+    const reversedEntries = entries.concat().reverse();
+
+    const defaultSort =
+      rowType === 'column' && columnConfigs.length
+        ? {
+            column: columnConfigs[0].key,
+            direction: 'desc',
+          }
+        : false;
 
     return (
       <div className={`time-table ${className}`} style={{ height }}>
-        <ListView
-          columns={this.memoizedColumns()}
-          data={this.memoizedRows()}
-          count={0}
-          // we don't use pagination
-          pageSize={0}
-          initialSort={this.initialSort}
-          fetchData={() => {}}
-          loading={false}
-          sticky={false}
-          fullHeight
-          manualSortBy={false}
-        />
+        <Table
+          className="table table-no-hover"
+          defaultSort={defaultSort}
+          sortBy={defaultSort}
+          sortable={columnConfigs.map(c => c.key)}
+        >
+          <Thead>
+            <Th column="metric">Metric</Th>
+            {columnConfigs.map((c, i) => (
+              <Th
+                key={c.key}
+                column={c.key}
+                width={c.colType === 'spark' ? '1%' : null}
+              >
+                {c.label}{' '}
+                {c.tooltip && (
+                  <InfoTooltipWithTrigger
+                    tooltip={c.tooltip}
+                    label={`tt-col-${i}`}
+                    placement="top"
+                  />
+                )}
+              </Th>
+            ))}
+          </Thead>
+          {rows.map(row => this.renderRow(row, entries, reversedEntries))}
+        </Table>
       </div>
     );
   }