[ASCollectionView] Tuning parameters not set #1820
Conversation
|
Wow great catch, and thanks for adding the unit tests to go with it. |
|
@gazreese thanks for fixing this. @Adlai-Holler this is my fault, a vestige of prior attempts to move logic into ASCollection/TableNode instead of it living in the Views, so that we could create a shared superclass like ASRangeNode. Soon after, it was discovered that my tactics to break the retain cycle when a directly-allocated ASCollectionView instance made its own ASCollectionNode did not work correctly, and so some of this was backpedalled. I recall being quite careful about this, but clearly missed a key area. Could someone check ASTableView too? |
|
@appleguy @Adlai-Holler |
| @@ -411,22 +411,22 @@ - (void)setAsyncDelegate:(id<ASCollectionViewDelegate>)asyncDelegate | |||
|
|
|||
| - (void)setTuningParameters:(ASRangeTuningParameters)tuningParameters forRangeType:(ASLayoutRangeType)rangeType | |||
| { | |||
| [_collectionNode setTuningParameters:tuningParameters forRangeType:rangeType]; | |||
| [_rangeController setTuningParameters:tuningParameters forRangeMode:ASLayoutRangeModeFull rangeType:rangeType]; | |||
There was a problem hiding this comment.
To align with ASTableView we should either change this in ASCollectionView to [_layoutController setTuningParameters:...] as in ASTableView or change ASTableView to use the range controller via [_rangeController setTuningParameters:...]
I've been trying to use the tuning parameters in a project and I noticed that they were having no effect. This appears to be due to the tuning parameter methods on ASCollectionView being passed straight through to the _collectionNode member.
The _collectionNode member doesn't appear to actually get set anywhere, and it would only pass these through to the range controller anyway. Passing these directly through seems to fix the problem, and I can see on my own project that the changes are having the desired affect.
The use of _collectionNode throughout this class does seem to be in need of a review, but I'm not that well up on the code to dive in myself unfortunately.
The get/set of the tuning parameters is quite easy to unit test, so I've covered this already. There might be more in-depth tests possible but I'm not sure how really if I'm honest.