[tabs] After removing tab scrollable mode is triggered (bad behavior of scrollable mode in case of dynamic tabs)

[konrazem]

Steps to reproduce

Link to live example: (required) https://codesandbox.io/p/sandbox/frosty-snow-tlq78l?file=%2Fpackage.json%3A6%2C32

Steps:

1. Remove "Item one"

2. After remove there is scrollable mode with arrow on the right

Current behavior

After removing last tab, scrollable mode is trigger when there is no need

Expected behavior

After remove tab there should be no arrows for scroll as width of tabs is smaller then width of the wrapper component.

Context

It looks like a BUG as arrow is suddenly shown at the end of tabs toolbar.

Your environment

npx @mui/envinfo

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: tabs, scrollable Order ID: 46820

[oliviertassinari]

This is a regression, it used to work correctly in v5.14.1 and got broken in v5.14.2. #36071 or #38133 broke this.

---

I'm reopening as #39415 doesn't include a regression test, the real fix is the regression test IMHO. Now, since this is a regression, and we will need to move that logic to Base UI, I think we definitely need a test.

---

As for the fix, it seems that the origin is the intersection observer, a minimal reproduction: https://codesandbox.io/p/sandbox/inspiring-star-hc3vns?file=%2Fsrc%2FApp.js%3A16%2C9. So I think we can solve this with simply ignoring elements that are from a previous render:

diff --git a/packages/mui-material/src/Tabs/Tabs.js b/packages/mui-material/src/Tabs/Tabs.js
index d4d41ef720..904d187e1e 100644
--- a/packages/mui-material/src/Tabs/Tabs.js
+++ b/packages/mui-material/src/Tabs/Tabs.js
@@ -619,13 +619,16 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
       };

       const handleScrollButtonStart = (entries) => {
-        setDisplayStartScroll(!entries[0].isIntersecting);
+        const isInDom = entries[0].target.parentElement !== null;
+        console.log('entries[0].isIntersecting', entries[0].isIntersecting)
+        setDisplayStartScroll(isInDom && !entries[0].isIntersecting);
       };
       const firstObserver = new IntersectionObserver(handleScrollButtonStart, observerOptions);
       firstObserver.observe(firstTab);

       const handleScrollButtonEnd = (entries) => {
-        setDisplayEndScroll(!entries[0].isIntersecting);
+        const isInDom = entries[0].target.parentElement !== null;
+        setDisplayEndScroll(isInDom && !entries[0].isIntersecting);
       };
       const lastObserver = new IntersectionObserver(handleScrollButtonEnd, observerOptions);
       lastObserver.observe(lastTab);

simpler fix.

But then, this is still hiding the root problem, we shouldn't observe an element that is unmounted. This fix makes more sense to me:

diff --git a/packages/mui-material/src/Tabs/Tabs.js b/packages/mui-material/src/Tabs/Tabs.js
index d4d41ef720..b44225cf58 100644
--- a/packages/mui-material/src/Tabs/Tabs.js
+++ b/packages/mui-material/src/Tabs/Tabs.js
@@ -17,6 +17,7 @@ import useEventCallback from '../utils/useEventCallback';
 import tabsClasses, { getTabsUtilityClass } from './tabsClasses';
 import ownerDocument from '../utils/ownerDocument';
 import ownerWindow from '../utils/ownerWindow';
+import useEnhancedEffect from '../utils/useEnhancedEffect';

 const nextItem = (list, item) => {
   if (list === item) {
@@ -597,48 +598,6 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
     };
   }, [updateIndicatorState]);

-  /**
-   * Toggle visibility of start and end scroll buttons
-   * Using IntersectionObserver on first and last Tabs.
-   */
-  React.useEffect(() => {
-    const tabListChildren = Array.from(tabListRef.current.children);
-    const length = tabListChildren.length;
-
-    if (
-      typeof IntersectionObserver !== 'undefined' &&
-      length > 0 &&
-      scrollable &&
-      scrollButtons !== false
-    ) {
-      const firstTab = tabListChildren[0];
-      const lastTab = tabListChildren[length - 1];
-      const observerOptions = {
-        root: tabsRef.current,
-        threshold: 0.99,
-      };
-
-      const handleScrollButtonStart = (entries) => {
-        setDisplayStartScroll(!entries[0].isIntersecting);
-      };
-      const firstObserver = new IntersectionObserver(handleScrollButtonStart, observerOptions);
-      firstObserver.observe(firstTab);
-
-      const handleScrollButtonEnd = (entries) => {
-        setDisplayEndScroll(!entries[0].isIntersecting);
-      };
-      const lastObserver = new IntersectionObserver(handleScrollButtonEnd, observerOptions);
-      lastObserver.observe(lastTab);
-
-      return () => {
-        firstObserver.disconnect();
-        lastObserver.disconnect();
-      };
-    }
-
-    return undefined;
-  }, [scrollable, scrollButtons, updateScrollObserver, childrenProp?.length]);
-
   React.useEffect(() => {
     setMounted(true);
   }, []);
@@ -707,6 +666,48 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
     });
   });

+  /**
+   * Toggle visibility of start and end scroll buttons
+   * Using IntersectionObserver on first and last Tabs.
+   */
+  useEnhancedEffect(() => {
+    const tabListChildren = Array.from(tabListRef.current.children);
+    const length = tabListChildren.length;
+
+    if (
+      typeof IntersectionObserver !== 'undefined' &&
+      length > 0 &&
+      scrollable &&
+      scrollButtons !== false
+    ) {
+      const firstTab = tabListChildren[0];
+      const lastTab = tabListChildren[length - 1];
+      const observerOptions = {
+        root: tabsRef.current,
+        threshold: 0.99,
+      };
+
+      const handleScrollButtonStart = (entries) => {
+        setDisplayStartScroll(!entries[0].isIntersecting);
+      };
+      const firstObserver = new IntersectionObserver(handleScrollButtonStart, observerOptions);
+      firstObserver.observe(firstTab);
+
+      const handleScrollButtonEnd = (entries) => {
+        setDisplayEndScroll(!entries[0].isIntersecting);
+      };
+      const lastObserver = new IntersectionObserver(handleScrollButtonEnd, observerOptions);
+      lastObserver.observe(lastTab);
+
+      return () => {
+        firstObserver.disconnect();
+        lastObserver.disconnect();
+      };
+    }
+
+    return undefined;
+  }, [scrollable, scrollButtons, updateScrollObserver, children.length]);
+
   const handleKeyDown = (event) => {
     const list = tabListRef.current;
     const currentFocus = ownerDocument(list).activeElement;

Potentially, the effect could be on children instead of children.length.