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

photosViewController:didDisplayPhoto:atIndex: is not called for the initial photo. #42

Closed
heikkihautala opened this issue Apr 10, 2015 · 3 comments
Labels

Comments

@heikkihautala
Copy link
Contributor

NYTPhotosViewControllerDelegate’s photosViewController:didDisplayPhoto:atIndex: is not called for the initial photo.

@cdzombak cdzombak added the bug label Apr 10, 2015
@bcapps
Copy link
Contributor

bcapps commented Apr 10, 2015

This was originally intended to reflect Apple's general approach with delegates like that, where the delegate is only called upon user interaction, not upon programmatic setting (which in this case would be programmatic setting of the initial photo or moveToPhoto:).

So, of course it can be changed based on what NYT thinks, but originally this was intended behavior.

@mbbischoff
Copy link
Contributor

I totally agree with @bcapps here. This is how nearly every UIKit delegate works and it’d be good to stay consistent. I recommend closing this issue.

cc @Twigz

@heikkihautala
Copy link
Contributor Author

Ha, I should have read the documentation of the method :). The method name was a little misleading. Maybe the method could be renamed to better describe the difference? For example "didMoveToPhoto"?

cdzombak added a commit that referenced this issue Apr 20, 2015
This more closely reflects that this delegate method/notification is tightly tied to user interactions.

closes #42
@Twigz Twigz closed this as completed in #47 May 1, 2015
ignazioc pushed a commit to technology-ebay-de/NYTPhotoViewer that referenced this issue Jan 11, 2016
This more closely reflects that this delegate method/notification is tightly tied to user interactions.

closes nytimes#42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants