fix(scrollview): ScrollView layout with legacy (always-visible) scrollbars#2883
Draft
fix(scrollview): ScrollView layout with legacy (always-visible) scrollbars#2883
Conversation
|
Saadnajmi
changed the title
Saadnajmi
force-pushed
the
scrollbar-insets
branch
from
8e72129 to
e440434
Compare
… (Fabric) On macOS, legacy scrollbars sit inside the NSScrollView frame and reduce the NSClipView's visible area. React's layout system measures children against the full frame, causing content to overflow behind the scrollbar. Following the pattern from facebook#53247 (Switch), this fix reads the scrollbar width synchronously during the Yoga layout pass via a cached atomic value — no state round-trip, so the first layout is immediately correct. Key changes: - Add ScrollViewShadowNode::setSystemScrollbarWidth() / getSystemScrollbarWidth() backed by a static atomic, called from native code at class load time - applyScrollbarPadding() reads the cached value directly (not from state) - Remove scrollbarTrailingWidth/scrollbarBottomHeight from ScrollViewState - Remove autoresizingMask from documentView (prevents AppKit frame corruption) - Move NSScroller hit test check before subview loop (fixes scrollbar clicks) - Observe NSPreferredScrollerStyleDidChangeNotification for runtime changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Saadnajmi
force-pushed
the
scrollbar-insets
branch
2 times, most recently
from
5614e2d to
836e42d
Compare
Apply paddingEnd on the ScrollView's shadow node to account for legacy scrollbar width, matching the Fabric approach. A new RCTScrollViewShadowView (defined inline in RCTScrollViewManager.m) reads +[NSScroller preferredScrollerStyle] and +scrollerWidthForControlSize: directly during layoutWithMetrics: — these are thread-safe class methods, so no localData round-trip is needed. Key changes: - Add RCTScrollViewShadowView with paddingEnd for scrollbar width - Revert RCTScrollContentShadowView to stock (only RTL fix remains) - Delete RCTScrollContentLocalData files (no longer used) - Update preferredScrollerStyleDidChange: in RCTScrollView to re-tile and trigger content size update on system preference changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Saadnajmi
force-pushed
the
scrollbar-insets
branch
from
836e42d to
fe5efee
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On macOS, legacy (always-visible) scrollbars sit inside the NSScrollView frame and reduce the NSClipView's visible area. React's layout system measures ScrollView children against the full frame, causing content to overflow behind the scrollbar.
Both Fabric and Paper now use the same approach: apply
paddingEndon the ScrollView's shadow node equal to the legacy scrollbar width. This reduces the available layout space for children to match the actual visible area — the same approach SwiftUI uses when proposing sizes to ScrollView children.Fabric (commit 1)
std::atomic— no state round-trip, so the first layout is immediately correctScrollViewShadowNode::setSystemScrollbarWidth()is called from+[RCTScrollViewComponentView initialize]at class load timeapplyScrollbarPadding()reads the atomic value directly during constructionautoresizingMaskfrom documentView (prevents AppKit frame corruption on first render)NSPreferredScrollerStyleDidChangeNotificationfor runtime changesPaper (commit 2)
RCTScrollViewShadowView(defined inline inRCTScrollViewManager.m) appliespaddingEndinlayoutWithMetrics:using thread-safe+[NSScroller preferredScrollerStyle]class methodsRCTScrollContentLocalDataround-trip mechanism entirely (files deleted)RCTScrollContentShadowViewreverted to stock (only RTL fix remains)preferredScrollerStyleDidChange:inRCTScrollViewre-tiles and triggers content size updateTest plan
🤖 Generated with Claude Code