-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add first-pass view model support to collection node. #trivial #356
Conversation
@@ -207,6 +207,7 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe | |||
unsigned int collectionViewNumberOfItemsInSection:1; | |||
unsigned int collectionNodeNodeForItem:1; | |||
unsigned int collectionNodeNodeBlockForItem:1; | |||
unsigned int viewModelForItem:1; |
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.
No prefix needed for this method, since there's no collectionNode/collectionView
distinction this time.
🚫 CI failed with log |
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.
LGTM
Source/ASCellNode.h
Outdated
* | ||
* The view-model currently assigned to this node, if any. | ||
* | ||
* This property may be set off the main thread, but it will never be set in parallel. |
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.
Can you explain further what you mean "it will never be set in parallel"?
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.
Instead of annotating the new APIs as beta, shouldn't they be declared in +Beta headers (e.g ASCellNode+Beta, ASCollectionNode+Beta, ASCollectionBetaDataSource, etc)?
@nguyenhuy For these changes I'd rather have smaller, simpler diffs and I don't see much drawback to it. |
…reGroup#356) * Add first-pass view model support for collection node. Much more to come! * Address issues * Update the gorram license header * Dear lord
This is the first of a series of diffs I intend to land over the next couple of days, adding support for view-model backed cell nodes, and eventually optional view-model diffing to drive updates.
There's not much to this first pass. You can provide view models, and they'll get assigned to the cell node and made available by the collection node.
The next pass will add the idea of
-canUpdateToViewModel:
and skip fetching a new node block onreloadItemsAtIndexPaths:
if the existing cell node can update.I'll update the CHANGELOG when something more significant happens.