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

[ASCellNode] Support for wrapping UIViewControllers#931

Merged
appleguy merged 6 commits intofacebookarchive:masterfrom
lappp9:cells-with-view-controllers
Dec 22, 2015
Merged

[ASCellNode] Support for wrapping UIViewControllers#931
appleguy merged 6 commits intofacebookarchive:masterfrom
lappp9:cells-with-view-controllers

Conversation

@lappp9
Copy link
Copy Markdown
Contributor

@lappp9 lappp9 commented Dec 10, 2015

This is a bit of a work in progress, but I'm taking a crack and initializing an ASCellNode with a UIViewControllerBlock. This example image shows the collection view example except the views shown in the cells are generated from an ImageViewController that has an imageView as a subview.

Right now it grabs the vc's view, makes a subnode out of it and adds it as a subnode of the cell. I'm resizing the vc's view based on what comes through in layoutSpecThatFits but that might be a little off or not the right way to do it.

Random thought, would it be smart to take the vc's view, calculate the aspect ratio of the size that was calculated for the cell by layoutSpecThatFits and then apply a transform to that view to make it fit in that aspect ratio?

Also touch handling is lost right now but I assume you could forward the taps to the saved vc if there is one?

@lappp9
Copy link
Copy Markdown
Contributor Author

lappp9 commented Dec 10, 2015

Also the sample app is called CollectionViewWithViewControllerCells btw

@appleguy
Copy link
Copy Markdown
Contributor

@lappp9 awesome head start on this feature - a highly desired one for a number of use cases, including @levi's ASPagerNode! Will review / give feedback soon.

@appleguy appleguy changed the title Cells with view controllers [ASCellNode] Support for wrapping UIViewControllers Dec 11, 2015
@appleguy appleguy added this to the 2.0 milestone Dec 11, 2015
@lappp9
Copy link
Copy Markdown
Contributor Author

lappp9 commented Dec 11, 2015

@appleguy thanks/no problem!

Side note: I noticed that viewWillAppear and viewDidAppear seemed to already work on the view controllers I threw in there.

Also, I think this way of resizing the view controller's views makes sense since it's similar to the way Apple talks about container view controllers resizing the views of the view controllers they contain.

@lappp9
Copy link
Copy Markdown
Contributor Author

lappp9 commented Dec 12, 2015

Oh also, View Controllers do have a preferredContentSize that's intended to be used for popovers. If we want to go the other direction and have the view controllers inform the cell size I assume we could use that?

https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIViewController_Class/#//apple_ref/occ/instp/UIViewController/preferredContentSize

Comment thread AsyncDisplayKit/ASCellNode.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to reduce API surface area, I think we should probably kill this variant. Passing nil as the didLoadBlock is pretty reasonable especially since this will not be a very common call in people's apps.

@appleguy
Copy link
Copy Markdown
Contributor

@lappp9 ok, I will land this tomorrow if you are able to do another round on feedback! I'd like to build off this patch this weekend with some customizations for ASViewController-based wrapping (setting interfaceState automatically etc), as I am currently adopting ASPagerNode in Pinterest and want it to page through ASVCs (but it currently expects ASCellNodes, so this is directionally what is necessary). Thanks for your work across the project, it is really cool!

@appleguy appleguy modified the milestones: 1.9.4, 2.0 Dec 19, 2015
@appleguy appleguy modified the milestones: 1.9.5, 1.9.4 Dec 20, 2015
@appleguy
Copy link
Copy Markdown
Contributor

Nice improvements! I'll go ahead and land this now, but it is /very/ important that we ensure the view controller as well as its view are not created until the -didLoad call.

I'll probably end up modifying this behavior for ASViewControllers though, so if you had to choose, feel free to focus on the video node first.

appleguy added a commit that referenced this pull request Dec 22, 2015
[ASCellNode] Support for wrapping UIViewControllers
@appleguy appleguy merged commit af9f8df into facebookarchive:master Dec 22, 2015
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.

3 participants