refactor: decouple datatable controls

fix lint typing
apache · Jun 1, 2022 · 4e7fc70 · 4e7fc70
1 parent 3d5ae62
commit 4e7fc70
Show file tree

Hide file tree

Showing 10 changed files with 133 additions and 163 deletions.
diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts
@@ -140,32 +140,6 @@ export function sliceUpdated(slice: Slice) {
   return { type: SLICE_UPDATED, slice };
 }
 
-export const SET_ORIGINAL_FORMATTED_TIME_COLUMN =
-  'SET_ORIGINAL_FORMATTED_TIME_COLUMN';
-export function setOriginalFormattedTimeColumn(
-  datasourceId: string,
-  columnName: string,
-) {
-  return {
-    type: SET_ORIGINAL_FORMATTED_TIME_COLUMN,
-    datasourceId,
-    columnName,
-  };
-}
-
-export const UNSET_ORIGINAL_FORMATTED_TIME_COLUMN =
-  'UNSET_ORIGINAL_FORMATTED_TIME_COLUMN';
-export function unsetOriginalFormattedTimeColumn(
-  datasourceId: string,
-  columnIndex: number,
-) {
-  return {
-    type: UNSET_ORIGINAL_FORMATTED_TIME_COLUMN,
-    datasourceId,
-    columnIndex,
-  };
-}
-
 export const SET_FORCE_QUERY = 'SET_FORCE_QUERY';
 export function setForceQuery(force: boolean) {
   return {
@@ -189,8 +163,6 @@ export const exploreActions = {
   updateChartTitle,
   createNewSlice,
   sliceUpdated,
-  setOriginalFormattedTimeColumn,
-  unsetOriginalFormattedTimeColumn,
   setForceQuery,
 };
 

diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useCallback, useMemo } from 'react';
+import React, { useMemo, useState, useEffect } from 'react';
 import {
   css,
   GenericDataType,
@@ -29,7 +29,6 @@ import {
 import { Global } from '@emotion/react';
 import { Column } from 'react-table';
 import debounce from 'lodash/debounce';
-import { useDispatch } from 'react-redux';
 import { Space } from 'src/components';
 import { Input } from 'src/components/Input';
 import {
@@ -45,10 +44,7 @@ import Popover from 'src/components/Popover';
 import { prepareCopyToClipboardTabularData } from 'src/utils/common';
 import CopyToClipboard from 'src/components/CopyToClipboard';
 import RowCountLabel from 'src/explore/components/RowCountLabel';
-import {
-  setOriginalFormattedTimeColumn,
-  unsetOriginalFormattedTimeColumn,
-} from 'src/explore/actions/exploreActions';
+import { getTimeColumns, setTimeColumns } from './utils';
 
 export const CellNull = styled('span')`
   color: ${({ theme }) => theme.colors.grayscale.light1};
@@ -130,8 +126,8 @@ export const RowCount = ({
 }) => <RowCountLabel rowcount={data?.length ?? 0} loading={loading} />;
 
 enum FormatPickerValue {
-  Formatted,
-  Original,
+  Formatted = 'formatted',
+  Original = 'original',
 }
 
 const FormatPicker = ({
@@ -165,47 +161,26 @@ const FormatPickerLabel = styled.span`
 
 const DataTableTemporalHeaderCell = ({
   columnName,
+  onTimeColumnChange,
   datasourceId,
-  originalFormattedTimeColumnIndex,
 }: {
   columnName: string;
+  onTimeColumnChange: (
+    columnName: string,
+    columnType: FormatPickerValue,
+  ) => void;
   datasourceId?: string;
-  originalFormattedTimeColumnIndex: number;
 }) => {
   const theme = useTheme();
-  const dispatch = useDispatch();
-  const isTimeColumnOriginalFormatted = originalFormattedTimeColumnIndex > -1;
-
-  const onChange = useCallback(
-    e => {
-      if (!datasourceId) {
-        return;
-      }
-      if (
-        e.target.value === FormatPickerValue.Original &&
-        !isTimeColumnOriginalFormatted
-      ) {
-        dispatch(setOriginalFormattedTimeColumn(datasourceId, columnName));
-      } else if (
-        e.target.value === FormatPickerValue.Formatted &&
-        isTimeColumnOriginalFormatted
-      ) {
-        dispatch(
-          unsetOriginalFormattedTimeColumn(
-            datasourceId,
-            originalFormattedTimeColumnIndex,
-          ),
-        );
-      }
-    },
-    [
-      originalFormattedTimeColumnIndex,
-      columnName,
-      datasourceId,
-      dispatch,
-      isTimeColumnOriginalFormatted,
-    ],
+  const [isOriginalTimeColumn, setIsOriginalTimeColumn] = useState<boolean>(
+    getTimeColumns(datasourceId).includes(columnName),
   );
+
+  const onChange = (e: any) => {
+    onTimeColumnChange(columnName, e.target.value);
+    setIsOriginalTimeColumn(getTimeColumns(datasourceId).includes(columnName));
+  };
+
   const overlayContent = useMemo(
     () =>
       datasourceId ? ( // eslint-disable-next-line jsx-a11y/no-static-element-interactions
@@ -222,14 +197,14 @@ const DataTableTemporalHeaderCell = ({
           <FormatPicker
             onChange={onChange}
             value={
-              isTimeColumnOriginalFormatted
+              isOriginalTimeColumn
                 ? FormatPickerValue.Original
                 : FormatPickerValue.Formatted
             }
           />
         </FormatPickerContainer>
       ) : null,
-    [datasourceId, isTimeColumnOriginalFormatted, onChange],
+    [datasourceId, isOriginalTimeColumn],
   );
 
   return datasourceId ? (
@@ -288,10 +263,45 @@ export const useTableColumns = (
   coltypes?: GenericDataType[],
   data?: Record<string, any>[],
   datasourceId?: string,
-  originalFormattedTimeColumns: string[] = [],
+  isVisible?: boolean,
   moreConfigs?: { [key: string]: Partial<Column> },
-) =>
-  useMemo(
+) => {
+  const [originalFormattedTimeColumns, setOriginalFormattedTimeColumns] =
+    useState<string[]>(getTimeColumns(datasourceId));
+
+  const onTimeColumnChange = (
+    columnName: string,
+    columnType: FormatPickerValue,
+  ) => {
+    if (!datasourceId) {
+      return;
+    }
+    if (
+      columnType === FormatPickerValue.Original &&
+      !originalFormattedTimeColumns.includes(columnName)
+    ) {
+      const cols = getTimeColumns(datasourceId);
+      cols.push(columnName);
+      setTimeColumns(datasourceId, cols);
+      setOriginalFormattedTimeColumns(cols);
+    } else if (
+      columnType === FormatPickerValue.Formatted &&
+      originalFormattedTimeColumns.includes(columnName)
+    ) {
+      const cols = getTimeColumns(datasourceId);
+      cols.splice(cols.indexOf(columnName), 1);
+      setTimeColumns(datasourceId, cols);
+      setOriginalFormattedTimeColumns(cols);
+    }
+  };
+
+  useEffect(() => {
+    if (isVisible) {
+      setOriginalFormattedTimeColumns(getTimeColumns(datasourceId));
+    }
+  }, [datasourceId, isVisible]);
+
+  return useMemo(
     () =>
       colnames && data?.length
         ? colnames
@@ -313,9 +323,7 @@ export const useTableColumns = (
                     <DataTableTemporalHeaderCell
                       columnName={key}
                       datasourceId={datasourceId}
-                      originalFormattedTimeColumnIndex={
-                        originalFormattedTimeColumnIndex
-                      }
+                      onTimeColumnChange={onTimeColumnChange}
                     />
                   ) : (
                     key
@@ -352,3 +360,4 @@ export const useTableColumns = (
       originalFormattedTimeColumns,
     ],
   );
+};
diff --git a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts
@@ -107,7 +107,7 @@ test('useTableColumns with no options', () => {
           name: 'DataTableTemporalHeaderCell',
         }),
         props: expect.objectContaining({
-          originalFormattedTimeColumnIndex: -1,
+          onTimeColumnChange: expect.any(Function),
         }),
       }),
       accessor: expect.any(Function),
@@ -135,7 +135,7 @@ test('useTableColumns with no options', () => {
 
 test('useTableColumns with options', () => {
   const hook = renderHook(() =>
-    useTableColumns(colnames, coltypes, data, undefined, [], {
+    useTableColumns(colnames, coltypes, data, undefined, true, {
       col01: { Header: 'Header' },
     }),
   );
@@ -171,7 +171,7 @@ test('useTableColumns with options', () => {
           name: 'DataTableTemporalHeaderCell',
         }),
         props: expect.objectContaining({
-          originalFormattedTimeColumnIndex: -1,
+          onTimeColumnChange: expect.any(Function),
         }),
       }),
       accessor: expect.any(Function),

diff --git a/...onents/useOriginalFormattedTimeColumns.ts → ...lore/components/DataTableControl/utils.ts b/...onents/useOriginalFormattedTimeColumns.ts → ...lore/components/DataTableControl/utils.ts
@@ -16,12 +16,34 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useSelector } from 'react-redux';
-import { ExplorePageState } from '../reducers/getInitialState';
+import { ensureIsArray } from '@superset-ui/core';
+import {
+  LocalStorageKeys,
+  setItem,
+  getItem,
+} from 'src/utils/localStorageHelpers';
 
-export const useOriginalFormattedTimeColumns = (datasourceId?: string) =>
-  useSelector<ExplorePageState, string[]>(state =>
-    datasourceId
-      ? state?.explore?.originalFormattedTimeColumns?.[datasourceId] ?? []
-      : [],
+export const getTimeColumns = (datasourceId?: string): string[] => {
+  const colsMap = getItem(
+    LocalStorageKeys.explore__data_table_original_formatted_time_columns,
+    {},
   );
+  if (datasourceId === undefined) {
+    return [];
+  }
+  return ensureIsArray(colsMap[datasourceId]);
+};
+
+export const setTimeColumns = (datasourceId: string, columns: string[]) => {
+  const colsMap = getItem(
+    LocalStorageKeys.explore__data_table_original_formatted_time_columns,
+    {},
+  );
+  setItem(
+    LocalStorageKeys.explore__data_table_original_formatted_time_columns,
+    {
+      ...colsMap,
+      [datasourceId]: columns,
+    },
+  );
+};
diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx
@@ -17,14 +17,16 @@
  * under the License.
  */
 import React, { useMemo } from 'react';
-import { css, styled } from '@superset-ui/core';
+import { zip } from 'lodash';
+import { css, GenericDataType, styled } from '@superset-ui/core';
 import {
   CopyToClipboardButton,
   FilterInput,
   RowCount,
 } from 'src/explore/components/DataTableControl';
 import { applyFormattingToTabularData } from 'src/utils/common';
-import { useOriginalFormattedTimeColumns } from 'src/explore/components/useOriginalFormattedTimeColumns';
+import { getTimeColumns } from 'src/explore/components/DataTableControl/utils';
+import { TableControlsProps } from '../types';
 
 export const TableControlsWrapper = styled.div`
   ${({ theme }) => `
@@ -44,19 +46,24 @@ export const TableControls = ({
   datasourceId,
   onInputChange,
   columnNames,
+  columnTypes,
   isLoading,
-}: {
-  data: Record<string, any>[];
-  datasourceId?: string;
-  onInputChange: (input: string) => void;
-  columnNames: string[];
-  isLoading: boolean;
-}) => {
-  const originalFormattedTimeColumns =
-    useOriginalFormattedTimeColumns(datasourceId);
+}: TableControlsProps) => {
+  const originalTimeColumns = getTimeColumns(datasourceId);
+  const formttedTimeColumns = zip<string, GenericDataType>(
+    columnNames,
+    columnTypes,
+  )
+    .filter(
+      ([name, type]) =>
+        type === GenericDataType.TEMPORAL &&
+        name &&
+        !originalTimeColumns.includes(name),
+    )
+    .map(([colname]) => colname);
   const formattedData = useMemo(
-    () => applyFormattingToTabularData(data, originalFormattedTimeColumns),
-    [data, originalFormattedTimeColumns],
+    () => applyFormattingToTabularData(data, formttedTimeColumns),
+    [data, formttedTimeColumns],
   );
   return (
     <TableControlsWrapper>

diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx
@@ -25,7 +25,6 @@ import {
   useFilteredTableData,
   useTableColumns,
 } from 'src/explore/components/DataTableControl';
-import { useOriginalFormattedTimeColumns } from 'src/explore/components/useOriginalFormattedTimeColumns';
 import { getChartDataRequest } from 'src/components/Chart/chartAction';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import { TableControls } from './DataTableControls';
@@ -111,17 +110,14 @@ export const ResultsPane = ({
     }
   }, [errorMessage]);
 
-  const originalFormattedTimeColumns = useOriginalFormattedTimeColumns(
-    queryFormData.datasource,
-  );
   // 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(
     colnames,
     coltypes,
     data,
     queryFormData.datasource,
-    originalFormattedTimeColumns,
+    isRequest,
   );
   const filteredData = useFilteredTableData(filterText, data);
 
@@ -140,6 +136,7 @@ export const ResultsPane = ({
         <TableControls
           data={filteredData}
           columnNames={colnames}
+          columnTypes={coltypes}
           datasourceId={queryFormData?.datasource}
           onInputChange={input => setFilterText(input)}
           isLoading={isLoading}
@@ -159,6 +156,7 @@ export const ResultsPane = ({
       <TableControls
         data={filteredData}
         columnNames={colnames}
+        columnTypes={coltypes}
         datasourceId={queryFormData?.datasource}
         onInputChange={input => setFilterText(input)}
         isLoading={isLoading}