[@mantine/core] ScrollArea: Fix scrollbar never visible with offsetScrollbars="present" #8844
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes ScrollArea/ScrollArea.Autosize ResizeObserver attachment when ref.current is null on first render (notably impacting offsetScrollbars="present"), ensuring scrollbar visibility/overflow detection updates correctly after mount.
Changes:
- Switch ResizeObserver target from
ref.currentto a callback-ref-backed state element so the observer reliably attaches after mount. - Apply the same callback-ref/state pattern to
ScrollArea.Autosizeoverflow detection. - Add a unit test asserting the viewport is observed when
offsetScrollbars="present".
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/@mantine/core/src/components/ScrollArea/ScrollArea.tsx | Uses callback refs + state to provide stable, post-mount elements to useResizeObserver for offsetScrollbars="present" and Autosize overflow detection. |
| packages/@mantine/core/src/components/ScrollArea/ScrollArea.test.tsx | Adds a test that mocks ResizeObserver to verify observation occurs when offsetScrollbars="present". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const combinedViewportRef = useMergeRefs([viewportRef, localViewportRef]); | ||
| const [viewportElement, setViewportElement] = useState<HTMLDivElement | null>(null); | ||
| const viewportCallbackRef = useCallback((node: HTMLDivElement | null) => { | ||
| setViewportElement(node); |
There was a problem hiding this comment.
Thanks, addressed all comments.
Changes:
- Guarded ref callback state updates against identical nodes.
- Made the ResizeObserver test mock restore safely via try/finally.
- Added Autosize coverage for observing the viewport when onOverflowChange is provided.
Verified with the ScrollArea Jest suite and oxfmt check.
| window.ResizeObserver = jest.fn().mockImplementation(() => ({ | ||
| observe, | ||
| unobserve, | ||
| })); | ||
|
|
||
| render( | ||
| <ScrollArea h={100} offsetScrollbars="present"> | ||
| <div style={{ height: 500 }}>Content</div> | ||
| </ScrollArea> | ||
| ); | ||
|
|
||
| expect(observe).toHaveBeenCalled(); | ||
|
|
||
| window.ResizeObserver = originalResizeObserver; |
There was a problem hiding this comment.
Thanks, addressed all comments.
| window.ResizeObserver = jest.fn().mockImplementation(() => ({ | ||
| observe, | ||
| unobserve, | ||
| })); |
There was a problem hiding this comment.
Thanks, addressed all comments.
| overflowingRef.current = isOverflowing; | ||
| } | ||
| }); | ||
|
|
||
| useResizeObserver(onOverflowChange ? viewportObserverRef.current : null, handleOverflowCheck); | ||
| useResizeObserver(onOverflowChange ? viewportObserverElement : null, handleOverflowCheck); |
There was a problem hiding this comment.
Thanks, addressed all comments.
1063109 to
8cc23a1
Compare
|
Thanks! |
Summary
useResizeObserverreceivedref.current(null at first render) as its element argument — since ref assignment doesn't causere-render, the ResizeObserver was never attached
useStateso the effect dependency updates after mount, correctly attaching the observerScrollArea.Autosize'sonOverflowChangedetection which had the identical bugFixes #8839
Test plan
observes viewport when offsetScrollbars is present