diff --git a/UNRELEASED.md b/UNRELEASED.md index 7b1689eb5c2..c4d8442e9cd 100644 --- 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 `` 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 index 57be8287fff..9d5f8439206 100644 --- 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([]); + const stickyTableHeadings = useRef([]); + const stickyHeaderWrapperElement = useRef(null); + const stickyHeaderCheckboxElement = useRef(null); const stickyHeaderElement = useRef(null); const scrollBarElement = useRef(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 > -
+
{renderCheckboxContent()}
@@ -362,7 +439,10 @@ function IndexTableBase({
) : ( -
+
{loadingMarkup}
{stickyColumnHeader} @@ -386,7 +466,7 @@ function IndexTableBase({
); - const scrollBarclassNames = classNames( + const scrollBarWrapperClassNames = classNames( styles.ScrollBarContainer, condensed && styles.scrollBarContainerCondensed, ); @@ -398,7 +478,7 @@ function IndexTableBase({ const scrollBarMarkup = itemCount > 0 ? ( -
+
{headingContent}
@@ -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 index fd2221b7515..421240e39d8 100644 --- 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('', () => { headings: mockTableHeadings, }; + beforeEach(() => { + jest.resetAllMocks(); + (getTableHeadingsBySelector as jest.Mock).mockReturnValue([]); + }); + it('renders an if no items are passed', () => { const index = mountWithApp(); @@ -157,6 +169,31 @@ describe('', () => { }); }); + 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( + + {mockTableItems.map(mockRenderRow)} + , + ); + + 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 new file mode 100644 index 00000000000..6d380c2327e --- /dev/null +++ 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 new file mode 100644 index 00000000000..5e6c26e6e3f --- /dev/null +++ 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 new file mode 100644 index 00000000000..f26671a78a7 --- /dev/null +++ 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(selector)) + : []; +} diff --git a/src/utilities/sticky-manager/sticky-manager.ts b/src/utilities/sticky-manager/sticky-manager.ts index 786e4dd50e6..718e0b98e59 100644 --- 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) {