Skip to content
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 empty ASViewController initializer to facilitate subclassing #1754

Merged
merged 1 commit into from
Jan 3, 2020
Merged

Add empty ASViewController initializer to facilitate subclassing #1754

merged 1 commit into from
Jan 3, 2020

Conversation

christianselig
Copy link
Contributor

[Take 2 of #1740 because I'm an idiot]

Per Slack discussion with @nguyenhuy.

If you want to use ASViewController throughout your app as a generalized UIViewController stand in (as the docs mention) it's tricky (at least in Swift) when you want to take a non-node based normal UIViewController and make it inherit from ASViewController (again to perhaps have a consistent root subclass) because there's only one designated initializer in ASViewController, init(node:) which does not accept nil values despite the docs indicating that should be okay.

Currently the only course of action is to pass in a dummy node (e.g.: ASViewController(node: ASDisplayNode())). Other courses of action include unmarking init(node:) as the designated initializer or letting it accept nil values, but neither of those are optimal, per @nguyhuy:

ah, you only hit this limitation while subclassing. When people initialize it directly, we want to steer them to do the right thing by making the node param nonnull.

A proposed solution would be to introduce a second designated "empty" initializer that would just use the underlying UIViewController, make node nil, and facilitate subclassing, which is what I did in this PR.

Other context:

@jparise
Copy link
Member

jparise commented Jan 2, 2020

@christianselig looks like another Swift examples needs to be updated:

/Users/runner/runners/2.163.1/work/Texture/Texture/examples/CustomCollectionView-Swift/Sample/ViewController.swift:20:3: overriding declaration requires an 'override' keyword

@christianselig
Copy link
Contributor Author

@jparise Fixed too :)

@jparise
Copy link
Member

jparise commented Jan 3, 2020

@christianselig One more. 😜

/Users/runner/runners/2.163.1/work/Texture/Texture/examples/LayoutSpecExamples-Swift/Sample/OverviewViewController.swift:16:3: overriding declaration requires an 'override' keyword

@christianselig
Copy link
Contributor Author

@jparise Done haha

Copy link
Member

@jparise jparise left a comment

Choose a reason for hiding this comment

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

Wonderful. Thank you!

@jparise jparise merged commit 80ffcdf into TextureGroup:master Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants