Skip to content

Conversation

pfo-omicsstudio
Copy link
Contributor

@pfo-omicsstudio pfo-omicsstudio commented Jun 13, 2025

This PR adds a new onOverflowChange?: (overflowing: boolean) => void prop to ScrollArea.Autosize.

It fires when content starts or stops exceeding max-height, enabling consumers to react when the container becomes scrollable.

95208be5a7182e7515d9a6eca68051df.mp4

@rtivital rtivital added the ? label Jun 16, 2025

useEffect(() => {
const el = viewportObserverRef.current;
if (!el) {
Copy link
Member

Choose a reason for hiding this comment

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

This logic might have a significant impact on performance, add early return if onOverflowChange is not a function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I also refactored it to use the useResizeObserver hook.

@pfo-omicsstudio pfo-omicsstudio requested a review from rtivital July 9, 2025 07:19

// Overflow detection (Autosize-only)
const [resizeObserverRef, rect] = useResizeObserver<HTMLDivElement>();
const combinedViewportRef = useMergeRefs([viewportRef, resizeObserverRef]);
Copy link
Member

Choose a reason for hiding this comment

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

Not a good solution: adds significant overhead with resize observer tracking when onOverflowChange is not passed. ResizeObserver must only listen for changes when it is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - I reverted the last two commits and added the early return guard.

@pfo-omicsstudio pfo-omicsstudio requested a review from rtivital July 9, 2025 11:37
@rtivital rtivital merged commit 1839f9c into mantinedev:master Jul 22, 2025
1 check passed
@rtivital
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants