From 5d04f7dbce8b1dc3daacc197a18905816df0db7f Mon Sep 17 00:00:00 2001
From: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Date: Sat, 9 Jan 2021 17:04:53 +0100
Subject: [PATCH] fix: Data tables styling issues on Explore view (#12383)

* Fix sort icons wrapping

* Fix large table stretching chart section container

* Implement sticky paginattion

* Display "loading" when data is loading

* Lint fix
---
 .../src/components/TableView/TableView.tsx    | 36 +++++++++++----
 .../dataViewCommon/TableCollection.tsx        |  4 ++
 .../explore/components/DataTableControl.tsx   | 14 +++++-
 .../src/explore/components/DataTablesPane.tsx |  4 +-
 .../explore/components/ExploreChartPanel.jsx  | 46 ++++++++++---------
 .../components/ExploreViewContainer.jsx       |  2 +
 .../src/explore/components/RowCountLabel.jsx  | 10 ++--
 7 files changed, 78 insertions(+), 38 deletions(-)

diff --git a/superset-frontend/src/components/TableView/TableView.tsx b/superset-frontend/src/components/TableView/TableView.tsx
index 58b02bd33..10dc4b29d 100644
--- a/superset-frontend/src/components/TableView/TableView.tsx
+++ b/superset-frontend/src/components/TableView/TableView.tsx
@@ -41,13 +41,17 @@ export interface TableViewProps {
   emptyWrapperType?: EmptyWrapperType;
   noDataText?: string;
   className?: string;
+  isPaginationSticky?: boolean;
+  showRowCount?: boolean;
 }
 
 const EmptyWrapper = styled.div`
   margin: ${({ theme }) => theme.gridUnit * 40}px 0;
 `;
 
-const TableViewStyles = styled.div`
+const TableViewStyles = styled.div<{
+  isPaginationSticky?: boolean;
+}>`
   .table-cell.table-cell {
     vertical-align: top;
   }
@@ -57,6 +61,15 @@ const TableViewStyles = styled.div`
     flex-direction: column;
     justify-content: center;
     align-items: center;
+    background-color: ${({ theme }) => theme.colors.grayscale.light5};
+
+    ${({ theme, isPaginationSticky }) =>
+      isPaginationSticky &&
+      `
+        position: sticky;
+        bottom: ${theme.gridUnit * 4}px;
+        left: 0;
+    `};
   }
 
   .row-count-container {
@@ -75,6 +88,7 @@ const TableView = ({
   withPagination = true,
   emptyWrapperType = EmptyWrapperType.Default,
   noDataText,
+  showRowCount = true,
   ...props
 }: TableViewProps) => {
   const initialState = {
@@ -151,15 +165,17 @@ const TableView = ({
             onChange={(p: number) => gotoPage(p - 1)}
             hideFirstAndLastPageLinks
           />
-          <div className="row-count-container">
-            {!loading &&
-              t(
-                '%s-%s of %s',
-                pageSize * pageIndex + (page.length && 1),
-                pageSize * pageIndex + page.length,
-                data.length,
-              )}
-          </div>
+          {showRowCount && (
+            <div className="row-count-container">
+              {!loading &&
+                t(
+                  '%s-%s of %s',
+                  pageSize * pageIndex + (page.length && 1),
+                  pageSize * pageIndex + page.length,
+                  data.length,
+                )}
+            </div>
+          )}
         </div>
       )}
     </TableViewStyles>
diff --git a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
index 1c11f811c..e1550044b 100644
--- a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
+++ b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
@@ -75,6 +75,10 @@ export const Table = styled.table`
       min-width: 200px;
     }
 
+    span {
+      white-space: nowrap;
+    }
+
     svg {
       display: inline-block;
       top: 6px;
diff --git a/superset-frontend/src/explore/components/DataTableControl.tsx b/superset-frontend/src/explore/components/DataTableControl.tsx
index 30ffbd43c..a8a598167 100644
--- a/superset-frontend/src/explore/components/DataTableControl.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl.tsx
@@ -80,8 +80,18 @@ export const FilterInput = ({
   );
 };
 
-export const RowCount = ({ data }: { data?: Record<string, any>[] }) => (
-  <RowCountLabel rowcount={data?.length ?? 0} suffix={t('rows retrieved')} />
+export const RowCount = ({
+  data,
+  loading,
+}: {
+  data?: Record<string, any>[];
+  loading: boolean;
+}) => (
+  <RowCountLabel
+    rowcount={data?.length ?? 0}
+    loading={loading}
+    suffix={t('rows retrieved')}
+  />
 );
 
 export const useFilteredTableData = (
diff --git a/superset-frontend/src/explore/components/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane.tsx
index a2c5f2cb9..a5bddda91 100644
--- a/superset-frontend/src/explore/components/DataTablesPane.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane.tsx
@@ -201,6 +201,8 @@ export const DataTablesPane = ({
           noDataText={t('No data')}
           emptyWrapperType={EmptyWrapperType.Small}
           className="table-condensed"
+          isPaginationSticky
+          showRowCount={false}
         />
       );
     }
@@ -209,7 +211,7 @@ export const DataTablesPane = ({
 
   const TableControls = (
     <TableControlsWrapper>
-      <RowCount data={data[activeTabKey]} />
+      <RowCount data={data[activeTabKey]} loading={isLoading[activeTabKey]} />
       <CopyToClipboardButton data={data[activeTabKey]} />
       <FilterInput onChangeHandler={setFilterText} />
     </TableControlsWrapper>
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index 8e99ec940..845baa619 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useState, useEffect, useRef } from 'react';
+import React, { useState, useEffect, useRef, useCallback } from 'react';
 import PropTypes from 'prop-types';
 import Split from 'react-split';
 import { ParentSize } from '@vx/responsive';
@@ -105,34 +105,38 @@ const ExploreChartPanel = props => {
   const gutterHeight = theme.gridUnit * GUTTER_SIZE_FACTOR;
 
   const panelHeadingRef = useRef(null);
-  const [headerHeight, setHeaderHeight] = useState(props.standalone ? 0 : 50);
   const [splitSizes, setSplitSizes] = useState(INITIAL_SIZES);
 
-  const calcSectionHeight = percent => {
-    const containerHeight = parseInt(props.height, 10) - headerHeight - 30;
-    return (
-      (containerHeight * percent) / 100 - (gutterHeight / 2 + gutterMargin)
-    );
-  };
+  const calcSectionHeight = useCallback(
+    percent => {
+      const headerHeight = props.standalone
+        ? 0
+        : panelHeadingRef?.current?.offsetHeight ?? 50;
+      const containerHeight = parseInt(props.height, 10) - headerHeight;
+      return (
+        (containerHeight * percent) / 100 - (gutterHeight / 2 + gutterMargin)
+      );
+    },
+    [gutterHeight, gutterMargin, props.height, props.standalone],
+  );
 
   const [tableSectionHeight, setTableSectionHeight] = useState(
     calcSectionHeight(INITIAL_SIZES[1]),
   );
 
-  useEffect(() => {
-    const calcHeaderSize = debounce(() => {
-      setHeaderHeight(
-        props.standalone ? 0 : panelHeadingRef?.current?.offsetHeight,
-      );
-    }, 100);
-    calcHeaderSize();
-    window.addEventListener('resize', calcHeaderSize);
-    return () => window.removeEventListener('resize', calcHeaderSize);
-  }, [props.standalone]);
+  const recalcPanelSizes = useCallback(
+    ([, southPercent]) => {
+      setTableSectionHeight(calcSectionHeight(southPercent));
+    },
+    [calcSectionHeight],
+  );
 
-  const recalcPanelSizes = ([, southPercent]) => {
-    setTableSectionHeight(calcSectionHeight(southPercent));
-  };
+  useEffect(() => {
+    const recalcSizes = debounce(() => recalcPanelSizes(splitSizes), 200);
+
+    window.addEventListener('resize', recalcSizes);
+    return () => window.removeEventListener('resize', recalcSizes);
+  }, [props.standalone, recalcPanelSizes, splitSizes]);
 
   const onDragEnd = sizes => {
     setSplitSizes(sizes);
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
index 62d9d370d..c5390563f 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
@@ -93,6 +93,8 @@ const Styles = styled.div`
     border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
   }
   .main-explore-content {
+    flex: 1;
+    min-width: ${({ theme }) => theme.gridUnit * 128}px;
     border-left: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
   }
   .controls-column {
diff --git a/superset-frontend/src/explore/components/RowCountLabel.jsx b/superset-frontend/src/explore/components/RowCountLabel.jsx
index 201dc4d5f..8fce6f804 100644
--- a/superset-frontend/src/explore/components/RowCountLabel.jsx
+++ b/superset-frontend/src/explore/components/RowCountLabel.jsx
@@ -28,26 +28,28 @@ const propTypes = {
   limit: PropTypes.number,
   rows: PropTypes.string,
   suffix: PropTypes.string,
+  loading: PropTypes.bool,
 };
 
 const defaultProps = {
   suffix: t('rows'),
 };
 
-export default function RowCountLabel({ rowcount, limit, suffix }) {
+export default function RowCountLabel({ rowcount, limit, suffix, loading }) {
   const limitReached = rowcount === limit;
-  const bsStyle = limitReached || rowcount === 0 ? 'danger' : 'default';
+  const bsStyle =
+    limitReached || (rowcount === 0 && !loading) ? 'danger' : 'default';
   const formattedRowCount = getNumberFormatter()(rowcount);
   const tooltip = (
     <span>
       {limitReached && <div>{t('Limit reached')}</div>}
-      {rowcount}
+      {loading ? 'Loading' : rowcount}
     </span>
   );
   return (
     <TooltipWrapper label="tt-rowcount" tooltip={tooltip}>
       <Label bsStyle={bsStyle}>
-        {formattedRowCount} {suffix}
+        {loading ? 'Loading...' : `${formattedRowCount} ${suffix}`}
       </Label>
     </TooltipWrapper>
   );