[Tabs] Fix and improve visibility of tab scroll buttons using the IntersectionObserver API (#36071)

Signed-off-by: SaidMarar <mararsaid@gmail.com>
Co-authored-by: ZeeshanTamboli <zeeshan.tamboli@gmail.com>

Author: SaidMarar

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,29 +597,68 @@ 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);
   }, []);
 
   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 */}
         <FlexContainer

packages/mui-material/src/Tabs/Tabs.test.js

@@ -8,6 +8,7 @@ import {
   fireEvent,
   screen,
   strictModeDoubleLoggingSuppressed,
+  waitFor,
 } from 'test/utils';
 import Tab from '@mui/material/Tab';
 import Tabs, { tabsClasses as classes } from '@mui/material/Tabs';
@@ -493,7 +494,6 @@ describe('<Tabs />', () => {
   });
 
   describe('prop: variant="scrollable"', () => {
-    clock.withFakeTimers();
     const tabs = (
       <Tabs value={0} style={{ width: 200 }} variant="scrollable">
         <Tab style={{ width: 120, minWidth: 'auto' }} />
@@ -509,34 +509,6 @@ describe('<Tabs />', () => {
       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(
         <Tabs value={0}>
@@ -569,8 +541,6 @@ describe('<Tabs />', () => {
   });
 
   describe('prop: scrollButtons', () => {
-    clock.withFakeTimers();
-
     it('should render scroll buttons', () => {
       const { container } = render(
         <Tabs value={0} variant="scrollable" scrollButtons>
@@ -608,65 +578,27 @@ describe('<Tabs />', () => {
       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(
-        <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
-          <Tab />
-          <Tab />
-          <Tab />
-        </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: 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(
           <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
             <Tab style={{ width: 50, minWidth: 'auto' }} />
             <Tab style={{ width: 50, minWidth: 'auto' }} />
           </Tabs>,
         );
-        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(
           <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
             <Tab style={{ width: 120, minWidth: 'auto' }} />
             <Tab style={{ width: 120, minWidth: 'auto' }} />
@@ -675,17 +607,19 @@ describe('<Tabs />', () => {
         );
         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(
           <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
             <Tab />
             <Tab />
@@ -694,67 +628,70 @@ describe('<Tabs />', () => {
         );
         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(
           <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
             <Tab style={{ width: 120, minWidth: 'auto' }} />
             <Tab style={{ width: 120, minWidth: 'auto' }} />
           </Tabs>,
         );
         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);
+        });
       });
     });
   });
 
   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(
         <Tabs value={0} variant="scrollable" scrollButtons style={{ width: 200 }}>
           <Tab style={{ width: 100, minWidth: 'auto' }} />
           <Tab style={{ width: 50, minWidth: 'auto' }} />
           <Tab style={{ width: 100, minWidth: 'auto' }} />
         </Tabs>,
       );
       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('<Tabs />', () => {
       ]);
     });
   });
+
+  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 (
+          <React.Fragment>
+            <button
+              data-testid="add"
+              onClick={() => {
+                setTabs([...tabs, `item${tabs.length + 1}`]);
+              }}
+            >
+              add
+            </button>
+            <button
+              data-testid="delete"
+              onClick={() => {
+                setTabs(tabs.slice(0, tabs.length - 1));
+                setValue(0);
+              }}
+            >
+              delete
+            </button>
+            <Tabs
+              onChange={handleChange}
+              value={value}
+              orientation="vertical"
+              variant="scrollable"
+              scrollButtons
+              style={{ width: '260px' }}
+            >
+              {tabs.map((label, index) => (
+                <Tab key={`tab${index}`} label={label} />
+              ))}
+            </Tabs>
+          </React.Fragment>
+        );
+      }
+      const { container, getByTestId, getAllByRole } = render(<DynamicTabs />);
+      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);
+    });
+  });
 });

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();
+    },
   };
 }