search

jdisho / Papr

🌁 An Unsplash app for iOS
Apache License 2.0
1.15k stars 149 forks source link

Feature/present interactive #72

Closed vaderdan closed 5 years ago

vaderdan commented 5 years ago

https://github.com/jdisho/Papr/issues/55

This pull makes possible interactively (with gesture) to close detail photo

I did some changes on Coordinator pop function. I made it cancelable (when dispose the observable do not set current controller)

jdisho commented 5 years ago

Thanks a lot for the PR. I will try to review asap 😊

Sent with GitHawk

vaderdan commented 5 years ago

Merry Christmas! I'm not in hurry

jdisho commented 5 years ago

Hey @vaderdan, I need some more information regarding the changes in pop(animated:). Why?

vaderdan commented 5 years ago

I needed to make pop cancelable, because of the nature of how the controllers are dismissed

we fire the dismiss(animated: true, completion: nil) Under the hood (this is how UIKit works) it starts the animation (which we modify the progress of animation under the hood with something like propertyAnimator.fractionComplete = gesture.percent) and if we cancel the animation it doesn't remove the current controller view from stack if we complete the animation it remove the current controller view

however pop function always thinks that the controller is removed from stack (and sets the parent controller as current) even if we cancel the controller animation transition

if the currentController is incorrect and we try to navigate app crashes (because of it tries to add controller to the currentController that is not visible on the screen or is not in the view stack)

vaderdan commented 5 years ago

I think that this is the most elegant way to do it, instead of adding additional logic inside the coordinator class. The PublishSubject is replaced with Observable because, if I subscribe on PublishSubject and then dispose subscription it won't notify the PublishSubject with completed