From 569a43e6e33dfb3781a54a222b58fb22623363ba Mon Sep 17 00:00:00 2001 From: SaidMarar Date: Sun, 23 Jul 2023 14:02:36 +0200 Subject: [PATCH] [Tabs] Fix and improve visibility of tab scroll buttons using the IntersectionObserver API (#36071) Signed-off-by: SaidMarar Co-authored-by: ZeeshanTamboli --- packages/mui-material/src/Tabs/Tabs.js | 96 ++++---- packages/mui-material/src/Tabs/Tabs.test.js | 239 ++++++++++---------- test/utils/createRenderer.tsx | 7 + 3 files changed, 189 insertions(+), 153 deletions(-) diff --git a/packages/mui-material/src/Tabs/Tabs.js b/packages/mui-material/src/Tabs/Tabs.js index 44d677851ffbbc..c68f8518422260 100644 --- a/packages/mui-material/src/Tabs/Tabs.js +++ b/packages/mui-material/src/Tabs/Tabs.js @@ -312,10 +312,9 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { const [mounted, setMounted] = React.useState(false); const [indicatorStyle, setIndicatorStyle] = React.useState(defaultIndicatorStyle); - const [displayScroll, setDisplayScroll] = React.useState({ - start: false, - end: false, - }); + const [displayStartScroll, setDisplayStartScroll] = React.useState(false); + const [displayEndScroll, setDisplayEndScroll] = React.useState(false); + const [updateScrollObserver, setUpdateScrollObserver] = React.useState(false); const [scrollerStyle, setScrollerStyle] = React.useState({ overflow: 'hidden', @@ -508,7 +507,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { /> ) : null; - const scrollButtonsActive = displayScroll.start || displayScroll.end; + const scrollButtonsActive = displayStartScroll || displayEndScroll; const showScrollButtons = scrollable && ((scrollButtons === 'auto' && scrollButtonsActive) || scrollButtons === true); @@ -519,7 +518,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { orientation={orientation} direction={isRtl ? 'right' : 'left'} onClick={handleStartScrollClick} - disabled={!displayScroll.start} + disabled={!displayStartScroll} {...TabScrollButtonProps} className={clsx(classes.scrollButtons, TabScrollButtonProps.className)} /> @@ -534,7 +533,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { orientation={orientation} direction={isRtl ? 'left' : 'right'} onClick={handleEndScrollClick} - disabled={!displayScroll.end} + disabled={!displayEndScroll} {...TabScrollButtonProps} className={clsx(classes.scrollButtons, TabScrollButtonProps.className)} /> @@ -563,23 +562,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { const updateScrollButtonState = useEventCallback(() => { if (scrollable && scrollButtons !== false) { - const { scrollTop, scrollHeight, clientHeight, scrollWidth, clientWidth } = tabsRef.current; - let showStartScroll; - let showEndScroll; - - if (vertical) { - showStartScroll = scrollTop > 1; - showEndScroll = scrollTop < scrollHeight - clientHeight - 1; - } else { - const scrollLeft = getNormalizedScrollLeft(tabsRef.current, theme.direction); - // use 1 for the potential rounding error with browser zooms. - showStartScroll = isRtl ? scrollLeft < scrollWidth - clientWidth - 1 : scrollLeft > 1; - showEndScroll = !isRtl ? scrollLeft < scrollWidth - clientWidth - 1 : scrollLeft > 1; - } - - if (showStartScroll !== displayScroll.start || showEndScroll !== displayScroll.end) { - setDisplayScroll({ start: showStartScroll, end: showEndScroll }); - } + setUpdateScrollObserver(!updateScrollObserver); } }); @@ -593,7 +576,6 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { // replaced by Suspense with a fallback, once React is updated to version 18 if (tabsRef.current) { updateIndicatorState(); - updateScrollButtonState(); } }); const win = ownerWindow(tabsRef.current); @@ -615,21 +597,61 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { resizeObserver.disconnect(); } }; - }, [updateIndicatorState, updateScrollButtonState]); - - const handleTabsScroll = React.useMemo( - () => - debounce(() => { - updateScrollButtonState(); - }), - [updateScrollButtonState], - ); + }, [updateIndicatorState]); + /** + * Toggle visibility of start and end scroll buttons + * Using IntersectionObserver on first and last Tabs. + */ React.useEffect(() => { + let firstObserver; + let lastObserver; + const tabListChildren = Array.from(tabListRef.current.children); + const length = tabListChildren.length; + const firstTab = tabListChildren[0]; + const lastTab = tabListChildren[length - 1]; + const threshold = 0.99; + const observerOptions = { + root: tabsRef.current, + threshold, + }; + + const handleScrollButtonStart = (entries) => { + let display = false; + entries.forEach(({ isIntersecting }) => { + display = !isIntersecting; + }); + setDisplayStartScroll(display); + }; + + const handleScrollButtonEnd = (entries) => { + let display = false; + entries.forEach(({ isIntersecting }) => { + display = !isIntersecting; + }); + setDisplayEndScroll(display); + }; + + if ( + typeof IntersectionObserver !== 'undefined' && + length > 0 && + scrollable && + scrollButtons !== false + ) { + firstObserver = new IntersectionObserver(handleScrollButtonStart, observerOptions); + firstObserver.observe(firstTab); + + if (length > 1) { + lastObserver = new IntersectionObserver(handleScrollButtonEnd, observerOptions); + lastObserver.observe(lastTab); + } + } + return () => { - handleTabsScroll.clear(); + firstObserver?.disconnect(); + lastObserver?.disconnect(); }; - }, [handleTabsScroll]); + }, [scrollable, scrollButtons, updateScrollObserver, childrenProp?.length]); React.useEffect(() => { setMounted(true); @@ -637,7 +659,6 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { React.useEffect(() => { updateIndicatorState(); - updateScrollButtonState(); }); React.useEffect(() => { @@ -763,7 +784,6 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) { : -scrollerStyle.scrollbarWidth, }} ref={tabsRef} - onScroll={handleTabsScroll} > {/* The tablist isn't interactive but the tabs are */} ', () => { }); describe('prop: variant="scrollable"', () => { - clock.withFakeTimers(); const tabs = ( @@ -509,34 +509,6 @@ describe('', () => { expect(container.querySelectorAll(selector)).to.have.lengthOf(1); }); - it('should response to scroll events', function test() { - if (isJSDOM) { - this.skip(); - } - const { container, forceUpdate, getByRole } = render(tabs); - const tablistContainer = getByRole('tablist').parentElement; - - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - tablistContainer.scrollLeft = 10; - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 216 }); - Object.defineProperty(tablistContainer, 'getBoundingClientRect', { - value: () => ({ - left: 0, - right: 50, - }), - }); - forceUpdate(); - clock.tick(1000); - expect(hasLeftScrollButton(container)).to.equal(true); - expect(hasRightScrollButton(container)).to.equal(true); - tablistContainer.scrollLeft = 0; - fireEvent.scroll(container.querySelector(`.${classes.scroller}.${classes.scrollableX}`)); - clock.tick(166); - - expect(hasLeftScrollButton(container)).to.equal(false); - expect(hasRightScrollButton(container)).to.equal(true); - }); - it('should get a scrollbar size listener', () => { const { setProps, getByRole } = render( @@ -569,8 +541,6 @@ describe('', () => { }); describe('prop: scrollButtons', () => { - clock.withFakeTimers(); - it('should render scroll buttons', () => { const { container } = render( @@ -608,65 +578,27 @@ describe('', () => { expect(container.querySelectorAll(`.${classes.scrollButtonsHideMobile}`)).to.have.lengthOf(0); }); - it('should handle window resize event', function test() { - if (isJSDOM) { - this.skip(); - } - - const { container, forceUpdate, getByRole } = render( - - - - - , - ); - - const tablistContainer = getByRole('tablist').parentElement; - - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - tablistContainer.scrollLeft = 10; - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 216 }); - Object.defineProperty(tablistContainer, 'getBoundingClientRect', { - value: () => ({ - left: 0, - right: 100, - }), - }); - forceUpdate(); - clock.tick(1000); - expect(hasLeftScrollButton(container)).to.equal(true); - expect(hasRightScrollButton(container)).to.equal(true); - tablistContainer.scrollLeft = 0; - - act(() => { - window.dispatchEvent(new window.Event('resize', {})); - }); - clock.tick(166); - - expect(hasLeftScrollButton(container)).to.equal(false); - expect(hasRightScrollButton(container)).to.equal(true); - }); - describe('scroll button visibility states', () => { - it('should set neither left nor right scroll button state', () => { - const { container, forceUpdate, getByRole } = render( + it('should set neither left nor right scroll button state', function test() { + if (isJSDOM) { + this.skip(); + } + const { container } = render( , ); - const tablistContainer = getByRole('tablist').parentElement; - - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 200 - 40 * 2 }); - forceUpdate(); expect(hasLeftScrollButton(container)).to.equal(false); expect(hasRightScrollButton(container)).to.equal(false); }); - it('should set only left scroll button state', () => { - const { container, forceUpdate, getByRole } = render( + it('should set only left scroll button state', async function test() { + if (isJSDOM) { + this.skip(); + } + const { container, getByRole } = render( @@ -675,17 +607,19 @@ describe('', () => { ); const tablistContainer = getByRole('tablist').parentElement; - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 216 }); - tablistContainer.scrollLeft = 96; + tablistContainer.scrollLeft = 240; - forceUpdate(); - expect(hasLeftScrollButton(container)).to.equal(true); - expect(hasRightScrollButton(container)).to.equal(false); + await waitFor(() => { + expect(hasLeftScrollButton(container)).to.equal(true); + expect(hasRightScrollButton(container)).to.equal(false); + }); }); - it('should set only right scroll button state', () => { - const { container, forceUpdate, getByRole } = render( + it('should set only right scroll button state', async function test() { + if (isJSDOM) { + this.skip(); + } + const { container, getByRole } = render( @@ -694,17 +628,19 @@ describe('', () => { ); const tablistContainer = getByRole('tablist').parentElement; - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 216 }); tablistContainer.scrollLeft = 0; - forceUpdate(); - expect(hasLeftScrollButton(container)).to.equal(false); - expect(hasRightScrollButton(container)).to.equal(true); + await waitFor(() => { + expect(hasLeftScrollButton(container)).to.equal(false); + expect(hasRightScrollButton(container)).to.equal(true); + }); }); - it('should set both left and right scroll button state', () => { - const { container, forceUpdate, getByRole } = render( + it('should set both left and right scroll button state', async function test() { + if (isJSDOM) { + this.skip(); + } + const { container, getByRole } = render( @@ -712,13 +648,12 @@ describe('', () => { ); const tablistContainer = getByRole('tablist').parentElement; - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 216 }); tablistContainer.scrollLeft = 5; - forceUpdate(); - expect(hasLeftScrollButton(container)).to.equal(true); - expect(hasRightScrollButton(container)).to.equal(true); + await waitFor(() => { + expect(hasLeftScrollButton(container)).to.equal(true); + expect(hasRightScrollButton(container)).to.equal(true); + }); }); }); }); @@ -726,8 +661,12 @@ describe('', () => { describe('scroll button behavior', () => { clock.withFakeTimers(); - it('should scroll visible items', () => { - const { container, forceUpdate, getByRole, getAllByRole } = render( + it('should scroll visible items', async function test() { + clock.restore(); + if (isJSDOM) { + this.skip(); + } + const { container, getByRole } = render( @@ -735,26 +674,24 @@ describe('', () => { , ); const tablistContainer = getByRole('tablist').parentElement; - const tabs = getAllByRole('tab'); - Object.defineProperty(tablistContainer, 'clientWidth', { value: 200 - 40 * 2 }); - Object.defineProperty(tabs[0], 'clientWidth', { value: 100 }); - Object.defineProperty(tabs[1], 'clientWidth', { value: 50 }); - Object.defineProperty(tabs[2], 'clientWidth', { value: 100 }); - Object.defineProperty(tablistContainer, 'scrollWidth', { value: 100 + 50 + 100 }); + tablistContainer.scrollLeft = 20; - forceUpdate(); - clock.tick(1000); - expect(hasLeftScrollButton(container)).to.equal(true); - expect(hasRightScrollButton(container)).to.equal(true); + + await waitFor(() => { + expect(hasLeftScrollButton(container)).to.equal(true); + expect(hasRightScrollButton(container)).to.equal(true); + }); fireEvent.click(findScrollButton(container, 'left')); - clock.tick(1000); - expect(tablistContainer.scrollLeft).not.to.be.above(0); + await waitFor(() => { + expect(tablistContainer.scrollLeft).not.to.be.above(0); + }); tablistContainer.scrollLeft = 0; fireEvent.click(findScrollButton(container, 'right')); - clock.tick(1000); - expect(tablistContainer.scrollLeft).equal(100); + await waitFor(() => { + expect(tablistContainer.scrollLeft).equal(100); + }); }); it('should horizontally scroll by width of partially visible item', () => { @@ -1397,4 +1334,76 @@ describe('', () => { ]); }); }); + + describe('dynamic tabs', () => { + const pause = (timeout) => + new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, timeout); + }); + + // https://github.com/mui/material-ui/issues/31936 + it('should not show scroll buttons if a tab added or removed in vertical mode', async function test() { + if (isJSDOM) { + this.skip(); + } + function DynamicTabs() { + const [value, setValue] = React.useState(0); + const handleChange = (event, newValue) => { + setValue(newValue); + }; + const [tabs, setTabs] = React.useState(['item1', 'item2']); + return ( + + + + + {tabs.map((label, index) => ( + + ))} + + + ); + } + const { container, getByTestId, getAllByRole } = render(); + const addButton = getByTestId('add'); + const deleteButton = getByTestId('delete'); + + fireEvent.click(addButton); + expect(hasLeftScrollButton(container)).to.equal(false); + expect(hasRightScrollButton(container)).to.equal(false); + + const tabs = getAllByRole('tab'); + const lastTab = tabs[tabs.length - 1]; + fireEvent.click(lastTab); + await pause(400); + + fireEvent.click(deleteButton); + expect(hasLeftScrollButton(container)).to.equal(false); + expect(hasRightScrollButton(container)).to.equal(false); + }); + }); }); diff --git a/test/utils/createRenderer.tsx b/test/utils/createRenderer.tsx index d4dd831f1b43f1..0385f0b301fd1f 100644 --- a/test/utils/createRenderer.tsx +++ b/test/utils/createRenderer.tsx @@ -357,6 +357,10 @@ interface Clock { * Runs the current test suite (i.e. `describe` block) with fake timers. */ withFakeTimers(): void; + /** + * Restore the real timer + */ + restore(): void; } type ClockConfig = undefined | number | Date; @@ -426,6 +430,9 @@ function createClock(defaultMode: 'fake' | 'real', config: ClockConfig): Clock { mode = defaultMode; }); }, + restore() { + clock?.restore(); + }, }; }