Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[ASCollectionNode] Add support for using UICollectionViewCells #2948

Closed
wants to merge 2 commits into from

Conversation

hannahmbanana
Copy link
Contributor

@hannahmbanana hannahmbanana commented Jan 29, 2017

Paired programming with @appleguy.

This method offers compatibility with synchronous, standard UICollectionViewCell objects. These cells will not have the performance benefits of ASCellNodes (like preloading, async layout, and async drawing) - even when mixed within the same ASCollectionNode.

In order to use this method, you must:

  1. Implement it on your ASCollectionDataSource object.
  2. Call registerCellClass: on the collectionNode.view (in viewDidLoad, or register an onDidLoad: block).
  3. Return nil from the nodeBlockForItem...: or nodeForItem...: method. NOTE: it is an error to return nil from within a nodeBlock, if you have returned a nodeBlock object.
  4. Lastly, you must implement a method to provide the size for the cell. There are two ways this is done:
    4a. UICollectionViewFlowLayout (incl. ASPagerNode). Implement collectionNode:constrainedSizeForItemAtIndexPath:.
    4b. Custom collection layouts. Set .view.layoutInspector and have it implement collectionView:constrainedSizeForNodeAtIndexPath:.

For an example of using this method with all steps above (including a custom layout, 4b.), see the app in examples/CustomCollectionView and enable kShowUICollectionViewCells = YES (example project included in this PR).

@hannahmbanana hannahmbanana changed the title UICollectionViewCell compatibility [ASCollectionNode] Add support for using UICollectionViewCells Jan 29, 2017

if (!AS_AT_LEAST_IOS8) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getting rid of this because we're dropping iOS 7 support

return cell;
}

- (void)collectionView:(UICollectionView *)collectionView willDisplayCell:(_ASCollectionViewCell *)cell forItemAtIndexPath:(NSIndexPath *)indexPath
{
// Since _ASCollectionViewCell is not available for subclassing, this is faster than isKindOfClass:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way to do this?

@hannahmbanana
Copy link
Contributor Author

cc @KieranLafferty

Copy link
Contributor

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

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

We should revisit this after/in light of #2942 which has a lot of these same hooks.

@QuynhNguyen
Copy link

Just wondering, what are we getting out of this? From the description, it seems like using ASCellNode over UICollectionViewCell is a no brainer.

@hannahmbanana
Copy link
Contributor Author

@QuynhNguyen - great question!

ASCellNode is always preferred. However, for code bases that have hundreds or more custom UICollectionViewCells subclasses, this feature will allow them to port to ASDK more easily, and then slowly convert their backlog of cells to be ASCellNode subclasses.

This change allows mixing both those un-ported UICollectionViewCells and ASCellNodes without a performance penalty for the UIKitCells. In addition, it still provides of ASCellNode, including automatic preload, asynchronous measurement and asynchronous rendering. Because of this difference in performance, app developers will clearly see the value of porting cells to ASCellNode and be able to do so over time.

That said, I think @Adlai-Holler implemented this feature in #2942 so we may not need this PR anymore.

@Adlai-Holler
Copy link
Contributor

I'm closing this PR – supporting this use case will be much different in light of #2942 so let's take a fresh stab at it. If you disagree and want to fix forward from this PR, feel free to reopen.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants