feat: Uses new table component in Drill to Detail (#22173)

apache · Nov 30, 2022 · 3ffe782 · 3ffe782
1 parent e80e10e
commit 3ffe782
Show file tree

Hide file tree

Showing 18 changed files with 684 additions and 92 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/drilltodetail.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/drilltodetail.test.ts
@@ -157,62 +157,47 @@ describe('Drill to detail modal', () => {
       it('refreshes the data', () => {
         openModalFromMenu('big_number_total');
         // move to the last page
-        cy.get(".pagination-container [role='navigation'] [role='button']")
-          .eq(7)
-          .click();
+        cy.get('.ant-pagination-item').eq(5).click();
+        // skips error on pagination
+        cy.on('uncaught:exception', () => false);
         cy.wait('@samples');
         // reload
         cy.get("[aria-label='reload']").click();
         cy.wait('@samples');
         // make sure it started back from first page
-        cy.get(".pagination-container [role='navigation'] li.active").should(
-          'contain',
-          '1',
-        );
+        cy.get('.ant-pagination-item-active').should('contain', '1');
       });
 
       it('paginates', () => {
         openModalFromMenu('big_number_total');
         // checking the data
         cy.getBySel('row-count-label').should('contain', '75.7k rows');
-        cy.get(".ant-modal-body [role='rowgroup'] [role='row']")
-          .should('have.length', 50)
-          .then($rows => {
-            expect($rows).to.contain('Amy');
-          });
+        cy.get('.virtual-table-cell').then($rows => {
+          expect($rows).to.contain('Amy');
+        });
         // checking the paginated data
-        cy.get(".pagination-container [role='navigation'] [role='button']")
-          .should('have.length', 9)
+        cy.get('.ant-pagination-item')
+          .should('have.length', 6)
           .then($pages => {
             expect($pages).to.contain('1');
             expect($pages).to.contain('1514');
           });
-        cy.get(".pagination-container [role='navigation'] [role='button']")
-          .eq(7)
-          .click();
+        cy.get('.ant-pagination-item').eq(4).click();
+        // skips error on pagination
+        cy.on('uncaught:exception', () => false);
         cy.wait('@samples');
-        cy.get("[role='rowgroup'] [role='row']")
-          .should('have.length', 43)
-          .then($rows => {
-            expect($rows).to.contain('Victoria');
-          });
+        cy.get('.virtual-table-cell').then($rows => {
+          expect($rows).to.contain('Kelly');
+        });
 
         // verify scroll top on pagination
-        cy.getBySelLike('Number-modal')
-          .find('.table-condensed')
-          .scrollTo(0, 100);
+        cy.getBySelLike('Number-modal').find('.virtual-grid').scrollTo(0, 200);
 
-        cy.get("[role='rowgroup'] [role='row']")
-          .contains('Miguel')
-          .should('not.be.visible');
+        cy.get('.virtual-grid').contains('Juan').should('not.be.visible');
 
-        cy.get(".pagination-container [role='navigation'] [role='button']")
-          .eq(1)
-          .click();
+        cy.get('.ant-pagination-item').eq(0).click();
 
-        cy.get("[role='rowgroup'] [role='row']")
-          .contains('Aaron')
-          .should('be.visible');
+        cy.get('.virtual-grid').contains('Aaron').should('be.visible');
       });
     });
 
@@ -478,8 +463,8 @@ describe('Drill to detail modal', () => {
           // checking the filter
           cy.getBySel('filter-val').should('contain', 'boy');
           cy.getBySel('row-count-label').should('contain', '39.2k rows');
-          cy.get(".pagination-container [role='navigation'] [role='button']")
-            .should('have.length', 9)
+          cy.get('.ant-pagination-item')
+            .should('have.length', 6)
             .then($pages => {
               expect($pages).to.contain('1');
               expect($pages).to.contain('785');
@@ -489,12 +474,9 @@ describe('Drill to detail modal', () => {
           cy.getBySel('filter-col').find("[aria-label='close']").click();
           cy.wait('@samples');
           cy.getBySel('row-count-label').should('contain', '75.7k rows');
-          cy.get(".pagination-container [role='navigation'] li.active").should(
-            'contain',
-            '1',
-          );
-          cy.get(".pagination-container [role='navigation'] [role='button']")
-            .should('have.length', 9)
+          cy.get('.ant-pagination-item-active').should('contain', '1');
+          cy.get('.ant-pagination-item')
+            .should('have.length', 6)
             .then($pages => {
               expect($pages).to.contain('1');
               expect($pages).to.contain('1514');

diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx
@@ -110,6 +110,7 @@ export default function DrillDetailModal({
       }}
       draggable
       destroyOnClose
+      maskClosable={false}
     >
       <DrillDetailPane formData={formData} initialFilters={initialFilters} />
     </Modal>

diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.test.tsx
@@ -134,7 +134,12 @@ test('should render the table with results', async () => {
   fetchWithData();
   await waitForRender();
   expect(screen.getByRole('table')).toBeInTheDocument();
-  expect(screen.getAllByRole('row')).toHaveLength(4);
+  expect(screen.getByText('1996')).toBeInTheDocument();
+  expect(screen.getByText('11.27')).toBeInTheDocument();
+  expect(screen.getByText('1989')).toBeInTheDocument();
+  expect(screen.getByText('23.2')).toBeInTheDocument();
+  expect(screen.getByText('1999')).toBeInTheDocument();
+  expect(screen.getByText('9')).toBeInTheDocument();
   expect(
     screen.getByRole('columnheader', { name: 'year' }),
   ).toBeInTheDocument();

diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailPane.tsx
@@ -22,6 +22,7 @@ import React, {
   useMemo,
   useCallback,
   useRef,
+  ReactElement,
 } from 'react';
 import { useSelector } from 'react-redux';
 import {
@@ -32,24 +33,54 @@ import {
   useTheme,
   QueryFormData,
   JsonObject,
+  GenericDataType,
 } from '@superset-ui/core';
+import { useResizeDetector } from 'react-resize-detector';
 import Loading from 'src/components/Loading';
+import BooleanCell from 'src/components/Table/cell-renderers/BooleanCell';
+import NullCell from 'src/components/Table/cell-renderers/NullCell';
+import TimeCell from 'src/components/Table/cell-renderers/TimeCell';
 import { EmptyStateMedium } from 'src/components/EmptyState';
-import TableView, { EmptyWrapperType } from 'src/components/TableView';
-import { useTableColumns } from 'src/explore/components/DataTableControl';
 import { getDatasourceSamples } from 'src/components/Chart/chartAction';
+import Table, {
+  ColumnsType,
+  TablePaginationConfig,
+  TableSize,
+} from 'src/components/Table';
 import MetadataBar, {
   ContentType,
   MetadataType,
 } from 'src/components/MetadataBar';
 import Alert from 'src/components/Alert';
 import { useApiV1Resource } from 'src/hooks/apiResources';
+import HeaderWithRadioGroup from 'src/components/Table/header-renderers/HeaderWithRadioGroup';
 import TableControls from './DrillDetailTableControls';
 import { getDrillPayload } from './utils';
 import { Dataset, ResultsPage } from './types';
 
 const PAGE_SIZE = 50;
 
+interface DataType {
+  [key: string]: any;
+}
+
+// Must be outside of the main component due to problems in
+// react-resize-detector with conditional rendering
+// https://github.com/maslianok/react-resize-detector/issues/178
+function Resizable({ children }: { children: ReactElement }) {
+  const { ref, height } = useResizeDetector();
+  return (
+    <div ref={ref} css={{ flex: 1 }}>
+      {React.cloneElement(children, { height })}
+    </div>
+  );
+}
+
+enum TimeFormatting {
+  Original,
+  Formatted,
+}
+
 export default function DrillDetailPane({
   formData,
   initialFilters,
@@ -66,6 +97,7 @@ export default function DrillDetailPane({
   const [resultsPages, setResultsPages] = useState<Map<number, ResultsPage>>(
     new Map(),
   );
+  const [timeFormatting, setTimeFormatting] = useState({});
 
   const SAMPLES_ROW_LIMIT = useSelector(
     (state: { common: { conf: JsonObject } }) =>
@@ -89,30 +121,69 @@ export default function DrillDetailPane({
     return resultsPages.get(lastPageIndex.current);
   }, [pageIndex, resultsPages]);
 
-  // this is to preserve the order of the columns, even if there are integer values,
-  // while also only grabbing the first column's keys
-  const columns = useTableColumns(
-    resultsPage?.colNames,
-    resultsPage?.colTypes,
-    resultsPage?.data,
-    formData.datasource,
+  const mappedColumns: ColumnsType<DataType> = useMemo(
+    () =>
+      resultsPage?.colNames.map((column, index) => ({
+        key: column,
+        dataIndex: column,
+        title:
+          resultsPage?.colTypes[index] === GenericDataType.TEMPORAL ? (
+            <HeaderWithRadioGroup
+              headerTitle={column}
+              groupTitle={t('Formatting')}
+              groupOptions={[
+                { label: t('Original value'), value: TimeFormatting.Original },
+                {
+                  label: t('Formatted value'),
+                  value: TimeFormatting.Formatted,
+                },
+              ]}
+              value={
+                timeFormatting[column] === TimeFormatting.Original
+                  ? TimeFormatting.Original
+                  : TimeFormatting.Formatted
+              }
+              onChange={value =>
+                setTimeFormatting(state => ({ ...state, [column]: value }))
+              }
+            />
+          ) : (
+            column
+          ),
+        render: value => {
+          if (value === true || value === false) {
+            return <BooleanCell value={value} />;
+          }
+          if (value === null) {
+            return <NullCell />;
+          }
+          if (
+            resultsPage?.colTypes[index] === GenericDataType.TEMPORAL &&
+            timeFormatting[column] !== TimeFormatting.Original &&
+            (typeof value === 'number' || value instanceof Date)
+          ) {
+            return <TimeCell value={value} />;
+          }
+          return String(value);
+        },
+        width: 150,
+      })) || [],
+    [resultsPage?.colNames, resultsPage?.colTypes, timeFormatting],
   );
 
-  // Disable sorting on columns
-  const sortDisabledColumns = useMemo(
+  const data: DataType[] = useMemo(
     () =>
-      columns.map(column => ({
-        ...column,
-        disableSortBy: true,
-      })),
-    [columns],
+      resultsPage?.data.map((row, index) =>
+        resultsPage?.colNames.reduce(
+          (acc, curr) => ({ ...acc, [curr]: row[curr] }),
+          {
+            key: index,
+          },
+        ),
+      ) || [],
+    [resultsPage?.colNames, resultsPage?.data],
   );
 
-  // Update page index on pagination click
-  const onServerPagination = useCallback(({ pageIndex }) => {
-    setPageIndex(pageIndex);
-  }, []);
-
   // Clear cache on reload button click
   const handleReload = useCallback(() => {
     setResponseError('');
@@ -222,29 +293,22 @@ export default function DrillDetailPane({
   } else {
     // Render table if at least one page has successfully loaded
     tableContent = (
-      <TableView
-        columns={sortDisabledColumns}
-        data={resultsPage?.data || []}
-        pageSize={PAGE_SIZE}
-        totalCount={resultsPage?.total}
-        serverPagination
-        initialPageIndex={pageIndex}
-        onServerPagination={onServerPagination}
-        loading={isLoading}
-        noDataText={t('No results')}
-        emptyWrapperType={EmptyWrapperType.Small}
-        className="table-condensed"
-        isPaginationSticky
-        showRowCount={false}
-        small
-        css={css`
-          overflow: auto;
-          .table {
-            margin-bottom: 0;
+      <Resizable>
+        <Table
+          data={data}
+          columns={mappedColumns}
+          size={TableSize.SMALL}
+          defaultPageSize={PAGE_SIZE}
+          recordCount={resultsPage?.total}
+          usePagination
+          loading={isLoading}
+          onChange={(pagination: TablePaginationConfig) =>
+            setPageIndex(pagination.current ? pagination.current - 1 : 0)
           }
-        `}
-        scrollTopOnPagination
-      />
+          resizable
+          virtualize
+        />
+      </Resizable>
     );
   }
 

diff --git a/superset-frontend/src/components/Chart/DrillDetail/DrillDetailTableControls.tsx b/superset-frontend/src/components/Chart/DrillDetail/DrillDetailTableControls.tsx
@@ -82,6 +82,7 @@ export default function TableControls({
         display: flex;
         justify-content: space-between;
         padding: ${theme.gridUnit / 2}px 0;
+        margin-bottom: ${theme.gridUnit * 2}px;
       `}
     >
       <div