From 4e7fc702891585d26d182f00d8830de2f238fde4 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 31 May 2022 16:32:22 +0800 Subject: [PATCH] refactor: decouple datatable controls fix lint typing --- .../src/explore/actions/exploreActions.ts | 28 ----- .../components/DataTableControl/index.tsx | 109 ++++++++++-------- .../DataTableControl/useTableColumns.test.ts | 6 +- .../utils.ts} | 36 ++++-- .../components/DataTableControls.tsx | 33 +++--- .../DataTablesPane/components/ResultsPane.tsx | 8 +- .../DataTablesPane/components/SamplesPane.tsx | 7 +- .../components/DataTablesPane/types.ts | 17 ++- .../src/explore/reducers/exploreReducer.js | 47 -------- .../src/explore/reducers/getInitialState.ts | 5 - 10 files changed, 133 insertions(+), 163 deletions(-) rename superset-frontend/src/explore/components/{useOriginalFormattedTimeColumns.ts => DataTableControl/utils.ts} (51%) diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts index 8e73b32a9cd63..fe45d0b63e110 100644 --- 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 index cc379eda63602..fb8af865a3914 100644 --- 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 = ({ }) => ; 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( + 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 = ({ ) : null, - [datasourceId, isTimeColumnOriginalFormatted, onChange], + [datasourceId, isOriginalTimeColumn], ); return datasourceId ? ( @@ -288,10 +263,45 @@ export const useTableColumns = ( coltypes?: GenericDataType[], data?: Record[], datasourceId?: string, - originalFormattedTimeColumns: string[] = [], + isVisible?: boolean, moreConfigs?: { [key: string]: Partial }, -) => - useMemo( +) => { + const [originalFormattedTimeColumns, setOriginalFormattedTimeColumns] = + useState(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 = ( ) : ( 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 index 0a3a73e6f09ef..8e51732c2e825 100644 --- 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/superset-frontend/src/explore/components/useOriginalFormattedTimeColumns.ts b/superset-frontend/src/explore/components/DataTableControl/utils.ts similarity index 51% rename from superset-frontend/src/explore/components/useOriginalFormattedTimeColumns.ts rename to superset-frontend/src/explore/components/DataTableControl/utils.ts index 140cd3ba6223d..f8e8e42e31d43 100644 --- a/superset-frontend/src/explore/components/useOriginalFormattedTimeColumns.ts +++ b/superset-frontend/src/explore/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(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 index 738a20c3b95ec..eedde328d19c5 100644 --- 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[]; - datasourceId?: string; - onInputChange: (input: string) => void; - columnNames: string[]; - isLoading: boolean; -}) => { - const originalFormattedTimeColumns = - useOriginalFormattedTimeColumns(datasourceId); +}: TableControlsProps) => { + const originalTimeColumns = getTimeColumns(datasourceId); + const formttedTimeColumns = zip( + 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 ( diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/ResultsPane.tsx index a71a1232efbf9..d69a244430550 100644 --- 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,9 +110,6 @@ 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( @@ -121,7 +117,7 @@ export const ResultsPane = ({ coltypes, data, queryFormData.datasource, - originalFormattedTimeColumns, + isRequest, ); const filteredData = useFilteredTableData(filterText, data); @@ -140,6 +136,7 @@ export const ResultsPane = ({ setFilterText(input)} isLoading={isLoading} @@ -159,6 +156,7 @@ export const ResultsPane = ({ setFilterText(input)} isLoading={isLoading} diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx index be30e4e827aa8..1997acf596ede 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx @@ -25,7 +25,6 @@ import { useFilteredTableData, useTableColumns, } from 'src/explore/components/DataTableControl'; -import { useOriginalFormattedTimeColumns } from 'src/explore/components/useOriginalFormattedTimeColumns'; import { getDatasetSamples } from 'src/components/Chart/chartAction'; import { TableControls } from './DataTableControls'; import { SamplesPaneProps } from '../types'; @@ -84,8 +83,6 @@ export const SamplesPane = ({ } }, [datasource, isRequest, queryForce]); - const originalFormattedTimeColumns = - useOriginalFormattedTimeColumns(datasourceId); // 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( @@ -93,7 +90,7 @@ export const SamplesPane = ({ coltypes, data, datasourceId, - originalFormattedTimeColumns, + isRequest, ); const filteredData = useFilteredTableData(filterText, data); @@ -107,6 +104,7 @@ export const SamplesPane = ({ setFilterText(input)} isLoading={isLoading} @@ -126,6 +124,7 @@ export const SamplesPane = ({ setFilterText(input)} isLoading={isLoading} diff --git a/superset-frontend/src/explore/components/DataTablesPane/types.ts b/superset-frontend/src/explore/components/DataTablesPane/types.ts index c17ae06e407f8..f526536640c6e 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/types.ts +++ b/superset-frontend/src/explore/components/DataTablesPane/types.ts @@ -16,7 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { Datasource, JsonObject, QueryFormData } from '@superset-ui/core'; +import { + Datasource, + GenericDataType, + JsonObject, + QueryFormData, +} from '@superset-ui/core'; import { ExploreActions } from 'src/explore/actions/exploreActions'; import { ChartStatus } from 'src/explore/types'; @@ -48,3 +53,13 @@ export interface SamplesPaneProps { actions?: ExploreActions; dataSize?: number; } + +export interface TableControlsProps { + data: Record[]; + // {datasource.id}__{datasource.type}, eg: 1__table + datasourceId: string; + onInputChange: (input: string) => void; + columnNames: string[]; + columnTypes: GenericDataType[]; + isLoading: boolean; +} diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 4dfcc9a1781bc..2897832ff055c 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -28,7 +28,6 @@ import { getControlValuesCompatibleWithDatasource, } from 'src/explore/controlUtils'; import * as actions from 'src/explore/actions/exploreActions'; -import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers'; export default function exploreReducer(state = {}, action) { const actionHandlers = { @@ -265,52 +264,6 @@ export default function exploreReducer(state = {}, action) { sliceName: action.slice.slice_name ?? state.sliceName, }; }, - [actions.SET_ORIGINAL_FORMATTED_TIME_COLUMN]() { - const { datasourceId, columnName } = action; - const newOriginalFormattedColumns = { - ...state.originalFormattedTimeColumns, - }; - const newOriginalFormattedColumnsForDatasource = ensureIsArray( - newOriginalFormattedColumns[datasourceId], - ).slice(); - - newOriginalFormattedColumnsForDatasource.push(columnName); - newOriginalFormattedColumns[datasourceId] = - newOriginalFormattedColumnsForDatasource; - setItem( - LocalStorageKeys.explore__data_table_original_formatted_time_columns, - newOriginalFormattedColumns, - ); - return { - ...state, - originalFormattedTimeColumns: newOriginalFormattedColumns, - }; - }, - [actions.UNSET_ORIGINAL_FORMATTED_TIME_COLUMN]() { - const { datasourceId, columnIndex } = action; - const newOriginalFormattedColumns = { - ...state.originalFormattedTimeColumns, - }; - const newOriginalFormattedColumnsForDatasource = ensureIsArray( - newOriginalFormattedColumns[datasourceId], - ).slice(); - - newOriginalFormattedColumnsForDatasource.splice(columnIndex, 1); - newOriginalFormattedColumns[datasourceId] = - newOriginalFormattedColumnsForDatasource; - - if (newOriginalFormattedColumnsForDatasource.length === 0) { - delete newOriginalFormattedColumns[datasourceId]; - } - setItem( - LocalStorageKeys.explore__data_table_original_formatted_time_columns, - newOriginalFormattedColumns, - ); - return { - ...state, - originalFormattedTimeColumns: newOriginalFormattedColumns, - }; - }, [actions.SET_FORCE_QUERY]() { return { ...state, diff --git a/superset-frontend/src/explore/reducers/getInitialState.ts b/superset-frontend/src/explore/reducers/getInitialState.ts index 659e834d2faad..45440f6f5b4b9 100644 --- a/superset-frontend/src/explore/reducers/getInitialState.ts +++ b/superset-frontend/src/explore/reducers/getInitialState.ts @@ -35,7 +35,6 @@ import { getFormDataFromControls, applyMapStateToPropsToControl, } from 'src/explore/controlUtils'; -import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; export interface ExplorePageBootstrapData extends JsonObject { can_add: boolean; @@ -78,10 +77,6 @@ export default function getInitialState( initialFormData, ) as ControlStateMapping, controlsTransferred: [], - originalFormattedTimeColumns: getItem( - LocalStorageKeys.explore__data_table_original_formatted_time_columns, - {}, - ), }; // apply initial mapStateToProps for all controls, must execute AFTER