-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[ASPagerNode] Initial implementation of a paging-specific ASCollectionNode subclass. #925
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
// | ||
|
||
#import "ASCollectionNode.h" | ||
#import "ASDisplayNode+Subclasses.h" | ||
|
||
@implementation ASCollectionNode | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// | ||
// ASPagerNode.h | ||
// AsyncDisplayKit | ||
// | ||
// Created by Levi McCallum on 12/7/15. | ||
// Copyright © 2015 Facebook. All rights reserved. | ||
// | ||
|
||
#import <AsyncDisplayKit/ASCollectionNode.h> | ||
|
||
@protocol ASPagerNodeDataSource; | ||
|
||
@interface ASPagerNode : ASCollectionNode | ||
|
||
@property (weak, nonatomic) id<ASPagerNodeDataSource> dataSource; | ||
|
||
- (void)reloadData; | ||
|
||
- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType; | ||
|
||
@end | ||
|
||
@protocol ASPagerNodeDataSource <NSObject> | ||
|
||
- (NSInteger)numberOfPagesInPagerNode:(ASPagerNode *)pagerNode; | ||
|
||
- (ASCellNode *)pagerNode:(ASPagerNode *)pagerNode nodeAtIndex:(NSInteger)index; | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
// | ||
// ASPagerNode.m | ||
// AsyncDisplayKit | ||
// | ||
// Created by Levi McCallum on 12/7/15. | ||
// Copyright © 2015 Facebook. All rights reserved. | ||
// | ||
|
||
#import "ASPagerNode.h" | ||
|
||
#import <AsyncDisplayKit/AsyncDisplayKit.h> | ||
|
||
@interface ASPagerNode () <ASCollectionViewDataSource, ASCollectionViewDelegateFlowLayout> { | ||
UICollectionViewFlowLayout *_flowLayout; | ||
} | ||
|
||
@end | ||
|
||
@implementation ASPagerNode | ||
|
||
- (instancetype)init | ||
{ | ||
_flowLayout = [[UICollectionViewFlowLayout alloc] init]; | ||
_flowLayout.scrollDirection = UICollectionViewScrollDirectionHorizontal; | ||
_flowLayout.minimumInteritemSpacing = 0; | ||
_flowLayout.minimumLineSpacing = 0; | ||
|
||
self = [super initWithCollectionViewLayout:_flowLayout]; | ||
if (self != nil) { | ||
self.view.asyncDataSource = self; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is essential that we set these in -didLoad, not in -init. Some big perf problems in Pinterest were addressed by ensuring we didn't poke the creation of ASCollectionViews too early. |
||
self.view.asyncDelegate = self; | ||
|
||
self.view.pagingEnabled = YES; | ||
self.view.allowsSelection = NO; | ||
self.view.showsVerticalScrollIndicator = NO; | ||
self.view.showsHorizontalScrollIndicator = NO; | ||
|
||
ASRangeTuningParameters tuningParams = { .leadingBufferScreenfuls = 1.0, .trailingBufferScreenfuls = 1.0 }; | ||
[self setTuningParameters:tuningParams forRangeType:ASLayoutRangeTypePreload]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, good idea to have new defaults for these. However, please set the Preload range to 2.0 both leading and trailing. |
||
[self setTuningParameters:tuningParams forRangeType:ASLayoutRangeTypeRender]; | ||
} | ||
return self; | ||
} | ||
|
||
- (void)reloadData | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe just implement this on ASCollectionNode? Redeclaring it in the header for clarity so people don't have to look at the superclass is fine for now, but we should make sure that anything relevant to ASCollectionNode is put there. Same is probably true for setTuningParameters. |
||
{ | ||
[self.view reloadData]; | ||
} | ||
|
||
- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not obvious that this will trigger view creation. Suggest putting it on ASCollectionNode - and creating a new data type similar to ASPendingViewState that both ASCollectionNode and/or ASTableNode (possibly different data types for each) can use to store properties like the delegate / dataSource / tuningParameters before the view is created, and apply them in -didLoad. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Totally agree with this suggestion. I think we need to introduce a broader data type which allows any custom node to defer application of a value until the view has loaded. I think we could go about this by upgrading ASPendingViewState to support custom properties. |
||
{ | ||
[self.view setTuningParameters:tuningParameters forRangeType:rangeType]; | ||
} | ||
|
||
#pragma mark - ASCollectionViewDataSource | ||
|
||
- (ASCellNode *)collectionView:(ASCollectionView *)collectionView nodeForItemAtIndexPath:(NSIndexPath *)indexPath | ||
{ | ||
ASDisplayNodeAssert(self.dataSource != nil, @"ASPagerNode must have a data source to load paging nodes"); | ||
return [self.dataSource pagerNode:self nodeAtIndex:indexPath.item]; | ||
} | ||
|
||
- (NSInteger)collectionView:(UICollectionView *)collectionView numberOfItemsInSection:(NSInteger)section | ||
{ | ||
ASDisplayNodeAssert(self.dataSource != nil, @"ASPagerNode must have a data source to load paging nodes"); | ||
return [self.dataSource numberOfPagesInPagerNode:self]; | ||
} | ||
|
||
- (ASSizeRange)collectionView:(ASCollectionView *)collectionView constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath | ||
{ | ||
return ASSizeRangeMake(CGSizeZero, self.view.bounds.size); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might actually make more sense for the minimum size to also be the bounds size, because we probably don't want pages laying out at a size less than the page, even if they have to flex to add more space. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to keep the restriction of having one node per a page a loose one. With an undefined minimum size, it keeps the possibility open of showing pages that have multiple nodes. |
||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
source 'https://github.com/CocoaPods/Specs.git' | ||
platform :ios, '8.0' | ||
pod 'AsyncDisplayKit', :path => '../..' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably wanna expose things like this so people can get spacing between pages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely agreed. Next changes!