From 2511e94ea6afcde340eca072aa88fb7f4940ce0c Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 4 Aug 2020 13:19:49 -0700 Subject: [PATCH] feat: align metrics title to the right (#721) --- .../src/DataTable/DataTable.tsx | 36 +------- .../src/DataTable/hooks/useColumnCellProps.ts | 38 -------- .../src/DataTable/index.tsx | 1 - .../src/DataTable/types/react-table.d.ts | 22 ++--- .../plugin-chart-table/src/TableChart.tsx | 91 +++++++++++-------- 5 files changed, 68 insertions(+), 120 deletions(-) delete mode 100644 superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/hooks/useColumnCellProps.ts diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index 906ea56b9155..248fcaf832c5 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -23,7 +23,6 @@ import { useSortBy, useGlobalFilter, PluginHook, - TableCellProps, TableOptions, FilterType, IdType, @@ -34,7 +33,6 @@ import GlobalFilter, { GlobalFilterProps } from './components/GlobalFilter'; import SelectPageSize, { SizeOption } from './components/SelectPageSize'; import SimplePagination from './components/Pagination'; import useSticky from './hooks/useSticky'; -import useColumnCellProps from './hooks/useColumnCellProps'; export interface DataTableProps extends TableOptions { tableClassName?: string; @@ -76,7 +74,6 @@ export default function DataTable({ useGlobalFilter, useSortBy, usePagination, - useColumnCellProps, doSticky ? useSticky : [], hooks || [], ].flat(); @@ -166,19 +163,10 @@ export default function DataTable({ return ( {headerGroup.headers.map(column => { - const { key: headerKey, className, ...props } = column.getHeaderProps( - column.getSortByToggleProps(), - ); - return ( - - {column.render('Header')} - {column.render('SortIcon')} - - ); + return column.render('Header', { + key: column.id, + ...column.getSortByToggleProps(), + }); })} ); @@ -191,21 +179,7 @@ export default function DataTable({ const { key: rowKey, ...rowProps } = row.getRowProps(); return ( - {row.cells.map(cell => { - const cellProps = cell.getCellProps() as TableCellProps & RenderHTMLCellProps; - const { key: cellKey, cellContent, ...restProps } = cellProps; - const key = cellKey || cell.column.id; - if (cellProps.dangerouslySetInnerHTML) { - return ; - } - // If cellProps renderes textContent already, then we don't have to - // render `Cell`. This saves some time for large tables. - return ( - - {cellContent || cell.render('Cell')} - - ); - })} + {row.cells.map(cell => cell.render('Cell', { key: cell.column.id }))} ); }) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/hooks/useColumnCellProps.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/hooks/useColumnCellProps.ts deleted file mode 100644 index 6acf25bbcc68..000000000000 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/hooks/useColumnCellProps.ts +++ /dev/null @@ -1,38 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { HTMLProps } from 'react'; -import { Hooks, Cell, TableCellProps, ColumnInstance } from 'react-table'; - -export interface UseColumnCellPropsColumnOption { - cellProps?: ( - cell: Cell, - props: Partial, - ) => Partial> | undefined; -} - -/** - * Configure cell props in column options. - */ -export default function useColumnCellProps(hooks: Hooks) { - hooks.getCellProps.push((props, { cell }) => { - const column = cell.column as ColumnInstance; - return (column.cellProps && column.cellProps(cell, props)) || []; - }); -} -useColumnCellProps.pluginName = 'useColumnCellProps'; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/index.tsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/index.tsx index 1af37594b639..b5e2a6ee5459 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/index.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/index.tsx @@ -16,7 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -export * from './hooks/useColumnCellProps'; export * from './hooks/useSticky'; export * from './components/GlobalFilter'; export * from './components/Pagination'; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts index af7aaeacc7a3..ce5f5a7de0d7 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts @@ -19,10 +19,10 @@ import { UseSortByHooks, TableInstance, ColumnInstance, - Column, + Renderer, + HeaderProps, } from 'react-table'; -import { UseColumnCellPropsColumnOption } from '../hooks/useColumnCellProps'; import { UseStickyState, UseStickyTableOptions, UseStickyInstanceProps } from '../hooks/useSticky'; declare module 'react-table' { @@ -53,13 +53,6 @@ declare module 'react-table' { UseSortByState, UseStickyState {} - export interface ColumnInterface - extends UseGlobalFiltersColumnOptions, - UseSortByColumnOptions, - UseColumnCellPropsColumnOption { - SortIcon: Column['Header']; - } - // Typing from @types/react-table is incomplete interface TableSortByToggleProps { style?: React.CSSProperties; @@ -67,10 +60,17 @@ declare module 'react-table' { onClick?: React.MouseEventHandler; } + export interface ColumnInterface + extends UseGlobalFiltersColumnOptions, + UseSortByColumnOptions { + // must define as a new property because it's not possible to override + // the existing `Header` renderer option + Header?: Renderer>; + } + export interface ColumnInstance extends UseGlobalFiltersColumnOptions, - UseSortByColumnProps, - UseColumnCellPropsColumnOption { + UseSortByColumnProps { getSortByToggleProps: (props?: Partial) => TableSortByToggleProps; } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx index 523ea36301a4..df407edd6ed5 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/TableChart.tsx @@ -17,7 +17,7 @@ * under the License. */ import React, { useState, useMemo, useCallback } from 'react'; -import { ColumnInstance, Column, DefaultSortTypes } from 'react-table'; +import { ColumnInstance, DefaultSortTypes, ColumnWithLooseAccessor } from 'react-table'; import { extent as d3Extent, max as d3Max } from 'd3-array'; import { FaSort, FaSortUp as FaSortAsc, FaSortDown as FaSortDesc } from 'react-icons/fa'; import { t } from '@superset-ui/translation'; @@ -83,7 +83,7 @@ function cellBar({ ); } -function SortIcon({ column }: { column: ColumnInstance }) { +function SortIcon({ column }: { column: ColumnInstance }) { const { isSorted, isSortedDesc } = column; let sortIcon = ; if (isSorted) { @@ -173,53 +173,66 @@ export default function TableChart( ); const getColumnConfigs = useCallback( - (column: DataColumnMeta, i: number): Column => { + (column: DataColumnMeta, i: number): ColumnWithLooseAccessor => { const { key, label, dataType } = column; + let className = ''; + if (dataType === DataType.Number) { + className += ' dt-metric'; + } else if (emitFilter) { + className += ' dt-is-filter'; + } const valueRange = showCellBars && getValueRange(key); - const cellProps: Column['cellProps'] = ({ value: value_ }, sharedCellProps) => { - let className = ''; - const value = value_ as DataRecordValue; - if (dataType === DataType.Number) { - className += ' dt-metric'; - } else if (emitFilter) { - className += ' dt-is-filter'; - if (isActiveFilterValue(key, value)) { - className += ' dt-is-active-filter'; - } - } - const [isHtml, text] = formatValue(column, value); - const style = { - ...sharedCellProps.style, - background: valueRange - ? cellBar({ - value: value as number, - valueRange, - alignPositiveNegative, - colorPositiveNegative, - }) - : undefined, - }; - return { - // show raw number in title in case of numeric values - title: typeof value === 'number' ? String(value) : undefined, - dangerouslySetInnerHTML: isHtml ? { __html: text } : undefined, - cellContent: text, - onClick: emitFilter && !valueRange ? () => toggleFilter(key, value) : undefined, - className, - style, - }; - }; return { id: String(i), // to allow duplicate column keys // must use custom accessor to allow `.` in column names // typing is incorrect in current version of `@types/react-table` // so we ask TS not to check. accessor: ((datum: D) => datum[key]) as never, - Header: label, - SortIcon, + Cell: ({ column: col, value }: { column: ColumnInstance; value: DataRecordValue }) => { + const [isHtml, text] = formatValue(column, value); + const style = { + background: valueRange + ? cellBar({ + value: value as number, + valueRange, + alignPositiveNegative, + colorPositiveNegative, + }) + : undefined, + }; + const html = isHtml ? { __html: text } : undefined; + const cellProps = { + // show raw number in title in case of numeric values + title: typeof value === 'number' ? String(value) : undefined, + onClick: emitFilter && !valueRange ? () => toggleFilter(key, value) : undefined, + className: `${className}${ + isActiveFilterValue(key, value) ? ' dt-is-active-filter' : '' + }`, + style, + }; + if (html) { + // eslint-disable-next-line react/no-danger + return ; + } + // If cellProps renderes textContent already, then we don't have to + // render `Cell`. This saves some time for large tables. + return {text}; + }, + Header: ({ column: col, title, onClick, style }) => { + return ( + + {label} + + + ); + }, sortDescFirst: sortDesc, sortType: getSortTypeByDataType(dataType), - cellProps, }; }, [