Material-ui: [Tabs] Scrollable Tabs are sometimes displaying partial tabs

[huysbs]

• [ X ] The issue is present in the latest release.
• [ X ] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When using scrollable tabs, the arrow buttons scroll the tabs on full width and this means that sometimes tabs will be rendered partially. A tab will be shown partially at the end of the bar and when you scroll to the right, it will also be shown partially at the beginning.

There are occasions where some tab will never been seen in full and have their label truncated at all time.

Expected Behavior 🤔

The expected behavior is either:

• all the tabs are always shown fully
• or at least the first tab should always show fully

Steps to Reproduce 🕹

The issue can be seen on:
https://codesandbox.io/s/material-demo-forked-gdrfp?file=/demo.js
At certain breakpoints, some tabs are displayed partially. In the screenshot below, "Item Seven" will never be displayed in full:

But also in the "Forced Scroll Buttons" section of the page linked below. You can see that "Item Five" is displayed partially and when scrolling
https://material-ui.com/components/tabs/

Context 🔦

We have a video streaming website and we are displaying a list of movie genres in Tabs but some of the genres are being truncated, for Horror will be either displayed as H or *orror

Questions

It would be good to fix this or at least provide a way to use a pixel offset value for the scrolling.

[mnajdova]

@huysbs thanks for reporting the issue. I would say this is very corner case scenario, but we could provide fix for it. What do you think about this changes:

diff --git a/packages/material-ui/src/Tabs/Tabs.js b/packages/material-ui/src/Tabs/Tabs.js
index 7495f28ed..b70914e73 100644
--- a/packages/material-ui/src/Tabs/Tabs.js
+++ b/packages/material-ui/src/Tabs/Tabs.js
@@ -239,12 +241,29 @@ const Tabs = React.forwardRef(function Tabs(props, ref) {
     scroll(scrollValue);
   };

+  const getLastElementSize = container => {
+    const containerSize = container[clientSize];
+    let totalSize = 0;
+
+    const children = Array.from(container.children);
+
+    for(let i = 0; i < children.length; i++) {
+      const button = children[i];
+      totalSize += button[clientSize];
+
+      if (totalSize >= containerSize) {
+        return totalSize - containerSize;
+      }
+    }
+  }
+
   const handleStartScrollClick = () => {
     moveTabsScroll(-tabsRef.current[clientSize]);
   };

   const handleEndScrollClick = () => {
-    moveTabsScroll(tabsRef.current[clientSize]);
+    const lastElementSize = getLastElementSize(tabsRef.current.firstChild);
+    moveTabsScroll(tabsRef.current[clientSize] - lastElementSize);
   };

   // TODO Remove <ScrollbarSize /> as browser support for hidding the scrollbar

What we are doing with this changes basically is, calculating the size of the last visible element and extracting it from the scroll difference, so that whenever are invoking the scroll to next, the first element will be fully visible.

Not sure if we want to always do these calculations, so maybe it would be a good idea to add a prop for this behavior. @oliviertassinari thoughts?

[oliviertassinari]

@mnajdova Interesting, I was wondering about only scrolling by the number of pixels of tabs that are fully visible. For instance here:

When a user clicks on the left arrow to scroll back, right now, we scroll for the full width but by only scrolling by the number of pixels of tabs that are fully visible, we can ensure that truncated tabs are ignored.

[mnajdova]

@oliviertassinari that could work too, the code would need to be slightly changed so that the size of the scroll would be the sum of all fully visible elements. Maybe something like this:

diff --git a/packages/material-ui/src/Tabs/Tabs.js b/packages/material-ui/src/Tabs/Tabs.js
index 7495f28ed..b70914e73 100644
--- a/packages/material-ui/src/Tabs/Tabs.js
+++ b/packages/material-ui/src/Tabs/Tabs.js
@@ -239,12 +241,29 @@ const Tabs = React.forwardRef(function Tabs(props, ref) {
     scroll(scrollValue);
   };

+  const getLastElementSize = container => {
+    const containerSize = container[clientSize];
+    let totalSize = 0;
+
+    const children = Array.from(container.children);
+
+    for(let i = 0; i < children.length; i++) {
+      const button = children[i];
+      if (totalSize + button[clientSize] > containerSize) {
+        break;
+      }
+      totalSize += button[clientSize];
+    }
+    return totalSize;
+  }
+
   const handleStartScrollClick = () => {
-   moveTabsScroll(-tabsRef.current[clientSize]);
+   moveTabsScroll(-1*getScrollSize(tabsRef.current.firstChild));
   };

   const handleEndScrollClick = () => {
-    moveTabsScroll(tabsRef.current[clientSize]);
+    moveTabsScroll(getScrollSize(tabsRef.current.firstChild));
   };

   // TODO Remove <ScrollbarSize /> as browser support for hidding the scrollbar

I think both approaches can work

[oliviertassinari]

@mnajdova that last solution looks great.
I guess the only question left is: should we solve the problem, does it worth the bundle size cost? It seems that it does :)

[mnajdova]

I think yes, it's definitely an improvement. @huysbs would you be interested in creating a PR? :)

[huysbs]

@mnajdova @oliviertassinari thank you for looking into this.
I'll give that a test and will try to make a PR for it.