Skip to content

fix: noop scrollRectToVisible on iOS#1336

Merged
kirillzyusko merged 4 commits intomainfrom
fix/undesired-scroll
Mar 4, 2026
Merged

fix: noop scrollRectToVisible on iOS#1336
kirillzyusko merged 4 commits intomainfrom
fix/undesired-scroll

Conversation

@kirillzyusko
Copy link
Copy Markdown
Owner

@kirillzyusko kirillzyusko commented Mar 3, 2026

📜 Description

Make scrollRectToVisible of ReactScrollView no-op.

💡 Motivation and Context

This issue became visible after #797

Before we were modifying height of fake view. And scrollRectToVisible is not sensitive to this:

- (void)scrollRectToVisible:(CGRect)rect animated:(BOOL)animated
{
  // Limiting scroll area to an area where we actually have content.
  CGSize contentSize = self.contentSize;
  UIEdgeInsets contentInset = self.contentInset;
  CGSize fullSize = CGSizeMake(
      contentSize.width + contentInset.left + contentInset.right,
      contentSize.height + contentInset.top + contentInset.bottom);

  rect = CGRectIntersection((CGRect){CGPointZero, fullSize}, rect);
  if (CGRectIsNull(rect)) {
    return;
  }

  [super scrollRectToVisible:rect animated:animated];
}

But since we started to modify contentInset this function now can also "scroll" and this causes strange effect such as #1331

This is a ReactScrollView built-in solution for "avoiding" TextInput. Logs:

1, 298.31225727257987, 116, 718.6666666666666
'syncKeyboardFrame', 308
14
116, 718.6666666666666, 834.6666666666666
1, 298.66666666666663, 116, 718.6666666666666
[scrollRectToVisible] rect=(16.0, 713.7, 10.3, 24.7) animated=1
  contentOffset=(0.0, 298.7) bounds=(402.0 x 758.0)
  contentSize=(402.0, 1084.7) contentInset=(t=0.0 l=0.0 b=339.0 r=0.0)
  fullSize=(402.0, 1423.7) visibleY=[298.7, 1056.7] alreadyVisible=1
  safeAreaInsets=(t=16.0 l=0.0 b=34.0 r=0.0)
  adjustedContentInset=(t=0.0 l=0.0 b=339.0 r=0.0)
  -> CALLING [super scrollRectToVisible]
'syncKeyboardFrame', 308

308 keyboard height + 1 static pixel + 30 extraKeyboardHeight = 339

Which we can evaluate as:

effective visible height = bounds.height - contentInset.bottom
                         = 758 - 339 = 419

visible range = [contentOffset.y, contentOffset.y + 419]
              = [298.7, 717.7]

rect bottom   = 713.7 + 24.7 = 738.4

738.4 > 717.7  →  NOT visible from UIKit's perspective!

So UIKit scrolls by 738.4 - 717.7 ≈ 20.7px 😡 I think this effect is totally undesirable for KeyboardAwareScrollVIew/ClippingScrollView since we control scroll position on our own, so via swizzling (this is the only one option at the moment, correct option would be to add a new prop to ScrollView in react-native) I make this method a no-op.

I checked and functionality like "tap-status-bar-to-scroll" works well. So I think it's safe to have this fix 🤞

Closes #1331

📢 Changelog

JS

  • make ClippingScrollView real view on iOS;

iOS

  • add ClippingScrollView shadow node;
  • replace scrollRectToVisible to no-op;

Android

  • add ClippingScrollView shadow node;

🤔 How Has This Been Tested?

Tested manually on iPhone 17 Pro (iOS 26.2).

📸 Screenshots (if appropriate):

scroll-rect-to-visible-no-op.mov

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko self-assigned this Mar 3, 2026
@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🍎 iOS iOS specific KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component KeyboardChatScrollView 💬 Anything about chat functionality labels Mar 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 3, 2026

📊 Package size report

Current size Target Size Difference
279973 bytes 279084 bytes 889 bytes 📈

@kirillzyusko kirillzyusko marked this pull request as ready for review March 4, 2026 10:01
@kirillzyusko kirillzyusko merged commit a096e39 into main Mar 4, 2026
30 of 31 checks passed
@kirillzyusko kirillzyusko deleted the fix/undesired-scroll branch March 4, 2026 10:01
trcoffman added a commit to trcoffman/react-native-keyboard-controller that referenced this pull request Mar 5, 2026
… iOS

Implements a runtime swizzling fix for React Native 0.81+ bug where
ScrollView's contentInset area fails to respond to touch events. This
prevents users from initiating scroll gestures by touching the inset
padding area, which is critical for chat UX patterns where contentInset
positions messages at the top of the screen.

## Problem

React Native's RCTScrollViewComponentView.betterHitTest returns the
container view (self) instead of the underlying UIScrollView instance
when the touch is in the contentInset area. This causes touch events
to be intercepted by the container, preventing the scroll view from
receiving gestures.

Upstream issue: facebook/react-native#54123

## Solution

Uses Objective-C runtime method swizzling to override hitTest:withEvent:
on the ScrollView's container view. The fix mirrors the upstream patch:

- Calls the original hitTest:withEvent: implementation
- When the result is self (the container) - which is the bug - dynamically
  finds and returns the UIScrollView child instead
- Otherwise preserves the original result (subviews, nil, etc.)

Key implementation detail: the UIScrollView is looked up dynamically at
hit-test time rather than captured during swizzling. This ensures the
fix works across React Native hot reloads and full refreshes where the
view hierarchy is recreated but the swizzled class persists.

Implementation follows the same pattern as PR kirillzyusko#1336's scrollRectToVisible
fix: dynamic subclass creation with idempotent prefix check, applied lazily
in didMoveToWindow.

## Benefits

✅ Users can now scroll by touching contentInset area
✅ Interactive content (Pressables, buttons, inputs) continues to work normally
✅ Survives React Native hot reloads and full refreshes
✅ Fixes chat UX where messages need top positioning via contentInset
✅ Non-breaking: defensive implementation returns early if structure unexpected
✅ Compatible with existing scrollRectToVisible noop fix
✅ Zero-cost for apps not using ClippingScrollView
✅ Provides immediate relief while waiting for upstream React Native fix

## Performance Considerations

The dynamic scrollView lookup only executes when:
- The original hitTest returns the container (empty space/contentInset area)
- NOT when touching actual content (messages, buttons, etc.)

Cost: iterating through container's direct subviews (typically 1-3 views)
- UIScrollView + maybe scroll indicators
- Just pointer comparisons and class checks
- Negligible performance impact

## Risks and Considerations

⚠️ Runtime swizzling: Fragile if React Native's view hierarchy changes
   Mitigation: Defensive checks, silently returns if container not found

⚠️ Dynamic lookup on every hit test in contentInset area: Small overhead
   Mitigation: Only 1-3 subviews to check, only runs for empty space touches

⚠️ React Native version variations: Different RN versions may have different hierarchies
   Mitigation: Graceful degradation - no fix applied but doesn't break

⚠️ Calling original implementation: Assumes it's safe to invoke
   Mitigation: Standard ObjC pattern, same lifetime guarantees as the view

## Testing Recommendations

- Test contentInset with top/bottom/left/right values
- Verify touch in contentInset area initiates scroll
- Verify interactive elements (Pressables, buttons, inputs) remain functional
- Test with inverted ScrollViews (chat pattern)
- Verify behavior persists after hot reload
- Verify behavior persists after full refresh
- Test keyboard interactions remain functional
- Verify no console warnings
- Test on iOS 17+ with React Native 0.81+

## Technical Details

Implementation in ClippingScrollViewDecoratorViewManager.mm:
- Added KCApplyFixedHitTest() function that swizzles hitTest:withEvent:
- Modified didMoveToWindow() to apply both fixes
- Captures original IMP and calls it via function pointer
- Conditional: if (result == self) lookup and return UIScrollView child
- Dynamic lookup avoids stale references across RN refreshes
- Uses KC_FixedHitTest_ prefix for idempotent check
trcoffman added a commit to trcoffman/react-native-keyboard-controller that referenced this pull request Mar 5, 2026
… iOS

Implements a runtime swizzling fix for React Native 0.81+ bug where
ScrollView's contentInset area fails to respond to touch events. This
prevents users from initiating scroll gestures by touching the inset
padding area, which is critical for chat UX patterns where contentInset
positions messages at the top of the screen.

## Problem

React Native's RCTScrollViewComponentView.betterHitTest returns the
container view (self) instead of the underlying UIScrollView instance
when the touch is in the contentInset area. This causes touch events
to be intercepted by the container, preventing the scroll view from
receiving gestures.

Upstream issue: facebook/react-native#54123

## Solution

Uses Objective-C runtime method swizzling to override hitTest:withEvent:
on the ScrollView's container view. The fix mirrors the upstream patch:

- Calls the original hitTest:withEvent: implementation
- When the result is self (the container) - which is the bug - dynamically
  finds and returns the UIScrollView child instead
- Otherwise preserves the original result (subviews, nil, etc.)

Key implementation detail: the UIScrollView is looked up dynamically at
hit-test time rather than captured during swizzling. This ensures the
fix works across React Native hot reloads and full refreshes where the
view hierarchy is recreated but the swizzled class persists.

Implementation follows the same pattern as PR kirillzyusko#1336's scrollRectToVisible
fix: dynamic subclass creation with idempotent prefix check, applied lazily
in didMoveToWindow.

## Benefits

✅ Users can now scroll by touching contentInset area
✅ Interactive content (Pressables, buttons, inputs) continues to work normally
✅ Survives React Native hot reloads and full refreshes
✅ Fixes chat UX where messages need top positioning via contentInset
✅ Non-breaking: defensive implementation returns early if structure unexpected
✅ Compatible with existing scrollRectToVisible noop fix
✅ Zero-cost for apps not using ClippingScrollView
✅ Provides immediate relief while waiting for upstream React Native fix

## Performance Considerations

The dynamic scrollView lookup only executes when:
- The original hitTest returns the container (empty space/contentInset area)
- NOT when touching actual content (messages, buttons, etc.)

Cost: iterating through container's direct subviews (typically 1-3 views)
- UIScrollView + maybe scroll indicators
- Just pointer comparisons and class checks
- Negligible performance impact

## Risks and Considerations

⚠️ Runtime swizzling: Fragile if React Native's view hierarchy changes
   Mitigation: Defensive checks, silently returns if container not found

⚠️ Dynamic lookup on every hit test in contentInset area: Small overhead
   Mitigation: Only 1-3 subviews to check, only runs for empty space touches

⚠️ React Native version variations: Different RN versions may have different hierarchies
   Mitigation: Graceful degradation - no fix applied but doesn't break

⚠️ Calling original implementation: Assumes it's safe to invoke
   Mitigation: Standard ObjC pattern, same lifetime guarantees as the view

## Testing Recommendations

- Test contentInset with top/bottom/left/right values
- Verify touch in contentInset area initiates scroll
- Verify interactive elements (Pressables, buttons, inputs) remain functional
- Test with inverted ScrollViews (chat pattern)
- Verify behavior persists after hot reload
- Verify behavior persists after full refresh
- Test keyboard interactions remain functional
- Verify no console warnings
- Test on iOS 17+ with React Native 0.81+

## Technical Details

Implementation in ClippingScrollViewDecoratorViewManager.mm:
- Added KCApplyFixedHitTest() function that swizzles hitTest:withEvent:
- Modified didMoveToWindow() to apply both fixes
- Captures original IMP and calls it via function pointer
- Conditional: if (result == self) lookup and return UIScrollView child
- Dynamic lookup avoids stale references across RN refreshes
- Uses KC_FixedHitTest_ prefix for idempotent check
trcoffman added a commit to trcoffman/react-native-keyboard-controller that referenced this pull request Mar 6, 2026
… iOS

Implements a runtime swizzling fix for React Native 0.81+ bug where
ScrollView's contentInset area fails to respond to touch events. This
prevents users from initiating scroll gestures by touching the inset
padding area, which is critical for chat UX patterns where contentInset
positions messages at the top of the screen.

## Problem

React Native's RCTScrollViewComponentView.betterHitTest returns the
container view (self) instead of the underlying UIScrollView instance
when the touch is in the contentInset area. This causes touch events
to be intercepted by the container, preventing the scroll view from
receiving gestures.

Upstream issue: facebook/react-native#54123

## Solution

Uses Objective-C runtime method swizzling to override hitTest:withEvent:
on the ScrollView's container view. The fix mirrors the upstream patch:

- Calls the original hitTest:withEvent: implementation
- When the result is self (the container) - which is the bug - dynamically
  finds and returns the UIScrollView child instead
- Otherwise preserves the original result (subviews, nil, etc.)

Key implementation detail: the UIScrollView is looked up dynamically at
hit-test time rather than captured during swizzling. This ensures the
fix works across React Native hot reloads and full refreshes where the
view hierarchy is recreated but the swizzled class persists.

Implementation follows the same pattern as PR kirillzyusko#1336's scrollRectToVisible
fix: dynamic subclass creation with idempotent prefix check, applied lazily
in didMoveToWindow.

## Benefits

✅ Users can now scroll by touching contentInset area
✅ Interactive content (Pressables, buttons, inputs) continues to work normally
✅ Survives React Native hot reloads and full refreshes
✅ Fixes chat UX where messages need top positioning via contentInset
✅ Non-breaking: defensive implementation returns early if structure unexpected
✅ Compatible with existing scrollRectToVisible noop fix
✅ Zero-cost for apps not using ClippingScrollView
✅ Provides immediate relief while waiting for upstream React Native fix

## Performance Considerations

The dynamic scrollView lookup only executes when:
- The original hitTest returns the container (empty space/contentInset area)
- NOT when touching actual content (messages, buttons, etc.)

Cost: iterating through container's direct subviews (typically 1-3 views)
- UIScrollView + maybe scroll indicators
- Just pointer comparisons and class checks
- Negligible performance impact

## Risks and Considerations

⚠️ Runtime swizzling: Fragile if React Native's view hierarchy changes
   Mitigation: Defensive checks, silently returns if container not found

⚠️ Dynamic lookup on every hit test in contentInset area: Small overhead
   Mitigation: Only 1-3 subviews to check, only runs for empty space touches

⚠️ React Native version variations: Different RN versions may have different hierarchies
   Mitigation: Graceful degradation - no fix applied but doesn't break

⚠️ Calling original implementation: Assumes it's safe to invoke
   Mitigation: Standard ObjC pattern, same lifetime guarantees as the view

## Testing Recommendations

- Test contentInset with top/bottom/left/right values
- Verify touch in contentInset area initiates scroll
- Verify interactive elements (Pressables, buttons, inputs) remain functional
- Test with inverted ScrollViews (chat pattern)
- Verify behavior persists after hot reload
- Verify behavior persists after full refresh
- Test keyboard interactions remain functional
- Verify no console warnings
- Test on iOS 17+ with React Native 0.81+

## Technical Details

Implementation in ClippingScrollViewDecoratorViewManager.mm:
- Added KCApplyFixedHitTest() function that swizzles hitTest:withEvent:
- Modified didMoveToWindow() to apply both fixes
- Captures original IMP and calls it via function pointer
- Conditional: if (result == self) lookup and return UIScrollView child
- Dynamic lookup avoids stale references across RN refreshes
- Uses KC_FixedHitTest_ prefix for idempotent check
trcoffman added a commit to trcoffman/react-native-keyboard-controller that referenced this pull request Mar 6, 2026
… iOS

Implements a runtime swizzling fix for React Native 0.81+ bug where
ScrollView's contentInset area fails to respond to touch events. This
prevents users from initiating scroll gestures by touching the inset
padding area, which is critical for chat UX patterns where contentInset
positions messages at the top of the screen.

## Problem

React Native's RCTScrollViewComponentView.betterHitTest returns the
container view (self) instead of the underlying UIScrollView instance
when the touch is in the contentInset area. This causes touch events
to be intercepted by the container, preventing the scroll view from
receiving gestures.

Upstream issue: facebook/react-native#54123

## Solution

Uses Objective-C runtime method swizzling to override hitTest:withEvent:
on the ScrollView's container view. The fix mirrors the upstream patch:

- Calls the original hitTest:withEvent: implementation
- When the result is self (the container) - which is the bug - dynamically
  finds and returns the UIScrollView child instead
- Otherwise preserves the original result (subviews, nil, etc.)

Key implementation detail: the UIScrollView is looked up dynamically at
hit-test time rather than captured during swizzling. This ensures the
fix works across React Native hot reloads and full refreshes where the
view hierarchy is recreated but the swizzled class persists.

Implementation follows the same pattern as PR kirillzyusko#1336's scrollRectToVisible
fix: dynamic subclass creation with idempotent prefix check, applied lazily
in didMoveToWindow.

## Benefits

✅ Users can now scroll by touching contentInset area
✅ Interactive content (Pressables, buttons, inputs) continues to work normally
✅ Survives React Native hot reloads and full refreshes
✅ Fixes chat UX where messages need top positioning via contentInset
✅ Non-breaking: defensive implementation returns early if structure unexpected
✅ Compatible with existing scrollRectToVisible noop fix
✅ Zero-cost for apps not using ClippingScrollView
✅ Provides immediate relief while waiting for upstream React Native fix

## Performance Considerations

The dynamic scrollView lookup only executes when:
- The original hitTest returns the container (empty space/contentInset area)
- NOT when touching actual content (messages, buttons, etc.)

Cost: iterating through container's direct subviews (typically 1-3 views)
- UIScrollView + maybe scroll indicators
- Just pointer comparisons and class checks
- Negligible performance impact

## Risks and Considerations

⚠️ Runtime swizzling: Fragile if React Native's view hierarchy changes
   Mitigation: Defensive checks, silently returns if container not found

⚠️ Dynamic lookup on every hit test in contentInset area: Small overhead
   Mitigation: Only 1-3 subviews to check, only runs for empty space touches

⚠️ React Native version variations: Different RN versions may have different hierarchies
   Mitigation: Graceful degradation - no fix applied but doesn't break

⚠️ Calling original implementation: Assumes it's safe to invoke
   Mitigation: Standard ObjC pattern, same lifetime guarantees as the view

## Testing Recommendations

- Test contentInset with top/bottom/left/right values
- Verify touch in contentInset area initiates scroll
- Verify interactive elements (Pressables, buttons, inputs) remain functional
- Test with inverted ScrollViews (chat pattern)
- Verify behavior persists after hot reload
- Verify behavior persists after full refresh
- Test keyboard interactions remain functional
- Verify no console warnings
- Test on iOS 17+ with React Native 0.81+

## Technical Details

Implementation in ClippingScrollViewDecoratorViewManager.mm:
- Added KCApplyFixedHitTest() function that swizzles hitTest:withEvent:
- Modified didMoveToWindow() to apply both fixes
- Captures original IMP and calls it via function pointer
- Conditional: if (result == self) lookup and return UIScrollView child
- Dynamic lookup avoids stale references across RN refreshes
- Uses KC_FixedHitTest_ prefix for idempotent check
trcoffman added a commit to trcoffman/react-native-keyboard-controller that referenced this pull request Mar 9, 2026
… iOS

Implements a runtime swizzling fix for React Native 0.81+ bug where
ScrollView's contentInset area fails to respond to touch events. This
prevents users from initiating scroll gestures by touching the inset
padding area, which is critical for chat UX patterns where contentInset
positions messages at the top of the screen.

## Problem

React Native's RCTScrollViewComponentView.betterHitTest returns the
container view (self) instead of the underlying UIScrollView instance
when the touch is in the contentInset area. This causes touch events
to be intercepted by the container, preventing the scroll view from
receiving gestures.

Upstream issue: facebook/react-native#54123

## Solution

Uses Objective-C runtime method swizzling to override hitTest:withEvent:
on the ScrollView's container view. The fix mirrors the upstream patch:

- Calls the original hitTest:withEvent: implementation
- When the result is self (the container) - which is the bug - dynamically
  finds and returns the UIScrollView child instead
- Otherwise preserves the original result (subviews, nil, etc.)

Key implementation detail: the UIScrollView is looked up dynamically at
hit-test time rather than captured during swizzling. This ensures the
fix works across React Native hot reloads and full refreshes where the
view hierarchy is recreated but the swizzled class persists.

Implementation follows the same pattern as PR kirillzyusko#1336's scrollRectToVisible
fix: dynamic subclass creation with idempotent prefix check, applied lazily
in didMoveToWindow.

## Benefits

✅ Users can now scroll by touching contentInset area
✅ Interactive content (Pressables, buttons, inputs) continues to work normally
✅ Survives React Native hot reloads and full refreshes
✅ Fixes chat UX where messages need top positioning via contentInset
✅ Non-breaking: defensive implementation returns early if structure unexpected
✅ Compatible with existing scrollRectToVisible noop fix
✅ Zero-cost for apps not using ClippingScrollView
✅ Provides immediate relief while waiting for upstream React Native fix

## Performance Considerations

The dynamic scrollView lookup only executes when:
- The original hitTest returns the container (empty space/contentInset area)
- NOT when touching actual content (messages, buttons, etc.)

Cost: iterating through container's direct subviews (typically 1-3 views)
- UIScrollView + maybe scroll indicators
- Just pointer comparisons and class checks
- Negligible performance impact

## Risks and Considerations

⚠️ Runtime swizzling: Fragile if React Native's view hierarchy changes
   Mitigation: Defensive checks, silently returns if container not found

⚠️ Dynamic lookup on every hit test in contentInset area: Small overhead
   Mitigation: Only 1-3 subviews to check, only runs for empty space touches

⚠️ React Native version variations: Different RN versions may have different hierarchies
   Mitigation: Graceful degradation - no fix applied but doesn't break

⚠️ Calling original implementation: Assumes it's safe to invoke
   Mitigation: Standard ObjC pattern, same lifetime guarantees as the view

## Testing Recommendations

- Test contentInset with top/bottom/left/right values
- Verify touch in contentInset area initiates scroll
- Verify interactive elements (Pressables, buttons, inputs) remain functional
- Test with inverted ScrollViews (chat pattern)
- Verify behavior persists after hot reload
- Verify behavior persists after full refresh
- Test keyboard interactions remain functional
- Verify no console warnings
- Test on iOS 17+ with React Native 0.81+

## Technical Details

Implementation in ClippingScrollViewDecoratorViewManager.mm:
- Added KCApplyFixedHitTest() function that swizzles hitTest:withEvent:
- Modified didMoveToWindow() to apply both fixes
- Captures original IMP and calls it via function pointer
- Conditional: if (result == self) lookup and return UIScrollView child
- Dynamic lookup avoids stale references across RN refreshes
- Uses KC_FixedHitTest_ prefix for idempotent check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working 🍎 iOS iOS specific KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component KeyboardChatScrollView 💬 Anything about chat functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KeyboardAwareScrollView does not work.

1 participant