fix(tabs): do not scroll to end in some instances (#4148)

* fix(tabs): non scroll to end * fix(tabs): missing current check * fix(tabs): include button widths for comparison
twilio-labs · Nov 12, 2024 · 0f7cb22 · 0f7cb22
1 parent d0eea12
commit 0f7cb22
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 37 deletions.
diff --git a/.changeset/healthy-squids-cheat.md b/.changeset/healthy-squids-cheat.md
@@ -0,0 +1,8 @@
+---
+"@twilio-paste/code-block": patch
+"@twilio-paste/in-page-navigation": patch
+"@twilio-paste/tabs": patch
+"@twilio-paste/core": patch
+---
+
+[Tabs, CodeBlock, InPageNavigation] fixed a bug where items in the tabs list may not complete the scroll, still showing the overflow right button.
diff --git a/packages/paste-core/components/code-block/src/CodeBlockTabList.tsx b/packages/paste-core/components/code-block/src/CodeBlockTabList.tsx
@@ -81,7 +81,7 @@ export const CodeBlockTabList = React.forwardRef<HTMLDivElement, CodeBlockTabLis
           <OverflowButton
             position="left"
             onClick={() =>
-              handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, listRef.current)
+              handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
             }
             visible={Boolean(elementOutOBoundsLeft)}
             element={element}
@@ -116,7 +116,7 @@ export const CodeBlockTabList = React.forwardRef<HTMLDivElement, CodeBlockTabLis
           <OverflowButton
             position="right"
             onClick={() =>
-              handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, listRef.current)
+              handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
             }
             visible={Boolean(elementOutOBoundsRight)}
             element={element}

diff --git a/packages/paste-core/components/code-block/src/utlis.ts b/packages/paste-core/components/code-block/src/utlis.ts
@@ -17,7 +17,6 @@ export const useElementsOutOfBounds = (): {
     if (scrollContainer && listContainer) {
       const currentScrollContainerRightPosition = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().right;
       const currentScrollContainerXOffset = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().x;
-
       let leftOutOfBounds: HTMLDivElement | null = null;
       let rightOutOfBounds: HTMLDivElement | null = null;
 
@@ -30,14 +29,14 @@ export const useElementsOutOfBounds = (): {
            * Compares the left side of the tab with the left side of the scrollable container position
            * as the x value will not be 0 due to being offset in the screen.
            */
-          if (x < currentScrollContainerXOffset) {
+          if (Math.round(x) < Math.round(currentScrollContainerXOffset - 28)) {
             leftOutOfBounds = tab;
           }
           /**
-           * Compares the right side to the end of container with some buffer. Also ensure there are
+           * Compares the right side to the end of container and button width. Also ensure there are
            * no value set as it loops through the array we don't want it to override the first value out of bounds.
            */
-          if (right > currentScrollContainerRightPosition + 10 && !rightOutOfBounds) {
+          if (Math.round(right) > Math.round(currentScrollContainerRightPosition + 28) && !rightOutOfBounds) {
             rightOutOfBounds = tab;
           }
         }
@@ -76,12 +75,20 @@ export const handleScrollDirection = (
   direction: "left" | "right",
   elementOutOBoundsLeft: HTMLDivElement | null,
   elementOutOBoundsRight: HTMLDivElement | null,
-  listContainer: HTMLElement | null,
+  scrollContainer: HTMLElement | null,
 ): void => {
-  if (listContainer) {
-    const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
-    if (elementToScrollTo) {
-      elementToScrollTo.scrollIntoView({ behavior: "smooth", block: "nearest", inline: "center" });
-    }
+  const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
+
+  if (scrollContainer && elementToScrollTo) {
+    const elementRect = elementToScrollTo.getBoundingClientRect();
+    const containerRect = scrollContainer.getBoundingClientRect();
+    const containerScrollLeft = scrollContainer.scrollLeft;
+
+    // Calculate the new scroll position
+    const newScrollLeft =
+      containerScrollLeft + (elementRect.left - containerRect.left) - containerRect.width / 2 + elementRect.width / 2;
+
+    // Set the new scroll position
+    scrollContainer.scrollTo({ left: newScrollLeft, behavior: "smooth" });
   }
 };
diff --git a/packages/paste-core/components/in-page-navigation/src/InPageNavigation.tsx b/packages/paste-core/components/in-page-navigation/src/InPageNavigation.tsx
@@ -196,7 +196,7 @@ const InPageNavigation = React.forwardRef<HTMLDivElement, InPageNavigationProps>
           <OverflowButton
             position="left"
             onClick={() =>
-              handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, listRef.current)
+              handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
             }
             visible={Boolean(elementOutOBoundsLeft)}
             element={element}
@@ -233,7 +233,7 @@ const InPageNavigation = React.forwardRef<HTMLDivElement, InPageNavigationProps>
           <OverflowButton
             position="right"
             onClick={() =>
-              handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, listRef.current)
+              handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
             }
             visible={Boolean(elementOutOBoundsRight)}
             element={element}

diff --git a/packages/paste-core/components/in-page-navigation/src/utils.ts b/packages/paste-core/components/in-page-navigation/src/utils.ts
@@ -17,7 +17,6 @@ export const useElementsOutOfBounds = (): {
     if (scrollContainer && listContainer) {
       const currentScrollContainerRightPosition = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().right;
       const currentScrollContainerXOffset = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().x;
-
       let leftOutOfBounds: HTMLDivElement | null = null;
       let rightOutOfBounds: HTMLDivElement | null = null;
 
@@ -30,14 +29,14 @@ export const useElementsOutOfBounds = (): {
            * Compares the left side of the tab with the left side of the scrollable container position
            * as the x value will not be 0 due to being offset in the screen.
            */
-          if (x < currentScrollContainerXOffset) {
+          if (Math.round(x) < Math.round(currentScrollContainerXOffset - 28)) {
             leftOutOfBounds = tab;
           }
           /**
-           * Compares the right side to the end of container with some buffer. Also ensure there are
+           * Compares the right side to the end of container and button width. Also ensure there are
            * no value set as it loops through the array we don't want it to override the first value out of bounds.
            */
-          if (right > currentScrollContainerRightPosition + 10 && !rightOutOfBounds) {
+          if (Math.round(right) > Math.round(currentScrollContainerRightPosition + 28) && !rightOutOfBounds) {
             rightOutOfBounds = tab;
           }
         }
@@ -76,12 +75,20 @@ export const handleScrollDirection = (
   direction: "left" | "right",
   elementOutOBoundsLeft: HTMLDivElement | null,
   elementOutOBoundsRight: HTMLDivElement | null,
-  listContainer: HTMLElement | null,
+  scrollContainer: HTMLElement | null,
 ): void => {
-  if (listContainer) {
-    const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
-    if (elementToScrollTo) {
-      elementToScrollTo.scrollIntoView({ behavior: "smooth", block: "nearest", inline: "center" });
-    }
+  const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
+
+  if (scrollContainer && elementToScrollTo) {
+    const elementRect = elementToScrollTo.getBoundingClientRect();
+    const containerRect = scrollContainer.getBoundingClientRect();
+    const containerScrollLeft = scrollContainer.scrollLeft;
+
+    // Calculate the new scroll position
+    const newScrollLeft =
+      containerScrollLeft + (elementRect.left - containerRect.left) - containerRect.width / 2 + elementRect.width / 2;
+
+    // Set the new scroll position
+    scrollContainer.scrollTo({ left: newScrollLeft, behavior: "smooth" });
   }
 };
diff --git a/packages/paste-core/components/tabs/src/TabList.tsx b/packages/paste-core/components/tabs/src/TabList.tsx
@@ -79,7 +79,7 @@ const HorizontalTabList: React.FC<React.PropsWithChildren<{ variant?: Variants;
   };
 
   React.useEffect(() => {
-    if (ref.current) {
+    if (ref.current && scrollableRef.current) {
       scrollableRef.current?.addEventListener("scroll", handleScrollEvent);
       window.addEventListener("resize", handleScrollEvent);
       determineElementsOutOfBounds(scrollableRef.current, ref.current);
@@ -116,7 +116,9 @@ const HorizontalTabList: React.FC<React.PropsWithChildren<{ variant?: Variants;
     <Box display="flex" overflow="hidden">
       <OverflowButton
         position="left"
-        onClick={() => handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, ref.current)}
+        onClick={() =>
+          handleScrollDirection("left", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
+        }
         visible={Boolean(elementOutOBoundsLeft)}
         element={element}
         showShadow={showShadow}
@@ -147,7 +149,9 @@ const HorizontalTabList: React.FC<React.PropsWithChildren<{ variant?: Variants;
       </Box>
       <OverflowButton
         position="right"
-        onClick={() => handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, ref.current)}
+        onClick={() =>
+          handleScrollDirection("right", elementOutOBoundsLeft, elementOutOBoundsRight, scrollableRef.current)
+        }
         visible={Boolean(elementOutOBoundsRight)}
         element={element}
         showShadow={showShadow}

diff --git a/packages/paste-core/components/tabs/src/utils.ts b/packages/paste-core/components/tabs/src/utils.ts
@@ -23,7 +23,6 @@ export const useElementsOutOfBounds = (): {
     if (scrollContainer && listContainer) {
       const currentScrollContainerRightPosition = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().right;
       const currentScrollContainerXOffset = (scrollContainer as HTMLDivElement)?.getBoundingClientRect().x;
-
       let leftOutOfBounds: HTMLDivElement | null = null;
       let rightOutOfBounds: HTMLDivElement | null = null;
 
@@ -36,14 +35,14 @@ export const useElementsOutOfBounds = (): {
            * Compares the left side of the tab with the left side of the scrollable container position
            * as the x value will not be 0 due to being offset in the screen.
            */
-          if (x < currentScrollContainerXOffset) {
+          if (Math.round(x) < Math.round(currentScrollContainerXOffset - 28)) {
             leftOutOfBounds = tab;
           }
           /**
-           * Compares the right side to the end of container with some buffer. Also ensure there are
+           * Compares the right side to the end of container and button width. Also ensure there are
            * no value set as it loops through the array we don't want it to override the first value out of bounds.
            */
-          if (right > currentScrollContainerRightPosition + 10 && !rightOutOfBounds) {
+          if (Math.round(right) > Math.round(currentScrollContainerRightPosition + 28) && !rightOutOfBounds) {
             rightOutOfBounds = tab;
           }
         }
@@ -82,12 +81,20 @@ export const handleScrollDirection = (
   direction: "left" | "right",
   elementOutOBoundsLeft: HTMLDivElement | null,
   elementOutOBoundsRight: HTMLDivElement | null,
-  listContainer: HTMLElement | null,
+  scrollContainer: HTMLElement | null,
 ): void => {
-  if (listContainer) {
-    const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
-    if (elementToScrollTo) {
-      elementToScrollTo.scrollIntoView({ behavior: "smooth", block: "nearest", inline: "center" });
-    }
+  const elementToScrollTo = direction === "left" ? elementOutOBoundsLeft : elementOutOBoundsRight;
+
+  if (scrollContainer && elementToScrollTo) {
+    const elementRect = elementToScrollTo.getBoundingClientRect();
+    const containerRect = scrollContainer.getBoundingClientRect();
+    const containerScrollLeft = scrollContainer.scrollLeft;
+
+    // Calculate the new scroll position
+    const newScrollLeft =
+      containerScrollLeft + (elementRect.left - containerRect.left) - containerRect.width / 2 + elementRect.width / 2;
+
+    // Set the new scroll position
+    scrollContainer.scrollTo({ left: newScrollLeft, behavior: "smooth" });
   }
 };