You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ar...@apache.org on 2024/03/08 00:44:15 UTC

(superset) 05/09: Table with Time Comparison:

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 <an...@gmail.com>
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>