Skip to content
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.

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

Remove necessity to use view to access rangeController in ASTableNode, ASCollectionNode #1103

Merged
merged 1 commit into from
Sep 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
- [ASPINRemoteImageManager] Add a new API for setting a preconfigured PINRemoteImageManager. [Ernest Ma](https://github.com/ernestmama) [#1124](https://github.com/TextureGroup/Texture/pull/1124)
- Small optimization to the layout spec & yoga layout systems by eliminating array copies. [Adlai Holler](https://github.com/Adlai-Holler)
- Optimize layout process by removing `ASRectMap`. [Adlai Holler](https://github.com/Adlai-Holler)
- Remove necessity to use view to access rangeController in ASTableNode, ASCollectionNode. [Michael Schneider](https://github.com/maicki)

## 2.7
- Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)
Expand Down
78 changes: 44 additions & 34 deletions Source/ASCollectionNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,20 @@

#pragma mark - _ASCollectionPendingState

@interface _ASCollectionPendingState : NSObject
@property (weak, nonatomic) id <ASCollectionDelegate> delegate;
@property (weak, nonatomic) id <ASCollectionDataSource> dataSource;
@interface _ASCollectionPendingState : NSObject {
@public
std::vector<std::vector<ASRangeTuningParameters>> _tuningParameters;
Copy link
Member

Choose a reason for hiding this comment

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

Not specific to this diff, but we should probably use ASRangeTuningParameters[ASLayoutRangeModeCount][ASLayoutRangeTypeCount] _tuningParameters here so we get static allocation.

}
@property (nonatomic, weak) id <ASCollectionDelegate> delegate;
@property (nonatomic, weak) id <ASCollectionDataSource> dataSource;
@property (nonatomic) UICollectionViewLayout *collectionViewLayout;
@property (nonatomic) ASLayoutRangeMode rangeMode;
@property (nonatomic) BOOL allowsSelection; // default is YES
@property (nonatomic) BOOL allowsMultipleSelection; // default is NO
@property (nonatomic) BOOL inverted; //default is NO
@property (nonatomic) BOOL usesSynchronousDataLoading;
@property (nonatomic) CGFloat leadingScreensForBatching;
@property (weak, nonatomic) id <ASCollectionViewLayoutInspecting> layoutInspector;
@property (nonatomic, weak) id <ASCollectionViewLayoutInspecting> layoutInspector;
@property (nonatomic) BOOL alwaysBounceVertical;
@property (nonatomic) BOOL alwaysBounceHorizontal;
@property (nonatomic) UIEdgeInsets contentInset;
Expand All @@ -52,11 +55,14 @@ @interface _ASCollectionPendingState : NSObject

@implementation _ASCollectionPendingState

#pragma mark Lifecycle

- (instancetype)init
{
self = [super init];
if (self) {
_rangeMode = ASLayoutRangeModeUnspecified;
_tuningParameters = std::vector<std::vector<ASRangeTuningParameters>> (ASLayoutRangeModeCount, std::vector<ASRangeTuningParameters> (ASLayoutRangeTypeCount, ASRangeTuningParametersZero));
_allowsSelection = YES;
_allowsMultipleSelection = NO;
_inverted = NO;
Expand All @@ -66,23 +72,8 @@ - (instancetype)init
}
return self;
}
@end

// TODO: Add support for tuning parameters in the pending state
#if 0 // This is not used yet, but will provide a way to avoid creating the view to set range values.
@implementation _ASCollectionPendingState {
std::vector<std::vector<ASRangeTuningParameters>> _tuningParameters;
}

- (instancetype)init
{
self = [super init];
if (self) {
_tuningParameters = std::vector<std::vector<ASRangeTuningParameters>> (ASLayoutRangeModeCount, std::vector<ASRangeTuningParameters> (ASLayoutRangeTypeCount));
_rangeMode = ASLayoutRangeModeUnspecified;
}
return self;
}
#pragma mark Tuning Parameters

- (ASRangeTuningParameters)tuningParametersForRangeType:(ASLayoutRangeType)rangeType
{
Expand All @@ -107,7 +98,6 @@ - (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeMo
}

@end
#endif

#pragma mark - ASCollectionNode

Expand All @@ -118,6 +108,7 @@ @interface ASCollectionNode ()
id<ASBatchFetchingDelegate> _batchFetchingDelegate;
}
@property (nonatomic) _ASCollectionPendingState *pendingState;
@property (nonatomic, weak) ASRangeController *rangeController;
@end

@implementation ASCollectionNode
Expand Down Expand Up @@ -188,6 +179,8 @@ - (void)didLoad

ASCollectionView *view = self.view;
view.collectionNode = self;

_rangeController = view.rangeController;

if (_pendingState) {
_ASCollectionPendingState *pendingState = _pendingState;
Expand Down Expand Up @@ -219,9 +212,24 @@ - (void)didLoad
if (!CGPointEqualToPoint(contentOffset, CGPointZero)) {
[view setContentOffset:contentOffset animated:pendingState.animatesContentOffset];
}


let tuningParametersVector = pendingState->_tuningParameters;
let tuningParametersVectorSize = tuningParametersVector.size();
for (NSInteger rangeMode = 0; rangeMode < tuningParametersVectorSize; rangeMode++) {
let tuningparametersRangeModeVector = tuningParametersVector[rangeMode];
let tuningParametersVectorRangeModeSize = tuningparametersRangeModeVector.size();
for (NSInteger rangeType = 0; rangeType < tuningParametersVectorRangeModeSize; rangeType++) {
ASRangeTuningParameters tuningParameters = tuningparametersRangeModeVector[rangeType];
if (!ASRangeTuningParametersEqualToRangeTuningParameters(tuningParameters, ASRangeTuningParametersZero)) {
[_rangeController setTuningParameters:tuningParameters
forRangeMode:(ASLayoutRangeMode)rangeMode
rangeType:(ASLayoutRangeType)rangeType];
}
}
}

if (pendingState.rangeMode != ASLayoutRangeModeUnspecified) {
[view.rangeController updateCurrentRangeWithMode:pendingState.rangeMode];
[_rangeController updateCurrentRangeWithMode:pendingState.rangeMode];
}

// Don't need to set collectionViewLayout to the view as the layout was already used to init the view in view block.
Expand Down Expand Up @@ -253,7 +261,7 @@ - (void)didEnterPreloadState
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
// TODO (ASCL) If this node supports async layout, kick off the initial data load without allocating the view
if (ASHierarchyStateIncludesRangeManaged(self.hierarchyState) && CGRectEqualToRect(self.bounds, CGRectZero) == NO) {
[[self view] layoutIfNeeded];
[self.view layoutIfNeeded];
}
}

Expand Down Expand Up @@ -285,12 +293,6 @@ - (ASDataController *)dataController
return self.view.dataController;
}

// TODO: Implement this without the view.
- (ASRangeController *)rangeController
{
return self.view.rangeController;
}

- (_ASCollectionPendingState *)pendingState
{
if (!_pendingState && ![self isNodeLoaded]) {
Expand Down Expand Up @@ -647,22 +649,30 @@ - (void)setUsesSynchronousDataLoading:(BOOL)usesSynchronousDataLoading

- (ASRangeTuningParameters)tuningParametersForRangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController tuningParametersForRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
return [self tuningParametersForRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType
{
[self.rangeController setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
[self setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (ASRangeTuningParameters)tuningParametersForRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController tuningParametersForRangeMode:rangeMode rangeType:rangeType];
if ([self pendingState]) {
return [_pendingState tuningParametersForRangeMode:rangeMode rangeType:rangeType];
} else {
return [self.rangeController tuningParametersForRangeMode:rangeMode rangeType:rangeType];
}
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
if ([self pendingState]) {
[_pendingState setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
} else {
return [self.rangeController setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
}
}

#pragma mark - Selection
Expand Down
4 changes: 2 additions & 2 deletions Source/ASTableNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ NS_ASSUME_NONNULL_BEGIN
@property (readonly) ASTableView *view;

// These properties can be set without triggering the view to be created, so it's fine to set them in -init.
@property (nullable, weak, nonatomic) id <ASTableDelegate> delegate;
@property (nullable, weak, nonatomic) id <ASTableDataSource> dataSource;
@property (nonatomic, weak) id <ASTableDelegate> delegate;
@property (nonatomic, weak) id <ASTableDataSource> dataSource;

/**
* The number of screens left to scroll before the delegate -tableNode:beginBatchFetchingWithContext: is called.
Expand Down
84 changes: 68 additions & 16 deletions Source/ASTableNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@

#pragma mark - _ASTablePendingState

@interface _ASTablePendingState : NSObject
@property (weak, nonatomic) id <ASTableDelegate> delegate;
@property (weak, nonatomic) id <ASTableDataSource> dataSource;
@interface _ASTablePendingState : NSObject {
@public
std::vector<std::vector<ASRangeTuningParameters>> _tuningParameters;
}
@property (nonatomic, weak) id <ASTableDelegate> delegate;
@property (nonatomic, weak) id <ASTableDataSource> dataSource;
@property (nonatomic) ASLayoutRangeMode rangeMode;
@property (nonatomic) BOOL allowsSelection;
@property (nonatomic) BOOL allowsSelectionDuringEditing;
Expand All @@ -39,14 +42,19 @@ @interface _ASTablePendingState : NSObject
@property (nonatomic) CGPoint contentOffset;
@property (nonatomic) BOOL animatesContentOffset;
@property (nonatomic) BOOL automaticallyAdjustsContentOffset;

@end

@implementation _ASTablePendingState

#pragma mark - Lifecycle

- (instancetype)init
{
self = [super init];
if (self) {
_rangeMode = ASLayoutRangeModeUnspecified;
_tuningParameters = std::vector<std::vector<ASRangeTuningParameters>> (ASLayoutRangeModeCount, std::vector<ASRangeTuningParameters> (ASLayoutRangeTypeCount, ASRangeTuningParametersZero));
_allowsSelection = YES;
_allowsSelectionDuringEditing = NO;
_allowsMultipleSelection = NO;
Expand All @@ -61,6 +69,30 @@ - (instancetype)init
return self;
}

#pragma mark Tuning Parameters

- (ASRangeTuningParameters)tuningParametersForRangeType:(ASLayoutRangeType)rangeType
{
return [self tuningParametersForRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType
{
return [self setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (ASRangeTuningParameters)tuningParametersForRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
ASDisplayNodeAssert(rangeMode < _tuningParameters.size() && rangeType < _tuningParameters[rangeMode].size(), @"Requesting a range that is OOB for the configured tuning parameters");
return _tuningParameters[rangeMode][rangeType];
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
ASDisplayNodeAssert(rangeMode < _tuningParameters.size() && rangeType < _tuningParameters[rangeMode].size(), @"Setting a range that is OOB for the configured tuning parameters");
_tuningParameters[rangeMode][rangeType] = tuningParameters;
}

@end

#pragma mark - ASTableView
Expand All @@ -72,6 +104,7 @@ @interface ASTableNode ()
}

@property (nonatomic) _ASTablePendingState *pendingState;
@property (nonatomic, weak) ASRangeController *rangeController;
@end

@implementation ASTableNode
Expand Down Expand Up @@ -116,9 +149,11 @@ - (void)didLoad

ASTableView *view = self.view;
view.tableNode = self;

_rangeController = view.rangeController;

if (_pendingState) {
_ASTablePendingState *pendingState = _pendingState;
_ASTablePendingState *pendingState = _pendingState;
view.asyncDelegate = pendingState.delegate;
view.asyncDataSource = pendingState.dataSource;
view.inverted = pendingState.inverted;
Expand All @@ -129,8 +164,23 @@ - (void)didLoad
view.contentInset = pendingState.contentInset;
self.pendingState = nil;

let tuningParametersVector = pendingState->_tuningParameters;
let tuningParametersVectorSize = tuningParametersVector.size();
for (NSInteger rangeMode = 0; rangeMode < tuningParametersVectorSize; rangeMode++) {
let tuningparametersRangeModeVector = tuningParametersVector[rangeMode];
let tuningParametersVectorRangeModeSize = tuningparametersRangeModeVector.size();
for (NSInteger rangeType = 0; rangeType < tuningParametersVectorRangeModeSize; rangeType++) {
ASRangeTuningParameters tuningParameters = tuningparametersRangeModeVector[rangeType];
if (!ASRangeTuningParametersEqualToRangeTuningParameters(tuningParameters, ASRangeTuningParametersZero)) {
[_rangeController setTuningParameters:tuningParameters
forRangeMode:(ASLayoutRangeMode)rangeMode
rangeType:(ASLayoutRangeType)rangeType];
}
}
}

if (pendingState.rangeMode != ASLayoutRangeModeUnspecified) {
[view.rangeController updateCurrentRangeWithMode:pendingState.rangeMode];
[_rangeController updateCurrentRangeWithMode:pendingState.rangeMode];
}

[view setContentOffset:pendingState.contentOffset animated:pendingState.animatesContentOffset];
Expand Down Expand Up @@ -159,7 +209,7 @@ - (void)didEnterPreloadState
[super didEnterPreloadState];
// Intentionally allocate the view here and trigger a layout pass on it, which in turn will trigger the intial data load.
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
[[self view] layoutIfNeeded];
[self.view layoutIfNeeded];
}

#if ASRangeControllerLoggingEnabled
Expand Down Expand Up @@ -190,12 +240,6 @@ - (ASDataController *)dataController
return self.view.dataController;
}

// TODO: Implement this without the view.
- (ASRangeController *)rangeController
{
return self.view.rangeController;
}

- (_ASTablePendingState *)pendingState
{
if (!_pendingState && ![self isNodeLoaded]) {
Expand Down Expand Up @@ -476,22 +520,30 @@ - (void)updateCurrentRangeWithMode:(ASLayoutRangeMode)rangeMode

- (ASRangeTuningParameters)tuningParametersForRangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController tuningParametersForRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
return [self tuningParametersForRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType
{
[self.rangeController setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
[self setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType];
}

- (ASRangeTuningParameters)tuningParametersForRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController tuningParametersForRangeMode:rangeMode rangeType:rangeType];
if ([self pendingState]) {
return [_pendingState tuningParametersForRangeMode:rangeMode rangeType:rangeType];
} else {
return [self.rangeController tuningParametersForRangeMode:rangeMode rangeType:rangeType];
}
}

- (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeMode:(ASLayoutRangeMode)rangeMode rangeType:(ASLayoutRangeType)rangeType
{
return [self.rangeController setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
if ([self pendingState]) {
[_pendingState setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
} else {
return [self.rangeController setTuningParameters:tuningParameters forRangeMode:rangeMode rangeType:rangeType];
}
}

#pragma mark - Selection
Expand Down
11 changes: 11 additions & 0 deletions Tests/ASCollectionViewTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,17 @@ - (void)testTuningParameters
XCTAssertTrue(ASRangeTuningParametersEqualToRangeTuningParameters(preloadParams, [collectionView tuningParametersForRangeType:ASLayoutRangeTypePreload]));
}

// Informations to test: https://github.com/TextureGroup/Texture/issues/1094
- (void)testThatCollectionNodeCanHandleNilRangeController
{
UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc] init];
ASCollectionNode *collectionNode = [[ASCollectionNode alloc] initWithCollectionViewLayout:layout];
[collectionNode recursivelySetInterfaceState:ASInterfaceStateDisplay];
[collectionNode setHierarchyState:ASHierarchyStateRangeManaged];
[collectionNode recursivelySetInterfaceState:ASInterfaceStateNone];
ASCATransactionQueueWait(nil);
}

/**
* This may seem silly, but we had issues where the runtime sometimes wouldn't correctly report
* conformances declared on categories.
Expand Down