Skip to content

Fix scrollIndicatorInsets on iOS 13 #27102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix scrollIndicatorInsets on iOS 13 #27102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RNTester/js/examples/ScrollView/ScrollViewExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ exports.examples = [
_scrollView = scrollView;
}}
automaticallyAdjustContentInsets={false}
scrollIndicatorInsets={{top: 50, bottom: 50}}
onScroll={() => {
console.log('onScroll!');
}}
Expand Down Expand Up @@ -89,6 +90,7 @@ exports.examples = [
ref={scrollView => {
_scrollView = scrollView;
}}
scrollIndicatorInsets={{left: 50, right: 50}}
automaticallyAdjustContentInsets={false}
horizontal={true}
style={[styles.scrollView, styles.horizontalScrollView]}>
Expand Down
2 changes: 1 addition & 1 deletion RNTester/js/examples/ScrollView/ScrollViewSimpleExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const styles = StyleSheet.create({
},
});

exports.title = '<ScrollView>';
exports.title = '<ScrollView> - Simple';
exports.description =
'Component that enables scrolling through child components.';

Expand Down
33 changes: 32 additions & 1 deletion React/Views/ScrollView/RCTScrollView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ - (instancetype)initWithEventDispatcher:(RCTEventDispatcher *)eventDispatcher
}
#endif

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED >= 130000 /* __IPHONE_13_0 */
Copy link
Contributor

@sammy-SC sammy-SC Nov 5, 2019

Choose a reason for hiding this comment

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

I don't think it is to have conditional compilation together with @available(iOS 13.0, *) check.
Using @available(iOS 13.0, *) is preferred.

This applies to the code below as well.

Copy link
Contributor Author

@radex radex Nov 5, 2019

Choose a reason for hiding this comment

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

I think it is, and it seems to be a convention in RN project.

@available(iOS 13.0, *) is a runtime check, and #if ... __IPHONE_OS_VERSION_MAX_ALLOWED check is for SDK the project is compiled against.

I can re-check but I think if you tried to compile this code in Xcode 10.2 or earlier, it would fail to compile

Copy link
Contributor

@hramos hramos Dec 13, 2019

Choose a reason for hiding this comment

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

I believe @radex may be referring to RCTAppearance.mm where I used this pattern as Dark Mode is new to iOS 13:

react-native/React/CoreModules/RCTAppearance.mm

Lines 28 to 29 in aff6bad

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_13_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_13_0
if (@available(iOS 13.0, *)) {

@sammy-SC do you still want this change to be made?

Copy link
Contributor

@sammy-SC sammy-SC Dec 17, 2019

Choose a reason for hiding this comment

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

@hramos is there any reason to have conditional compilation there? If not, then let's remove it.

if (@available(iOS 13.0, *)) {
_scrollView.automaticallyAdjustsScrollIndicatorInsets = NO;
}
#endif

_automaticallyAdjustContentInsets = YES;
_DEPRECATED_sendUpdatedChildFrames = NO;
_contentInset = UIEdgeInsetsZero;
Expand Down Expand Up @@ -1027,7 +1033,32 @@ - (type)getter \
RCT_SET_AND_PRESERVE_OFFSET(setShowsHorizontalScrollIndicator, showsHorizontalScrollIndicator, BOOL)
RCT_SET_AND_PRESERVE_OFFSET(setShowsVerticalScrollIndicator, showsVerticalScrollIndicator, BOOL)
RCT_SET_AND_PRESERVE_OFFSET(setZoomScale, zoomScale, CGFloat);
RCT_SET_AND_PRESERVE_OFFSET(setScrollIndicatorInsets, scrollIndicatorInsets, UIEdgeInsets);

- (UIEdgeInsets)scrollIndicatorInsets
{
return [_scrollView scrollIndicatorInsets];
}

- (void)setScrollIndicatorInsets:(UIEdgeInsets)value
{
CGPoint contentOffset = _scrollView.contentOffset;

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED >= 130000 /* __IPHONE_13_0 */
if (@available(iOS 13.0, *)) {
UIEdgeInsets verticalInsets = UIEdgeInsetsMake(value.top, 0, value.bottom, 0);
UIEdgeInsets horizontalInsets = UIEdgeInsetsMake(0, value.left, 0, value.right);
_scrollView.verticalScrollIndicatorInsets = verticalInsets;
_scrollView.horizontalScrollIndicatorInsets = horizontalInsets;
} else {
_scrollView.scrollIndicatorInsets = value;
}
#else
_scrollView.scrollIndicatorInsets = value;
#endif


_scrollView.contentOffset = contentOffset;
}

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED >= 110000 /* __IPHONE_11_0 */
- (void)setContentInsetAdjustmentBehavior:(UIScrollViewContentInsetAdjustmentBehavior)behavior
Expand Down