[IndexTable] Fix UX / jank when resizing (#4033)

* Fix indextable UX / jank when resizing
Shopify · Mar 15, 2021 · 58a8a32 · 58a8a32
1 parent b59d092
commit 58a8a32
Show file tree

Hide file tree

Showing 7 changed files with 186 additions and 34 deletions.
diff --git a/UNRELEASED.md b/UNRELEASED.md
@@ -15,6 +15,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 - `IndexTable` Remove parent resource name from bulk select action ([#4013](https://github.com/Shopify/polaris-react/pull/4013))
 - Ensured `@charset` declaration is the first thing in our styles.css file ([#4019](https://github.com/Shopify/polaris-react/pull/4019))
 - Fix `Modal.Section` divider color to match header and footer divider ([#4021](https://github.com/Shopify/polaris-react/pull/4021))
+- Fix `IndexTable` sticky header alignment and jank ([#4033](https://github.com/Shopify/polaris-react/pull/4033)
 - Remove focus ring on click for ActionList ([#4034](https://github.com/Shopify/polaris-react/pull/4034))
 - Updated `<TextField>` to use `autocomplete=nope` instead of `autocomplete=off` ([#4053](https://github.com/Shopify/polaris-react/pull/4053))
 

diff --git a/src/components/IndexTable/IndexTable.tsx b/src/components/IndexTable/IndexTable.tsx
@@ -28,6 +28,7 @@ import {AfterInitialMount} from '../AfterInitialMount';
 import {IndexProvider} from '../IndexProvider';
 import type {NonEmptyArray} from '../../types';
 
+import {getTableHeadingsBySelector} from './utilities';
 import {ScrollContainer, Cell, Row} from './components';
 import styles from './IndexTable.scss';
 
@@ -52,6 +53,9 @@ export interface TableHeadingRect {
 }
 
 const SCROLL_BAR_PADDING = 4;
+const SIXTY_FPS = 1000 / 60;
+const SCROLL_BAR_DEBOUNCE_PERIOD = 300;
+const SMALL_SCREEN_WIDTH = 458;
 
 function IndexTableBase({
   headings,
@@ -91,6 +95,10 @@ function IndexTableBase({
 
   const [isSmallScreenSelectable, setIsSmallScreenSelectable] = useState(false);
 
+  const tableHeadings = useRef<HTMLElement[]>([]);
+  const stickyTableHeadings = useRef<HTMLElement[]>([]);
+  const stickyHeaderWrapperElement = useRef<HTMLDivElement>(null);
+  const stickyHeaderCheckboxElement = useRef<HTMLDivElement>(null);
   const stickyHeaderElement = useRef<HTMLDivElement>(null);
   const scrollBarElement = useRef<HTMLDivElement>(null);
   const scrollingWithBar = useRef(false);
@@ -118,21 +126,64 @@ function IndexTableBase({
     );
   }, [handleSelectionChange, selectedItemsCount]);
 
-  const resizeTableHeadings = useCallback(() => {
-    if (!tableElement.current || !scrollableContainerElement.current) {
-      return;
-    }
+  const calculateFirstHeaderOffset = useCallback(() => {
+    return condensed
+      ? tableHeadingRects.current[0].offsetWidth
+      : tableHeadingRects.current[0].offsetWidth +
+          tableHeadingRects.current[1].offsetWidth;
+  }, [condensed]);
 
-    const measuredTableHeadingRects = Array.from(
-      tableElement.current.querySelectorAll('[data-index-table-heading]'),
-    ).map((heading) => ({
-      offsetWidth: heading instanceof HTMLElement ? heading.offsetWidth : 0,
-      offsetLeft: heading instanceof HTMLElement ? heading.offsetLeft : 0,
-    }));
-    const boundingRect = scrollableContainerElement.current.getBoundingClientRect();
-    tablePosition.current = {top: boundingRect.top, left: boundingRect.left};
-    tableHeadingRects.current = measuredTableHeadingRects;
-  }, []);
+  const resizeTableHeadings = useMemo(
+    () =>
+      debounce(
+        () => {
+          if (!tableElement.current || !scrollableContainerElement.current) {
+            return;
+          }
+
+          const boundingRect = scrollableContainerElement.current.getBoundingClientRect();
+          tablePosition.current = {
+            top: boundingRect.top,
+            left: boundingRect.left,
+          };
+
+          tableHeadingRects.current = tableHeadings.current.map((heading) => ({
+            offsetWidth: heading.offsetWidth || 0,
+            offsetLeft: heading.offsetLeft || 0,
+          }));
+
+          if (tableHeadings.current.length === 0) {
+            return;
+          }
+
+          // update left offset for first column
+          if (tableHeadings.current.length > 1)
+            tableHeadings.current[1].style.left = `${tableHeadingRects.current[0].offsetWidth}px`;
+
+          // update the min width of the checkbox to be the be the un-padded width of the first heading
+          if (stickyHeaderCheckboxElement?.current) {
+            const elementStyle = getComputedStyle(tableHeadings.current[0]);
+            const boxWidth = tableHeadings.current[0].offsetWidth;
+            stickyHeaderCheckboxElement.current.style.minWidth = `calc(${boxWidth}px - ${elementStyle.paddingLeft} - ${elementStyle.paddingRight} + 2px)`;
+          }
+
+          // update sticky header min-widths
+          stickyTableHeadings.current.forEach((heading, index) => {
+            let minWidth = 0;
+            if (index === 0 && !isSmallScreen()) {
+              minWidth = calculateFirstHeaderOffset();
+            } else if (tableHeadingRects.current.length > index) {
+              minWidth = tableHeadingRects.current[index].offsetWidth;
+            }
+
+            heading.style.minWidth = `${minWidth}px`;
+          });
+        },
+        SIXTY_FPS,
+        {leading: true, trailing: true, maxWait: SIXTY_FPS},
+      ),
+    [calculateFirstHeaderOffset],
+  );
 
   const resizeTableScrollBar = useCallback(() => {
     if (scrollBarElement.current && tableElement.current && tableInitialized) {
@@ -143,15 +194,25 @@ function IndexTableBase({
     }
   }, [tableInitialized]);
 
-  const handleResize = useMemo(
-    () =>
-      debounce(() => {
-        resizeTableHeadings();
-        resizeTableScrollBar();
-      }, 50),
-    [resizeTableHeadings, resizeTableScrollBar],
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  const debounceResizeTableScrollbar = useCallback(
+    debounce(resizeTableScrollBar, SCROLL_BAR_DEBOUNCE_PERIOD, {
+      trailing: true,
+    }),
+    [resizeTableScrollBar],
   );
 
+  const handleResize = useCallback(() => {
+    // hide the scrollbar when resizing
+    scrollBarElement.current?.style.setProperty(
+      '--p-scroll-bar-content-width',
+      `0px`,
+    );
+
+    resizeTableHeadings();
+    debounceResizeTableScrollbar();
+  }, [debounceResizeTableScrollbar, resizeTableHeadings]);
+
   const handleScrollContainerScroll = useCallback(
     (canScrollLeft, canScrollRight) => {
       if (!scrollableContainerElement.current || !scrollBarElement.current) {
@@ -197,7 +258,22 @@ function IndexTableBase({
     scrollingContainer.current = false;
   }, []);
 
-  useEffect(() => resizeTableHeadings(), [headings, resizeTableHeadings]);
+  useEffect(() => {
+    tableHeadings.current = getTableHeadingsBySelector(
+      tableElement.current,
+      '[data-index-table-heading]',
+    );
+    stickyTableHeadings.current = getTableHeadingsBySelector(
+      stickyHeaderWrapperElement.current,
+      '[data-index-table-sticky-heading]',
+    );
+    resizeTableHeadings();
+  }, [
+    headings,
+    resizeTableHeadings,
+    stickyHeaderCheckboxElement,
+    tableInitialized,
+  ]);
 
   useEffect(() => {
     resizeTableScrollBar();
@@ -225,10 +301,7 @@ function IndexTableBase({
   const stickyColumnHeaderStyle =
     tableHeadingRects.current && tableHeadingRects.current.length > 0
       ? {
-          minWidth: condensed
-            ? tableHeadingRects.current[0].offsetWidth
-            : tableHeadingRects.current[0].offsetWidth +
-              tableHeadingRects.current[1].offsetWidth,
+          minWidth: calculateFirstHeaderOffset(),
         }
       : undefined;
 
@@ -237,9 +310,13 @@ function IndexTableBase({
       className={styles.TableHeading}
       key={headings[0].title}
       style={stickyColumnHeaderStyle}
+      data-index-table-sticky-heading
     >
       <Stack spacing="none" wrap={false} alignment="center">
-        <div className={styles.StickyColumnHeaderCheckbox}>
+        <div
+          className={styles.StickyColumnHeaderCheckbox}
+          ref={stickyHeaderCheckboxElement}
+        >
           {renderCheckboxContent()}
         </div>
         <div className={styles['StickyTableHeading-second-scrolling']}>
@@ -362,7 +439,10 @@ function IndexTableBase({
               </Button>
             </div>
           ) : (
-            <div className={stickyHeaderClassNames}>
+            <div
+              className={stickyHeaderClassNames}
+              ref={stickyHeaderWrapperElement}
+            >
               {loadingMarkup}
               <div className={stickyColumnHeaderClassNames}>
                 {stickyColumnHeader}
@@ -386,7 +466,7 @@ function IndexTableBase({
     </div>
   );
 
-  const scrollBarclassNames = classNames(
+  const scrollBarWrapperClassNames = classNames(
     styles.ScrollBarContainer,
     condensed && styles.scrollBarContainerCondensed,
   );
@@ -398,7 +478,7 @@ function IndexTableBase({
   const scrollBarMarkup =
     itemCount > 0 ? (
       <AfterInitialMount>
-        <div className={scrollBarclassNames}>
+        <div className={scrollBarWrapperClassNames}>
           <div
             onScroll={handleScrollBarScroll}
             className={styles.ScrollBar}
@@ -580,6 +660,7 @@ function IndexTableBase({
         className={stickyHeadingClassName}
         key={heading.title}
         style={headingStyle}
+        data-index-table-sticky-heading
       >
         {headingContent}
       </div>
@@ -611,6 +692,12 @@ function IndexTableBase({
   }
 }
 
+const isSmallScreen = () => {
+  return typeof window === 'undefined'
+    ? false
+    : window.innerWidth < SMALL_SCREEN_WIDTH;
+};
+
 export interface IndexTableProps
   extends IndexTableBaseProps,
     IndexProviderProps {}

diff --git a/src/components/IndexTable/tests/IndexTable.test.tsx b/src/components/IndexTable/tests/IndexTable.test.tsx
@@ -3,7 +3,9 @@ import React from 'react';
 import {mountWithAppProvider} from 'test-utilities/legacy';
 import {mountWithApp} from 'test-utilities';
 import {Sticky} from 'components/Sticky';
+import {EventListener} from 'components/EventListener';
 
+import {getTableHeadingsBySelector} from '../utilities';
 import {EmptySearchResult} from '../../EmptySearchResult';
 import {Spinner} from '../../Spinner';
 import {Button} from '../../Button';
@@ -16,6 +18,11 @@ import {ScrollContainer} from '../components';
 import {SelectionType} from '../../../utilities/index-provider';
 import {AfterInitialMount} from '../../AfterInitialMount';
 
+jest.mock('../utilities', () => ({
+  ...jest.requireActual('../utilities'),
+  getTableHeadingsBySelector: jest.fn(),
+}));
+
 const mockTableItems = [
   {
     id: 'item-1',
@@ -64,6 +71,11 @@ describe('<IndexTable>', () => {
     headings: mockTableHeadings,
   };
 
+  beforeEach(() => {
+    jest.resetAllMocks();
+    (getTableHeadingsBySelector as jest.Mock).mockReturnValue([]);
+  });
+
   it('renders an <EmptySearchResult /> if no items are passed', () => {
     const index = mountWithApp(<IndexTable {...defaultProps} itemCount={0} />);
 
@@ -157,6 +169,31 @@ describe('<IndexTable>', () => {
     });
   });
 
+  describe('resize', function () {
+    it('does not update columns if no headings are present', () => {
+      const spy = jest.spyOn(window, 'getComputedStyle');
+
+      const headings: IndexTableProps['headings'] = [
+        {title: 'Heading one'},
+        {title: 'Heading two'},
+      ];
+
+      const index = mountWithApp(
+        <IndexTable
+          {...defaultProps}
+          itemCount={mockTableItems.length}
+          headings={headings}
+        >
+          {mockTableItems.map(mockRenderRow)}
+        </IndexTable>,
+      );
+
+      index.find(EventListener)!.trigger('handler');
+
+      expect(spy).not.toHaveBeenCalled();
+    });
+  });
+
   describe('scroll bar', () => {
     it('sets scrollleft on scroll', () => {
       const updatedScrollLeft = 25;

diff --git a/src/components/IndexTable/utilities/index.ts b/src/components/IndexTable/utilities/index.ts
@@ -0,0 +1 @@
+export {getTableHeadingsBySelector} from './utilities';
diff --git a/src/components/IndexTable/utilities/tests/utilities.test.ts b/src/components/IndexTable/utilities/tests/utilities.test.ts
@@ -0,0 +1,16 @@
+import {getTableHeadingsBySelector} from '../utilities';
+
+describe('#getTableHeadingsBySelector', function () {
+  it('returns empty array when no wrapper is passed', () => {
+    expect(getTableHeadingsBySelector(null, '')).toStrictEqual([]);
+  });
+
+  it('returns array when wrapper is passed', () => {
+    const response = [{id: 'test'}];
+    const fakeElement = ({
+      querySelectorAll: () => response,
+    } as unknown) as HTMLElement;
+
+    expect(getTableHeadingsBySelector(fakeElement, '')).toStrictEqual(response);
+  });
+});
diff --git a/src/components/IndexTable/utilities/utilities.ts b/src/components/IndexTable/utilities/utilities.ts
@@ -0,0 +1,8 @@
+export function getTableHeadingsBySelector(
+  wrapperElement: HTMLElement | null,
+  selector: string,
+) {
+  return wrapperElement
+    ? Array.from(wrapperElement.querySelectorAll<HTMLElement>(selector))
+    : [];
+}
diff --git a/src/utilities/sticky-manager/sticky-manager.ts b/src/utilities/sticky-manager/sticky-manager.ts
@@ -25,6 +25,8 @@ interface StickyItem {
   ): void;
 }
 
+const SIXTY_FPS = 1000 / 60;
+
 export class StickyManager {
   private stickyItems: StickyItem[] = [];
   private stuckItems: StickyItem[] = [];
@@ -35,16 +37,16 @@ export class StickyManager {
     () => {
       this.manageStickyItems();
     },
-    40,
-    {leading: true, trailing: true, maxWait: 40},
+    SIXTY_FPS,
+    {leading: true, trailing: true, maxWait: SIXTY_FPS},
   );
 
   private handleScroll = debounce(
     () => {
       this.manageStickyItems();
     },
-    40,
-    {leading: true, trailing: true, maxWait: 40},
+    SIXTY_FPS,
+    {leading: true, trailing: true, maxWait: SIXTY_FPS},
   );
 
   constructor(container?: Document | HTMLElement) {