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

Presentation without targetViewController should be removed #19

Open
felixLam opened this issue Mar 5, 2014 · 3 comments
Open

Presentation without targetViewController should be removed #19

felixLam opened this issue Mar 5, 2014 · 3 comments

Comments

@felixLam
Copy link

felixLam commented Mar 5, 2014

Adding the view of a view controller to another view controllers view is not a good practice. We might consider deprecating the showImage... methods that lack the inViewController: argument. This also reduces the number of code-branches to support and is likely to ease the handling of interface orientation.

@felixLam
Copy link
Author

felixLam commented Mar 5, 2014

Internally they can still be used but can rely on the existence of a target view controller.

@u10int
Copy link
Owner

u10int commented Mar 5, 2014

In my testing using a target view controller did not always perform as expected, and in most cases one would want this media focus view to be displayed as a modal subview in the view stack of the application's keyWindow. Some of this could have to do with how iOS handles adding child view controllers via addChildViewController.

Also, it's perfectly acceptable to add one view controller's view to another view controller's view, especially if the presenting view is just a modal view on top of UIWindow which is what this control does if no target view controller is specified. The only issue with not using the addChildViewController approach Apple recommends for container controllers is that your child controller will not receive the necessary view event methods (e.g. viewWillAppear, viewDidAppear, etc), none of which this control depends on for setup. So technically this control isn't a true view controller in that sense.

I will leave this open as I'll revisit some of the weird issues with using a target view controller to see if that can be improved.

@felixLam
Copy link
Author

felixLam commented Mar 5, 2014

i have just finished a first draft of the interface rotation. It somewhat relies on the the rotation calls to update the snapshots.

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

No branches or pull requests

2 participants