Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/healthy-squids-cheat.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
27 changes: 17 additions & 10 deletions packages/paste-core/components/code-block/src/utlis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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" });
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
27 changes: 17 additions & 10 deletions packages/paste-core/components/in-page-navigation/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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" });
}
};
10 changes: 7 additions & 3 deletions packages/paste-core/components/tabs/src/TabList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
27 changes: 17 additions & 10 deletions packages/paste-core/components/tabs/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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" });
}
};
Loading