fix refresh control offset
This commit is contained in:
@@ -198,66 +198,43 @@ index ac553045a9c0ce77e288277912538d9e131ebc01..d99c8f4db5a07f1e4ffe7e03ff23adce
|
|||||||
NSRange visibleGlyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
|
NSRange visibleGlyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
|
||||||
|
|
||||||
diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
||||||
index 60160efb163d91813fa2ca7ca758b51afcf261e1..4791a4f123f74d9737e2d0cb1b7fa6e129315562 100644
|
index 60160efb163d91813fa2ca7ca758b51afcf261e1..fb646fe945ffe4aa4a386f80a1e42a90180691f1 100644
|
||||||
--- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
--- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
||||||
+++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
+++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTPullToRefreshViewComponentView.mm
|
||||||
@@ -39,8 +39,45 @@ - (void)setRefreshing:(BOOL)refreshing
|
@@ -42,6 +42,32 @@ - (void)setRefreshing:(BOOL)refreshing
|
||||||
|
|
||||||
#else
|
|
||||||
|
|
||||||
+// Fix for https://github.com/facebook/react-native/issues/43388 — on iOS 17.4+
|
|
||||||
+// the pull-to-refresh trigger haptic is permanently suppressed if tintColor is
|
|
||||||
+// set before the control is actually inside the scroll view's view hierarchy.
|
|
||||||
+// Assigning scrollView.refreshControl does NOT insert it immediately (UIKit
|
|
||||||
+// does that lazily on a later layout pass), so attach-time is still too early.
|
|
||||||
+// Instead, store the wanted color and apply it from didMoveToSuperview, and
|
|
||||||
+// refuse direct tintColor sets until the superview is the scroll view.
|
|
||||||
+// Mirrors Bluesky's Paper patch (bluesky-social/social-app#5605).
|
|
||||||
+@interface RCTHapticCompatibleRefreshControl : UIRefreshControl
|
|
||||||
+@property (nonatomic, strong) UIColor *customTintColor;
|
|
||||||
+@end
|
|
||||||
+
|
|
||||||
+@implementation RCTHapticCompatibleRefreshControl
|
|
||||||
+
|
|
||||||
+- (void)didMoveToSuperview
|
|
||||||
+{
|
|
||||||
+ [super didMoveToSuperview];
|
|
||||||
+ if (self.customTintColor) {
|
|
||||||
+ [self setTintColor:self.customTintColor];
|
|
||||||
+ }
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+- (void)setCustomTintColor:(UIColor *)customTintColor
|
|
||||||
+{
|
|
||||||
+ _customTintColor = customTintColor;
|
|
||||||
+ [self setTintColor:customTintColor];
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+- (void)setTintColor:(UIColor *)tintColor
|
|
||||||
+{
|
|
||||||
+ if ([self.superview isKindOfClass:[UIScrollView class]] && self.tintColor != tintColor) {
|
|
||||||
+ [super setTintColor:tintColor];
|
|
||||||
+ }
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+@end
|
|
||||||
+
|
|
||||||
@implementation RCTPullToRefreshViewComponentView {
|
@implementation RCTPullToRefreshViewComponentView {
|
||||||
- UIRefreshControl *_refreshControl;
|
UIRefreshControl *_refreshControl;
|
||||||
+ RCTHapticCompatibleRefreshControl *_refreshControl;
|
|
||||||
RCTScrollViewComponentView *__weak _scrollViewComponentView;
|
RCTScrollViewComponentView *__weak _scrollViewComponentView;
|
||||||
|
+ /*
|
||||||
|
+ * Deferred props: updateProps runs during the Create mount mutation, before
|
||||||
|
+ * _attach puts the control on the scroll view, and writes to a detached
|
||||||
|
+ * UIRefreshControl are hazardous:
|
||||||
|
+ *
|
||||||
|
+ * - tintColor: writing it to a detached control permanently suppresses the
|
||||||
|
+ * pull-to-refresh trigger haptic on iOS 17.4+
|
||||||
|
+ * (https://github.com/facebook/react-native/issues/43388).
|
||||||
|
+ *
|
||||||
|
+ * - progressViewOffset (the bounds.origin shift): on iOS 26 the control's
|
||||||
|
+ * _UIRefreshControlModernContentView positions itself at whatever
|
||||||
|
+ * bounds.origin it observes when it is CREATED - at insertion into the
|
||||||
|
+ * scroll view, or earlier if a pre-attach property write materializes it -
|
||||||
|
+ * and keeps that y forever (width tracks, y never re-pins; verified via
|
||||||
|
+ * on-device frame logging, Aug 2026). A pre-attach shift is therefore
|
||||||
|
+ * baked into the content view's own frame and cancelled exactly, hiding
|
||||||
|
+ * the spinner. Applied post-attach, the content view has already been
|
||||||
|
+ * created at origin 0 and the same bounds shift works as intended.
|
||||||
|
+ *
|
||||||
|
+ * Both props are parked here and applied only once the control is inside
|
||||||
|
+ * the scroll view.
|
||||||
|
+ */
|
||||||
|
+ UIColor *_pendingTintColor;
|
||||||
|
+ BOOL _hasPendingTintColor;
|
||||||
|
+ CGFloat _pendingProgressViewOffset;
|
||||||
|
+ BOOL _hasPendingProgressViewOffset;
|
||||||
// This variable keeps track of whether the view is recycled or not. Once the view is recycled, the component
|
// This variable keeps track of whether the view is recycled or not. Once the view is recycled, the component
|
||||||
// creates a new instance of UIRefreshControl, resetting the native props to the default values.
|
// creates a new instance of UIRefreshControl, resetting the native props to the default values.
|
||||||
@@ -66,7 +103,7 @@ - (instancetype)initWithFrame:(CGRect)frame
|
// However, when recycling, we are keeping around the old _props. The flag is used to force the application
|
||||||
|
@@ -79,10 +105,25 @@ + (ComponentDescriptorProvider)componentDescriptorProvider
|
||||||
- (void)_initializeUIRefreshControl
|
|
||||||
{
|
|
||||||
- _refreshControl = [UIRefreshControl new];
|
|
||||||
+ _refreshControl = [RCTHapticCompatibleRefreshControl new];
|
|
||||||
[_refreshControl addTarget:self
|
|
||||||
action:@selector(handleUIControlEventValueChanged)
|
|
||||||
forControlEvents:UIControlEventValueChanged];
|
|
||||||
@@ -79,6 +116,17 @@ + (ComponentDescriptorProvider)componentDescriptorProvider
|
|
||||||
return concreteComponentDescriptorProvider<PullToRefreshViewComponentDescriptor>();
|
return concreteComponentDescriptorProvider<PullToRefreshViewComponentDescriptor>();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -275,23 +252,82 @@ index 60160efb163d91813fa2ca7ca758b51afcf261e1..4791a4f123f74d9737e2d0cb1b7fa6e1
|
|||||||
- (void)prepareForRecycle
|
- (void)prepareForRecycle
|
||||||
{
|
{
|
||||||
[super prepareForRecycle];
|
[super prepareForRecycle];
|
||||||
@@ -93,7 +141,9 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
|
_scrollViewComponentView = nil;
|
||||||
|
+ _pendingTintColor = nil;
|
||||||
|
+ _hasPendingTintColor = NO;
|
||||||
|
+ _pendingProgressViewOffset = 0;
|
||||||
|
+ _hasPendingProgressViewOffset = NO;
|
||||||
|
[self _initializeUIRefreshControl];
|
||||||
|
_recycled = YES;
|
||||||
|
}
|
||||||
|
@@ -93,7 +134,8 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
|
||||||
const auto &newConcreteProps = static_cast<const PullToRefreshViewProps &>(*props);
|
const auto &newConcreteProps = static_cast<const PullToRefreshViewProps &>(*props);
|
||||||
|
|
||||||
if (_recycled || newConcreteProps.tintColor != oldConcreteProps.tintColor) {
|
if (_recycled || newConcreteProps.tintColor != oldConcreteProps.tintColor) {
|
||||||
- _refreshControl.tintColor = RCTUIColorFromSharedColor(newConcreteProps.tintColor);
|
- _refreshControl.tintColor = RCTUIColorFromSharedColor(newConcreteProps.tintColor);
|
||||||
+ // Goes through RCTHapticCompatibleRefreshControl, which defers the real
|
+ // Deferred until the control is inside the scroll view (#43388).
|
||||||
+ // tintColor set until the control is inside the scroll view (#43388).
|
+ [self _updateTintColor:RCTUIColorFromSharedColor(newConcreteProps.tintColor)];
|
||||||
+ _refreshControl.customTintColor = RCTUIColorFromSharedColor(newConcreteProps.tintColor);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (_recycled || newConcreteProps.progressViewOffset != oldConcreteProps.progressViewOffset) {
|
if (_recycled || newConcreteProps.progressViewOffset != oldConcreteProps.progressViewOffset) {
|
||||||
@@ -153,7 +203,12 @@ - (void)_updateTitle
|
@@ -141,11 +183,50 @@ - (void)handleUIControlEventValueChanged
|
||||||
|
|
||||||
|
- (void)_updateProgressViewOffset:(Float)progressViewOffset
|
||||||
|
{
|
||||||
|
+ _pendingProgressViewOffset = progressViewOffset;
|
||||||
|
+ _hasPendingProgressViewOffset = YES;
|
||||||
|
+ // Applies immediately for runtime changes while the control is attached;
|
||||||
|
+ // pre-attach sets wait until the control is inside the scroll view (see the
|
||||||
|
+ // _pendingProgressViewOffset declaration).
|
||||||
|
+ [self _applyPendingProgressViewOffsetIfPossible];
|
||||||
|
+ if (_hasPendingProgressViewOffset) {
|
||||||
|
+ [self setNeedsLayout];
|
||||||
|
+ }
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+- (void)_applyPendingProgressViewOffsetIfPossible
|
||||||
|
+{
|
||||||
|
+ if (!_hasPendingProgressViewOffset || ![_refreshControl.superview isKindOfClass:[UIScrollView class]]) {
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
_refreshControl.bounds = CGRectMake(
|
||||||
|
_refreshControl.bounds.origin.x,
|
||||||
|
- -progressViewOffset,
|
||||||
|
+ -_pendingProgressViewOffset,
|
||||||
|
_refreshControl.bounds.size.width,
|
||||||
|
_refreshControl.bounds.size.height);
|
||||||
|
+ _hasPendingProgressViewOffset = NO;
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+- (void)_updateTintColor:(UIColor *)tintColor
|
||||||
|
+{
|
||||||
|
+ _pendingTintColor = tintColor;
|
||||||
|
+ _hasPendingTintColor = YES;
|
||||||
|
+ // Applies immediately for runtime changes while the control is attached;
|
||||||
|
+ // pre-attach sets wait until the control is inside the scroll view.
|
||||||
|
+ [self _applyPendingTintColorIfPossible];
|
||||||
|
+ if (_hasPendingTintColor) {
|
||||||
|
+ [self setNeedsLayout];
|
||||||
|
+ }
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+- (void)_applyPendingTintColorIfPossible
|
||||||
|
+{
|
||||||
|
+ if (!_hasPendingTintColor || ![_refreshControl.superview isKindOfClass:[UIScrollView class]]) {
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
+ _refreshControl.tintColor = _pendingTintColor;
|
||||||
|
+ _pendingTintColor = nil;
|
||||||
|
+ _hasPendingTintColor = NO;
|
||||||
|
}
|
||||||
|
|
||||||
|
- (void)_updateTitle
|
||||||
|
@@ -153,7 +234,12 @@ - (void)_updateTitle
|
||||||
const auto &concreteProps = static_cast<const PullToRefreshViewProps &>(*_props);
|
const auto &concreteProps = static_cast<const PullToRefreshViewProps &>(*_props);
|
||||||
|
|
||||||
if (concreteProps.title.empty()) {
|
if (concreteProps.title.empty()) {
|
||||||
- _refreshControl.attributedTitle = nil;
|
- _refreshControl.attributedTitle = nil;
|
||||||
+ // Avoid touching the control when there is nothing to clear — writing
|
+ // Avoid touching the control when there is nothing to clear - writing
|
||||||
+ // attributedTitle (even nil) before the control is in the scroll view
|
+ // attributedTitle (even nil) before the control is in the scroll view
|
||||||
+ // hierarchy can suppress the pull-to-refresh haptic (#43388).
|
+ // hierarchy can suppress the pull-to-refresh haptic (#43388).
|
||||||
+ if (_refreshControl.attributedTitle != nil) {
|
+ if (_refreshControl.attributedTitle != nil) {
|
||||||
@@ -300,3 +336,38 @@ index 60160efb163d91813fa2ca7ca758b51afcf261e1..4791a4f123f74d9737e2d0cb1b7fa6e1
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@@ -172,6 +258,18 @@ - (void)layoutSubviews
|
||||||
|
{
|
||||||
|
[super layoutSubviews];
|
||||||
|
|
||||||
|
+ /*
|
||||||
|
+ * Fallback for the pending props: _attach applies them right after the
|
||||||
|
+ * refreshControl assignment (insertion is synchronous there on current iOS),
|
||||||
|
+ * but should UIKit ever defer the insertion to a later layout pass, re-arm
|
||||||
|
+ * and retry until the control is actually inside the scroll view.
|
||||||
|
+ */
|
||||||
|
+ [self _applyPendingTintColorIfPossible];
|
||||||
|
+ [self _applyPendingProgressViewOffsetIfPossible];
|
||||||
|
+ if ((_hasPendingTintColor || _hasPendingProgressViewOffset) && _scrollViewComponentView != nil) {
|
||||||
|
+ [self setNeedsLayout];
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
// Attempts to begin refreshing before the initial layout are ignored by _refreshControl. So if the control is
|
||||||
|
// refreshing when mounted, we need to call beginRefreshing in layoutSubviews or it won't work.
|
||||||
|
if (self.window) {
|
||||||
|
@@ -209,6 +307,15 @@ - (void)_attach
|
||||||
|
|
||||||
|
// This ensures that layoutSubviews is called. Without this, recycled instances won't refresh on mount
|
||||||
|
[self setNeedsLayout];
|
||||||
|
+
|
||||||
|
+ /*
|
||||||
|
+ * The assignment above inserts the control (and creates its content view)
|
||||||
|
+ * synchronously on current iOS - verified via frame logging - so the
|
||||||
|
+ * pending props can be applied immediately. layoutSubviews is the fallback
|
||||||
|
+ * if insertion is ever deferred.
|
||||||
|
+ */
|
||||||
|
+ [self _applyPendingTintColorIfPossible];
|
||||||
|
+ [self _applyPendingProgressViewOffsetIfPossible];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -6,27 +6,60 @@ Patching `RCTRefreshControl.mm` temporarily to play an impact haptic on refresh
|
|||||||
17.4, there has been a regression somewhere causing haptics to not play on iOS on refresh. Should monitor for an update
|
17.4, there has been a regression somewhere causing haptics to not play on iOS on refresh. Should monitor for an update
|
||||||
in the RN repo: https://github.com/facebook/react-native/issues/43388
|
in the RN repo: https://github.com/facebook/react-native/issues/43388
|
||||||
|
|
||||||
## RCTPullToRefreshViewComponentView.mm Patch - Same iOS 17.4+ haptic regression on New Arch
|
## RCTPullToRefreshViewComponentView.mm Patch - iOS 17.4+ haptic regression and iOS 26 progressViewOffset cancellation on New Arch
|
||||||
|
|
||||||
The Paper fix above does not cover Fabric: `RCTPullToRefreshViewComponentView` owns a plain
|
Both bugs share one root cause, established by instrumented frame-logging runs on the iOS 26
|
||||||
`UIRefreshControl` and applies `tintColor` in `updateProps`, which runs during the Create mount
|
simulator (Aug 2026): **writes to a detached `UIRefreshControl` are hazardous, because the
|
||||||
mutation - before `_attach` puts the control on the scroll view - so the haptic regression
|
control's `_UIRefreshControlModernContentView` bakes in the state it observes at its own
|
||||||
(react-native#43388) resurfaced on the New Architecture.
|
creation.** Facts proven by the logs:
|
||||||
|
|
||||||
Port of the Paper approach to Fabric (developed and A/B tested in the `fresh-expo57-refresh`
|
- `scrollView.refreshControl` assignment inserts the control and creates its content view
|
||||||
repro app against bare RN 0.86.2 and Expo 54/RN 0.81.5 controls):
|
**synchronously** on iOS 26 (the "UIKit inserts lazily on a later layout pass" folklore is
|
||||||
|
false there).
|
||||||
|
- The content view can also be materialized **earlier** by a pre-attach property write (observed
|
||||||
|
with `tintColor`) while the control is still detached.
|
||||||
|
- The content view positions itself at whatever `bounds.origin` exists at its creation and keeps
|
||||||
|
that y forever - width tracks on later layouts, y never re-pins.
|
||||||
|
|
||||||
|
Consequences:
|
||||||
|
|
||||||
|
**1. progressViewOffset.** Stock Fabric writes the offset as a `bounds.origin` shift in
|
||||||
|
`updateProps`, pre-attach. The content view is then created (at insertion) already inside the
|
||||||
|
shifted bounds, pins to it, and cancels the shift exactly - spinner hidden behind the floating
|
||||||
|
home header (home is the only screen passing a non-zero offset). Stock RN appeared to work only
|
||||||
|
by accident: its own pre-attach `tintColor` write materialized the content view at origin 0
|
||||||
|
*before* the offset write. Possibly related upstream: react-native#54183.
|
||||||
|
|
||||||
|
**2. Haptic (react-native#43388).** The Paper fix above does not cover Fabric: `updateProps`
|
||||||
|
writes `tintColor` pre-attach, and a tint write on a detached control materializes the content
|
||||||
|
view outside the scroll view, permanently suppressing the trigger haptic on iOS 17.4+ (the
|
||||||
|
creation-time-state story likely explains this too, though the haptic wiring itself is not
|
||||||
|
observable in logs).
|
||||||
|
|
||||||
|
**The fix**: both `tintColor` and `progressViewOffset` are parked in the component view
|
||||||
|
(`_pendingTintColor` / `_pendingProgressViewOffset`, no `UIRefreshControl` subclass) and applied
|
||||||
|
only once `_refreshControl.superview` is the scroll view - by then the content view exists,
|
||||||
|
was created at origin 0, and a bounds shift lands visibly. Application points: immediately in
|
||||||
|
`_updateX` for runtime changes while attached; in `_attach` right after the assignment (insertion
|
||||||
|
is synchronous); and from `layoutSubviews` with a `setNeedsLayout` re-arm as a fallback should
|
||||||
|
insertion ever be deferred.
|
||||||
|
|
||||||
|
Supporting changes:
|
||||||
|
|
||||||
- `RCTHapticCompatibleRefreshControl` subclass stores the wanted color in `customTintColor` and
|
|
||||||
refuses direct `setTintColor:` until the superview is the scroll view; `didMoveToSuperview`
|
|
||||||
applies the stored color once actually inside the hierarchy.
|
|
||||||
- `shouldBeRecycled = NO`: recycled instances get all props force-applied in `updateProps` before
|
- `shouldBeRecycled = NO`: recycled instances get all props force-applied in `updateProps` before
|
||||||
the new control is inserted into the scroll view, which would re-trigger the bug; opting out of
|
the new control is attached, which would re-trigger the pre-attach hazards; opting out keeps
|
||||||
recycling keeps every mount on the untouched-before-attach path.
|
every mount on the untouched-before-attach path.
|
||||||
- `_updateTitle` no longer writes `attributedTitle = nil` when there is nothing to clear - even a
|
- `_updateTitle` no longer writes `attributedTitle = nil` when there is nothing to clear - even a
|
||||||
nil write before attach suppresses the haptic.
|
nil write before attach suppresses the haptic.
|
||||||
|
|
||||||
Upstream issue still open as of Aug 2026. Haptics cannot be verified on the simulator - physical
|
History: an earlier iteration fixed the offset by porting Paper's frame-offset trick into an
|
||||||
device only.
|
`RCTHapticCompatibleRefreshControl` subclass (worked, verified on device) - replaced by the
|
||||||
|
deferral once the root cause was understood. The control's `didMoveToSuperview` appeared broken
|
||||||
|
as a tint application point in early non-rigorous testing; unproven, not disproven.
|
||||||
|
|
||||||
|
Upstream issue #43388 still open as of Aug 2026. Haptics cannot be verified on the simulator -
|
||||||
|
physical device only. Spinner position verified via frame logs; haptic on this variant NOT yet
|
||||||
|
device-verified.
|
||||||
|
|
||||||
## RCTEnhancedScrollView.mm / RCTScrollViewComponentView.mm Patch - centerContent insets stale after content resize on New Arch
|
## RCTEnhancedScrollView.mm / RCTScrollViewComponentView.mm Patch - centerContent insets stale after content resize on New Arch
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user