Repository navigation
fix SIMULATE_WEB_RESPONSE not imported #449 - #450
Conversation
|
|
nguyenhuy
left a comment
nguyenhuy
left a comment
There was a problem hiding this comment.
Thanks for contributing, @wsdwsd0829! Please change the file license as suggested by our bot and other small stuffs.
| [strongSelf->_collectionNode performBatchUpdates:^{ | ||
| [strongSelf->_collectionNode insertSections:[[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, 100)]]; | ||
| } completion:nil]; | ||
| NSLog(@"ViewController finished updating collectionView"); |
There was a problem hiding this comment.
Please update the comments here and a couple of lines below to use "collectionNode"
There was a problem hiding this comment.
with SIMULATE_WEB_RESPONSE, compiler will give error saying collectionView undefined, this change will follow the property to use collectionNode
There was a problem hiding this comment.
Yes, I understand the change here. I was suggesting to update the logs to use "collectionNode":
NSLog(@"ViewController finished updating collectionNode") and NSLog(@"ViewController is nil - won't update collectionNode"). Sorry for the confusion :)
There was a problem hiding this comment.
Actually, if we keep using 2s it will cause crash at L115 ASCollectionLayout, because poping VC and layout happen at same time; So I keep 4.
- (CGSize)collectionViewContentSize
{
...
ASDisplayNodeAssertNotNil(_layout, @"Collection layout state should not be nil at this point")
...
Let me know if it's potential bug and how to fix it. Thanks.
|
|
||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), mockWebService); | ||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ | ||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ |
There was a problem hiding this comment.
While I'm not opposing to the change at this line, I'm interested in your reasons.
There was a problem hiding this comment.
I do not know why need last dispatch, current behavior is when click push, the collection node/view will display after 2 second, then pop after another 2s (thus 4 total)
Actually I think we do not need to pop automatically, this change just try to make minimal change
There was a problem hiding this comment.
To make minimal change, let's revert the change here (i.e change it back to 4 seconds).
|
|
||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), mockWebService); | ||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ | ||
| dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ |
There was a problem hiding this comment.
To make minimal change, let's revert the change here (i.e change it back to 4 seconds).
| [strongSelf->_collectionNode performBatchUpdates:^{ | ||
| [strongSelf->_collectionNode insertSections:[[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, 100)]]; | ||
| } completion:nil]; | ||
| NSLog(@"ViewController finished updating collectionView"); |
There was a problem hiding this comment.
Yes, I understand the change here. I was suggesting to update the logs to use "collectionNode":
NSLog(@"ViewController finished updating collectionNode") and NSLog(@"ViewController is nil - won't update collectionNode"). Sorry for the confusion :)
nguyenhuy
left a comment
nguyenhuy
left a comment
There was a problem hiding this comment.
LGTM now. Thank you for the contribution!
* fix SIMULATE_WEB_RESPONSE not imported (Reported in TextureGroup#449). * changes per review * Update license of ViewController.m
3 participants
I could not found the correct format for "license as suggested by our bot", what should I use exactly?