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.

Sign up for GitHub

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

perf: remove redundant overflow style manipulation #2678

Merged
merged 1 commit into from
Dec 29, 2024
Merged

perf: remove redundant overflow style manipulation #2678

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions packages/core/src/hooks/useIsOverflowing/useIsOverflowing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,9 @@ function checkOverflow(element: HTMLElement, ignoreHeightOverflow: boolean, tole
if (!element) {
return false;
}
const curOverflow = element.style.overflow;
if (!curOverflow || curOverflow === "visible") element.style.overflow = "hidden";
Comment on lines -8 to -9
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably intended for more accurate calculation but it triggers a redraw which makes this function heavy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you say more accurate, what are we "losing" here? only legacy browsers?

Copy link
Member Author

@talkor talkor Dec 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In modern browsers, elements can have overflow styles like visible, hidden, auto, or scroll, which control how content is displayed when it exceeds the container's dimensions. Historically (2017<), and in some edge cases even today, browsers have had quirks in how they calculate scrollWidth and scrollHeight when overflow is set to visible:

  1. overflow: visible Behavior:
    • When overflow is visible, the browser does not clip or restrict content, so scrollWidth and scrollHeight might not accurately represent the "overflowing" content dimensions.
    • Some older browsers would inconsistently calculate the scrollHeight or scrollWidth, leading to unreliable results. For example:
      • If the content extended outside the container, scrollHeight might only reflect the visible portion of the element.
  2. Dynamic Overflow Calculations:
    • Some browsers historically recalculated layout properties like scrollHeight and scrollWidth lazily or only when overflow was set to hidden, auto, or scroll. As a result, developers often had to temporarily set overflow to force the browser to update these values.
  3. Rendering Engine Differences:
    • Older rendering engines (e.g., Trident in Internet Explorer or older versions of WebKit) had subtle bugs in handling overflow. Developers would use workarounds like temporarily changing the overflow property to get consistent values.

The approach of temporarily setting overflow: hidden is a deliberate attempt to work around these historical inconsistencies. Now, it assumes that these historical quirks do not apply or that the overflow property is already managed correctly. In modern browsers, the need for such workarounds has significantly decreased. The major benefit is the dramatic performance improvement

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough

const isOverflowing =
element.clientWidth < element.scrollWidth ||
(!ignoreHeightOverflow && element.clientHeight + tolerance < element.scrollHeight);
element.style.overflow = curOverflow;
return isOverflowing;
const isOverflowingWidth = element.clientWidth < element.scrollWidth;
const isOverflowingHeight = !ignoreHeightOverflow && element.clientHeight + tolerance < element.scrollHeight;
return isOverflowingWidth || isOverflowingHeight;
}

export default function useIsOverflowing({
Expand All @@ -27,7 +23,11 @@ export default function useIsOverflowing({
checkOverflow(ref?.current, ignoreHeightOverflow, tolerance)
);
const callback = useCallback(() => {
setIsOverflowing(checkOverflow(ref?.current, ignoreHeightOverflow, tolerance));
const element = ref?.current;
if (!element) return;

const newOverflowState = checkOverflow(element, ignoreHeightOverflow, tolerance);
setIsOverflowing(prevState => (prevState !== newOverflowState ? newOverflowState : prevState));
}, [ignoreHeightOverflow, ref, tolerance]);

useResizeObserver({
Expand Down
Loading