Skip to content

Fix typos (RCTCustomRefreshControlProtocol) #36363

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 2 commits into from
Closed

Fix typos (RCTCustomRefreshControlProtocol) #36363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79d76fe
Fix typos (RCTCustomRefreshControlProtocol)
jiggag Mar 3, 2023
a8fee68
Add deprecated protocol
jiggag Mar 13, 2023
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: 1 addition & 1 deletion React/Views/RefreshControl/RCTRefreshControl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#import <React/RCTComponent.h>
#import <React/RCTScrollableProtocol.h>

@interface RCTRefreshControl : UIRefreshControl <RCTCustomRefreshContolProtocol>
@interface RCTRefreshControl : UIRefreshControl <RCTCustomRefreshControlProtocol>

@property (nonatomic, copy) NSString *title;
@property (nonatomic, copy) RCTDirectEventBlock onRefresh;
Expand Down
14 changes: 7 additions & 7 deletions React/Views/ScrollView/RCTScrollView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
@interface RCTCustomScrollView : UIScrollView <UIGestureRecognizerDelegate>

@property (nonatomic, assign) BOOL centerContent;
@property (nonatomic, strong) UIView<RCTCustomRefreshContolProtocol> *customRefreshControl;
@property (nonatomic, strong) UIView<RCTCustomRefreshControlProtocol> *customRefreshControl;
@property (nonatomic, assign) BOOL pinchGestureEnabled;

@end
Expand Down Expand Up @@ -215,14 +215,14 @@ - (void)setFrame:(CGRect)frame
}
}

- (void)setCustomRefreshControl:(UIView<RCTCustomRefreshContolProtocol> *)refreshControl
- (void)setCustomRefreshControl:(UIView<RCTCustomRefreshControlProtocol> *)refreshControl
{
if (_customRefreshControl) {
[_customRefreshControl removeFromSuperview];
}
_customRefreshControl = refreshControl;
// We have to set this because we can't always guarantee the
// `RCTCustomRefreshContolProtocol`'s superview will always be of class
// `RCTCustomRefreshControlProtocol`'s superview will always be of class
// `UIScrollView` like we were previously
if ([_customRefreshControl respondsToSelector:@selector(setScrollView:)]) {
_customRefreshControl.scrollView = self;
Expand Down Expand Up @@ -412,8 +412,8 @@ - (void)setRemoveClippedSubviews:(__unused BOOL)removeClippedSubviews
- (void)insertReactSubview:(UIView *)view atIndex:(NSInteger)atIndex
{
[super insertReactSubview:view atIndex:atIndex];
if ([view conformsToProtocol:@protocol(RCTCustomRefreshContolProtocol)]) {
[_scrollView setCustomRefreshControl:(UIView<RCTCustomRefreshContolProtocol> *)view];
if ([view conformsToProtocol:@protocol(RCTCustomRefreshControlProtocol)]) {
[_scrollView setCustomRefreshControl:(UIView<RCTCustomRefreshControlProtocol> *)view];
if (![view isKindOfClass:[UIRefreshControl class]] && [view conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
[self addScrollListener:(UIView<UIScrollViewDelegate> *)view];
}
Expand All @@ -431,7 +431,7 @@ - (void)insertReactSubview:(UIView *)view atIndex:(NSInteger)atIndex
- (void)removeReactSubview:(UIView *)subview
{
[super removeReactSubview:subview];
if ([subview conformsToProtocol:@protocol(RCTCustomRefreshContolProtocol)]) {
if ([subview conformsToProtocol:@protocol(RCTCustomRefreshControlProtocol)]) {
[_scrollView setCustomRefreshControl:nil];
if (![subview isKindOfClass:[UIRefreshControl class]] &&
[subview conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
Expand Down Expand Up @@ -486,7 +486,7 @@ - (void)layoutSubviews

#if !TARGET_OS_TV
// Adjust the refresh control frame if the scrollview layout changes.
UIView<RCTCustomRefreshContolProtocol> *refreshControl = _scrollView.customRefreshControl;
UIView<RCTCustomRefreshControlProtocol> *refreshControl = _scrollView.customRefreshControl;
if (refreshControl && refreshControl.isRefreshing && ![refreshControl isKindOfClass:UIRefreshControl.class]) {
refreshControl.frame =
(CGRect){_scrollView.contentOffset, {_scrollView.frame.size.width, refreshControl.frame.size.height}};
Expand Down
6 changes: 5 additions & 1 deletion React/Views/ScrollView/RCTScrollableProtocol.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@
/**
* Denotes a view which implements custom pull to refresh functionality.
*/
@protocol RCTCustomRefreshContolProtocol
@protocol RCTCustomRefreshControlProtocol
Copy link
Contributor

Choose a reason for hiding this comment

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

This is effectively a breaking change that adds really little value imho:
https://cs.github.com/?scopeName=All+repos&scope=&q=RCTCustomRefreshContolProtocol

I know the typo is not cool, but we can't just rename.
@cipolleschi what's your opinion on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops I was late 😅 saw your comment above.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I said above. I will rename it, but not from one day to another.
I'll let the two protocols live for a couple of versions, so that users can be prompted on updating the naming and then we can remove it in 0.74 or 0.75, for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cipolleschi I'm not sure if what you suggested is documented or not, but your suggestion above is very informative at least in the context of renaming public APIs.

Provided typo-fix PRs are frequent, I suggest documenting your suggestions above in the Contributing Guide. Glad to open a PR (on react-native-website) for adding those suggestions. Lmk :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Pranav-yadav! That would be amazing, although I don't have a clear idea on where it should go.

Probably in the Contributing section of the Website? 🤔
To contribute to the website, you can work on the website repo. ;)


@property (nonatomic, copy) RCTDirectEventBlock onRefresh;
@property (nonatomic, readonly, getter=isRefreshing) BOOL refreshing;

@optional
@property (nonatomic, weak) UIScrollView *scrollView;

@end

__attribute__ ((deprecated("Use RCTCustomRefreshControlProtocol instead")))
@protocol RCTCustomRefreshContolProtocol <RCTCustomRefreshControlProtocol>
@end