This is an automated email from the ASF dual-hosted git repository. arivero pushed a commit to branch table-time-comparison in repository https://gitbox.apache.org/repos/asf/superset.git
commit 82fe8f5223e313d738f869b38fbd4927881936c2 Author: Antonio Rivero <[email protected]> AuthorDate: Mon Mar 4 18:12:57 2024 +0100 Table with Time Comparison: - Add colums separators to better identify comparison groups --- .../plugin-chart-table/src/DataTable/DataTable.tsx | 41 +---------- .../plugins/plugin-chart-table/src/Styles.tsx | 8 +++ .../plugins/plugin-chart-table/src/TableChart.tsx | 79 ++++++++++++++++++++-- 3 files changed, 83 insertions(+), 45 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index a0af54eb6a..79ab44981e 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -67,7 +67,7 @@ export interface DataTableProps<D extends object> extends TableOptions<D> { rowCount: number; wrapperRef?: MutableRefObject<HTMLDivElement>; onColumnOrderChange: () => void; - groupHeaderColumns?: Record<string, number[]>; + renderGroupingHeaders?: () => JSX.Element; } export interface RenderHTMLCellProps extends HTMLProps<HTMLTableCellElement> { @@ -100,7 +100,7 @@ export default typedMemo(function DataTable<D extends object>({ serverPagination, wrapperRef: userWrapperRef, onColumnOrderChange, - groupHeaderColumns, + renderGroupingHeaders, ...moreUseTableOptions }: DataTableProps<D>): JSX.Element { const tableHooks: PluginHook<D>[] = [ @@ -250,46 +250,11 @@ export default typedMemo(function DataTable<D extends object>({ e.preventDefault(); }; - const renderDynamicHeaders = () => { - // TODO: Make use of ColumnGroup to render the aditional headers - const headers: any = []; - let currentColumnIndex = 0; - - Object.entries(groupHeaderColumns || {}).forEach(([key, value], index) => { - // Calculate the number of placeholder columns needed before the current header - const startPosition = value[0]; - const colSpan = value.length; - - // Add placeholder <th> for columns before this header - for (let i = currentColumnIndex; i < startPosition; i += 1) { - headers.push( - <th - key={`placeholder-${i}`} - style={{ borderBottom: 0 }} - aria-label={`Header-${i}`} - />, - ); - } - - // Add the current header <th> - headers.push( - <th key={`header-${key}`} colSpan={colSpan} style={{ borderBottom: 0 }}> - {key} - </th>, - ); - - // Update the current column index - currentColumnIndex = startPosition + colSpan; - }); - - return headers; - }; - const renderTable = () => ( <table {...getTableProps({ className: tableClassName })}> <thead> {/* Render dynamic headers based on resultMap */} - {groupHeaderColumns ? <tr>{renderDynamicHeaders()}</tr> : null} + {renderGroupingHeaders ? renderGroupingHeaders() : null} {headerGroups.map(headerGroup => { const { key: headerGroupKey, ...headerGroupProps } = headerGroup.getHeaderGroupProps(); diff --git a/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx b/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx index 9219b6f003..ba03a83614 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/Styles.tsx @@ -111,5 +111,13 @@ export default styled.div` text-align: center; padding: 1em 0.6em; } + + .right-border-only { + border-right: 2px solid ${theme.colors.grayscale.light2}; + } + + table .right-border-only:last-child { + border-right: none; + } `} `; diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx index 0fca8cfd79..a41001a446 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -48,6 +48,7 @@ import { css, t, tn, + useTheme, } from '@superset-ui/core'; import { isEmpty } from 'lodash'; @@ -251,6 +252,7 @@ export default function TableChart<D extends DataRecord = DataRecord>( }); // keep track of whether column order changed, so that column widths can too const [columnOrderToggle, setColumnOrderToggle] = useState(false); + const theme = useTheme(); // only take relevant page size options const pageSizeOptions = useMemo(() => { @@ -446,6 +448,62 @@ export default function TableChart<D extends DataRecord = DataRecord>( return resultMap; }; + const renderGroupingHeaders = (): JSX.Element => { + // TODO: Make use of ColumnGroup to render the aditional headers + const headers: any = []; + let currentColumnIndex = 0; + + Object.entries(groupHeaderColumns || {}).forEach(([key, value]) => { + // Calculate the number of placeholder columns needed before the current header + const startPosition = value[0]; + const colSpan = value.length; + + // Add placeholder <th> for columns before this header + for (let i = currentColumnIndex; i < startPosition; i += 1) { + headers.push( + <th + key={`placeholder-${i}`} + style={{ borderBottom: 0 }} + aria-label={`Header-${i}`} + />, + ); + } + + // Add the current header <th> + headers.push( + <th key={`header-${key}`} colSpan={colSpan} style={{ borderBottom: 0 }}> + {key} + </th>, + ); + + // Update the current column index + currentColumnIndex = startPosition + colSpan; + }); + + return ( + <tr + css={css` + th { + border-right: 2px solid ${theme.colors.grayscale.light2}; + } + th:first-child { + border-left: none; + } + th:last-child { + border-right: none; + } + `} + > + {headers} + </tr> + ); + }; + + const groupHeaderColumns = useMemo( + () => getHeaderColumns(columnsMeta, enableTimeComparison), + [columnsMeta, enableTimeComparison], + ); + const getColumnConfigs = useCallback( (column: DataColumnMeta, i: number): ColumnWithLooseAccessor<D> => { const { @@ -493,6 +551,18 @@ export default function TableChart<D extends DataRecord = DataRecord>( className += ' dt-is-filter'; } + if (enableTimeComparison) { + if (!isMetric && !isPercentMetric) { + className += ' right-border-only'; + } else if (comparisonLabels.includes(label)) { + const groupinHeader = key.substring(label.length); + const columnsUnderHeader = groupHeaderColumns[groupinHeader] || []; + if (i === columnsUnderHeader[columnsUnderHeader.length - 1]) { + className += ' right-border-only'; + } + } + } + return { id: String(i), // to allow duplicate column keys // must use custom accessor to allow `.` in column names @@ -704,11 +774,6 @@ export default function TableChart<D extends DataRecord = DataRecord>( [columnsMeta, getColumnConfigs], ); - const groupHeaderColumns = useMemo( - () => getHeaderColumns(columnsMeta, enableTimeComparison), - [columnsMeta, enableTimeComparison], - ); - const handleServerPaginationChange = useCallback( (pageNumber: number, pageSize: number) => { updateExternalFormData(setDataMask, pageNumber, pageSize); @@ -773,8 +838,8 @@ export default function TableChart<D extends DataRecord = DataRecord>( selectPageSize={pageSize !== null && SelectPageSize} // not in use in Superset, but needed for unit tests sticky={sticky} - groupHeaderColumns={ - !isEmpty(groupHeaderColumns) ? groupHeaderColumns : undefined + renderGroupingHeaders={ + !isEmpty(groupHeaderColumns) ? renderGroupingHeaders : undefined } /> </Styles>
