fix(tabs): change scrollintoview logic on load (#4143)

* fix(tabs): change scrollintoview logic on load. Use container scorll instead * fix(tabs): changeset * fix(tabs): linting --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
twilio-labs · Nov 6, 2024 · e9586bd · e9586bd
1 parent aa19228
commit e9586bd
Show file tree

Hide file tree

Showing 7 changed files with 194 additions and 27 deletions.
diff --git a/.changeset/cold-eagles-search.md b/.changeset/cold-eagles-search.md
@@ -0,0 +1,6 @@
+---
+"@twilio-paste/tabs": patch
+"@twilio-paste/core": patch
+---
+
+[Tabs] fix issue with currently selected item causing vertical scroll
diff --git a/.changeset/hot-owls-dream.md b/.changeset/hot-owls-dream.md
@@ -0,0 +1,6 @@
+---
+"@twilio-paste/in-page-navigation": patch
+"@twilio-paste/core": patch
+---
+
+[InPageNavigation] fix issue with currently selected item causing vertical scroll
diff --git a/packages/paste-core/components/in-page-navigation/src/InPageNavigation.tsx b/packages/paste-core/components/in-page-navigation/src/InPageNavigation.tsx
@@ -106,15 +106,19 @@ const InPageNavigation = React.forwardRef<HTMLDivElement, InPageNavigationProps>
     // Scroll to the selected tab if it exists on mount
     React.useEffect(() => {
       if (listRef.current && scrollableRef.current) {
-        setTimeout(
-          () =>
-            listRef.current
-              ?.querySelector(`[aria-current="page"]`)
-              // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-              // @ts-ignore - behavior is typed incorrectly in Typescript v4, fixed in v5+
-              ?.scrollIntoView({ behavior: "smooth", block: "nearest", inline: "center" }),
-          1,
-        );
+        const currentSelectedTab = listRef.current.querySelector(`[aria-current="page"]`);
+        const scrollableWidth = scrollableRef.current.getBoundingClientRect().width;
+
+        if (
+          currentSelectedTab &&
+          (currentSelectedTab?.getBoundingClientRect().x < 0 ||
+            currentSelectedTab?.getBoundingClientRect().right > scrollableWidth)
+        ) {
+          const scrollLeft =
+            currentSelectedTab.getBoundingClientRect().x - scrollableRef.current.getBoundingClientRect().x;
+          scrollableRef.current.scrollLeft += scrollLeft;
+        }
+
         scrollableRef.current?.addEventListener("scroll", handleScrollEvent);
         window.addEventListener("resize", handleScrollEvent);
         determineElementsOutOfBounds(scrollableRef.current, listRef.current);

diff --git a/packages/paste-core/components/in-page-navigation/stories/index.stories.tsx b/packages/paste-core/components/in-page-navigation/stories/index.stories.tsx
@@ -112,6 +112,39 @@ export const LinkOverflowExample: StoryFn = () => {
   );
 };
 
+export const LinkOverflowExampleScrollTest: StoryFn = () => {
+  /* using UID here to make unique labels for landmarks in Storybook for axe testing */
+  return (
+    <Box width="size60">
+      <Box height="1800px" />
+      <InPageNavigation aria-label={`get started ${useUID()}`}>
+        <InPageNavigationItem href="#">Super SIM</InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Programmable Wireless">
+          Programmable Wireless
+        </InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Super Duper SIM">
+          Super Duper SIM
+        </InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Programmable Wirefull">
+          Programmable Wirefull
+        </InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Super SIM">
+          Super SIM
+        </InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Programmable Wireless">
+          Programmable Wireless
+        </InPageNavigationItem>
+        <InPageNavigationItem currentPage={true} href="#" title="Super Duper SIM">
+          Super Duper SIM
+        </InPageNavigationItem>
+        <InPageNavigationItem href="#" title="Programmable Wirefull">
+          Programmable Wirefull
+        </InPageNavigationItem>
+      </InPageNavigation>
+    </Box>
+  );
+};
+
 export const Customized: StoryFn = () => {
   const theme = useTheme();
   return (

diff --git a/packages/paste-core/components/tabs/src/TabList.tsx b/packages/paste-core/components/tabs/src/TabList.tsx
@@ -65,6 +65,7 @@ const HorizontalTabList: React.FC<React.PropsWithChildren<{ variant?: Variants;
   element,
 }) => {
   const ref = React.useRef<HTMLElement>(null);
+  const { selectedId } = React.useContext(TabsContext);
   //  ref to the scrollable element
   const scrollableRef = React.useRef<HTMLDivElement>(null);
   const isInverse = variant === "inverse" || variant === "inverse_fitted";
@@ -85,6 +86,22 @@ const HorizontalTabList: React.FC<React.PropsWithChildren<{ variant?: Variants;
     }
   }, [ref.current, scrollableRef.current]);
 
+  React.useEffect(() => {
+    if (scrollableRef.current && selectedId) {
+      // eslint-disable-next-line unicorn/prefer-query-selector
+      const selectedTabEl = document.getElementById(selectedId);
+      const scrollableWidth = scrollableRef.current.getBoundingClientRect().width;
+
+      if (
+        selectedTabEl &&
+        (selectedTabEl?.getBoundingClientRect().x < 0 || selectedTabEl?.getBoundingClientRect().right > scrollableWidth)
+      ) {
+        const scrollLeft = selectedTabEl.getBoundingClientRect().x - scrollableRef.current.getBoundingClientRect().x;
+        scrollableRef.current.scrollLeft += scrollLeft;
+      }
+    }
+  }, [scrollableRef.current, selectedId]);
+
   // Cleanup event listeners on destroy
   React.useEffect(() => {
     return () => {

diff --git a/packages/paste-core/components/tabs/src/Tabs.tsx b/packages/paste-core/components/tabs/src/Tabs.tsx
@@ -48,7 +48,6 @@ export interface TabsProps extends TabPrimitiveInitialState {
 const Tabs = React.forwardRef<HTMLDivElement, TabsProps>(
   ({ children, element, orientation = "horizontal", state, variant, ...initialState }, ref) => {
     // If returned state from primitive has orientation set to undefined, use the default "horizontal"
-    const [prevSelectedTab, setPrevSelectedTab] = React.useState<string | undefined>(undefined);
     const { orientation: tabOrientation = orientation, ...tab } =
       state || useTabPrimitiveState({ orientation, ...initialState });
     const elementName = getElementName(tabOrientation, "TABS", element);
@@ -57,23 +56,6 @@ const Tabs = React.forwardRef<HTMLDivElement, TabsProps>(
       [tab, tabOrientation, variant],
     );
 
-    const { selectedId } = tab;
-    // Scroll to the selected tab if it exists on mount
-    React.useEffect(() => {
-      if (typeof selectedId === "string") {
-        setTimeout(() => {
-          document
-            // eslint-disable-next-line unicorn/prefer-query-selector
-            .getElementById(selectedId)
-            // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-            // @ts-ignore - behavior is typed incorrectly in Typescript v4, fixed in v5+
-            ?.scrollIntoView({ behavior: prevSelectedTab === undefined ? "instant" : "smooth" });
-
-          setPrevSelectedTab(selectedId);
-        }, 1);
-      }
-    }, [prevSelectedTab, selectedId]);
-
     const returnValue = <TabsContext.Provider value={value}>{children}</TabsContext.Provider>;
 
     if (tabOrientation === "vertical") {

diff --git a/packages/paste-core/components/tabs/stories/index.stories.tsx b/packages/paste-core/components/tabs/stories/index.stories.tsx
@@ -128,6 +128,68 @@ export const HorizontalTabsOverflow = (): JSX.Element => {
   );
 };
 
+export const HorizontalTabOverflowScrollCheck = (): JSX.Element => {
+  const selectedId = useUID();
+  const uniqueBaseID = useUID();
+  return (
+    <Box maxWidth="size80">
+      <Box height="1800px" />
+      <Tabs selectedId={selectedId} baseId={`${uniqueBaseID}-horizontal-tabs-example`}>
+        <TabList aria-label="LGBTQ+ Projects">
+          <Tab>Inside Out</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab id={selectedId}>Transgender District</Tab>
+          <Tab>Audre Lorde Project</Tab>
+          <Tab disabled>Coming soon...</Tab>
+        </TabList>
+        <TabPanels>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Inside Out
+            </Heading>
+            <Paragraph>
+              Inside Out empowers, educates, and advocates for LGBTQ+ of youth from the Pikes Peak Region in Southern
+              Colorado. Inside Out does this by creating safe spaces, support systems and teaching life skills to all
+              youth in the community and working to make the community safer and more accepting of gender and sexual
+              orientation diversity.
+            </Paragraph>
+            <Anchor href="https://insideoutys.org/">Support Inside Out</Anchor>
+          </TabPanel>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Transgender District
+            </Heading>
+            <Paragraph>
+              The mission of the Transgender District is to create an urban environment that fosters the rich history,
+              culture, legacy, and empowerment of transgender people and its deep roots in the southeastern Tenderloin
+              neighborhood. The transgender district aims to stabilize and economically empower the transgender
+              community through ownership of homes, businesses, historic and cultural sites, and safe community spaces.
+            </Paragraph>
+            <Anchor href="https://www.transgenderdistrictsf.com/">Support The Transgender District</Anchor>
+          </TabPanel>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Audre Lorde Project
+            </Heading>
+            <Paragraph>
+              The Audre Lorde Project is a Lesbian, Gay, Bisexual, Two Spirit, Trans and Gender Non Conforming People of
+              Color center for community organizing, focusing on the New York City area. Through mobilization, education
+              and capacity-building, they work for community wellness and progressive social and economic justice.
+              Committed to struggling across differences, they seek to responsibly reflect, represent and serve their
+              various communities.
+            </Paragraph>
+            <Anchor href="https://alp.org/">Support The Audre Lorde Project</Anchor>
+          </TabPanel>
+        </TabPanels>
+      </Tabs>
+    </Box>
+  );
+};
+
 export const FittedTabs = (): JSX.Element => {
   const selectedId = useUID();
   const uniqueBaseID = useUID();
@@ -236,6 +298,63 @@ export const VerticalTabs = (): JSX.Element => {
   );
 };
 
+export const VerticalTabsScrollCheck = (): JSX.Element => {
+  const selectedId = useUID();
+  const uniqueBaseID = useUID();
+  return (
+    <Box>
+      <Box height="1800px" />
+      <Tabs orientation="vertical" selectedId={selectedId} baseId={`${uniqueBaseID}-vertical-tabs-example`}>
+        <TabList aria-label="LGBTQ+ Projects">
+          <Tab id={selectedId}>Inside Out</Tab>
+          <Tab>Transgender District</Tab>
+          <Tab>Audre Lorde Project</Tab>
+          <Tab disabled>Coming soon...</Tab>
+        </TabList>
+        <TabPanels>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Inside Out
+            </Heading>
+            <Paragraph>
+              Inside Out empowers, educates, and advocates for LGBTQ+ of youth from the Pikes Peak Region in Southern
+              Colorado. Inside Out does this by creating safe spaces, support systems and teaching life skills to all
+              youth in the community and working to make the community safer and more accepting of gender and sexual
+              orientation diversity.
+            </Paragraph>
+            <Anchor href="https://insideoutys.org/">Support Inside Out</Anchor>
+          </TabPanel>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Transgender District
+            </Heading>
+            <Paragraph>
+              The mission of the Transgender District is to create an urban environment that fosters the rich history,
+              culture, legacy, and empowerment of transgender people and its deep roots in the southeastern Tenderloin
+              neighborhood. The transgender district aims to stabilize and economically empower the transgender
+              community through ownership of homes, businesses, historic and cultural sites, and safe community spaces.
+            </Paragraph>
+            <Anchor href="https://www.transgenderdistrictsf.com/">Support The Transgender District</Anchor>
+          </TabPanel>
+          <TabPanel>
+            <Heading as="h2" variant="heading20">
+              Audre Lorde Project
+            </Heading>
+            <Paragraph>
+              The Audre Lorde Project is a Lesbian, Gay, Bisexual, Two Spirit, Trans and Gender Non Conforming People of
+              Color center for community organizing, focusing on the New York City area. Through mobilization, education
+              and capacity-building, they work for community wellness and progressive social and economic justice.
+              Committed to struggling across differences, they seek to responsibly reflect, represent and serve their
+              various communities.
+            </Paragraph>
+            <Anchor href="https://alp.org/">Support The Audre Lorde Project</Anchor>
+          </TabPanel>
+        </TabPanels>
+      </Tabs>
+    </Box>
+  );
+};
+
 export const VerticalTabsOverflow = (): JSX.Element => {
   const selectedId = useUID();
   const uniqueBaseID = useUID();