Introducing DrawerKit #2
Conversation
…d some refactoring.
wltrup
requested review from
diegopetrucci,
RuiAAPeres,
dmcrodrigues,
zzcgumn,
viorel15,
sergdort,
AnnKatF and
andersio
|
Here's the README as it will actually look like when live. The badges at the top still need fixing since this isn't live yet. |
DrawerKit.podspec
Outdated
| s.author = { "Wagner Truppel" => "wagner.truppel@babylonhealth.com" } | ||
| s.platform = :ios, "10.3" | ||
| s.source = { :git => "github.com/Babylonpartners/DrawerKit.git", :tag => "#{s.version}" } | ||
| s.source_files = "Classes/**/*.{swift}" |
There was a problem hiding this comment.
Podspec needs an update. Please loosen the deployment target, and update the author field too. Possibly ios.development@babylonhealth.com? @RuiAAPeres
There was a problem hiding this comment.
I see your point about updating the author and I agree with it but, if I'm not mistaken, ios.development@babylonhealth.com is the email address for our development account with ITC. Perhaps ios-developers@babylonhealth.com instead?
There was a problem hiding this comment.
In fact, I'll make that change anyway.
…pod lib lint the pod.
| func animateTransition(using transitionContext: UIViewControllerContextTransitioning) { | ||
| let key = (isPresentation ? | ||
| UITransitionContextViewControllerKey.to : | ||
| UITransitionContextViewControllerKey.from) |
There was a problem hiding this comment.
Because I find them to help readability, especially in cases like this where the contents are split between multiple lines.
| public func interactionControllerForPresentation(using animator: UIViewControllerAnimatedTransitioning) -> UIViewControllerInteractiveTransitioning? { | ||
| guard isDrawerDraggable else { return nil } | ||
| return InteractionController(isPresentation: true, | ||
| presentingVC: presentingVC!, presentedVC: presentedVC) |
There was a problem hiding this comment.
Is there a way to avoid the IUO? Also presentedVC: presentedVC) could be on a new line
| } | ||
| } | ||
|
|
||
| // TODO: Implement support for adaptive presentations. |
There was a problem hiding this comment.
Could you add this as an Issue (or whatever you prefer, TODOs in code are usually easily forgotten).
There was a problem hiding this comment.
Done. Check them here.
…on animation doesn't complete, therefore not invoking viewDidAppear, which causes the drawer not to show up at all. The work-around means that the initial presentation and dismissal aren't interactive but Product signed off on that decision. The drawers work fine under iOS 11.
|
Can you add a short description on how to use the API? I appreciate that the changes in #3 might make it obsolete so feel free to ignore this. |
Already there, in the README. |
| public let configuration: DrawerConfiguration | ||
|
|
||
| weak var presentingVC: (UIViewController & DrawerPresenting)? | ||
| /* strong */ var presentedVC: (UIViewController & DrawerPresentable) |
There was a problem hiding this comment.
/* strong */ what's the purpose of this?
There was a problem hiding this comment.
It's a way to say "this is a strong property and it's intentional, not an accident". There is no "strong" qualifier so I commented it.
| pod 'DrawerKit', '~> 0.0.1' | ||
| ``` | ||
|
|
||
| [CocoaPods]: https://cocoapods.org/ |
There was a problem hiding this comment.
Sure, but that's not what @diegopetrucci asked. I personally do not have any love for Carthage. 😉
There was a problem hiding this comment.
why? Conceptually it's simpler than cocoapods and less intrusive.
There was a problem hiding this comment.
You just need to drop a line and say it supports Carthage, and ensures the Xcode framework target is correctly configured. This framework has no third-party dependency i.e. no need for a Cartfile.
…xcuse to turn CI off since DrawerKit fails it for lack of configuration for testing.
This PR introduces our implementation of "drawer views" that behave similarly to those in the Apple Maps app.
The architecture is based on a custom presentation to present and dismiss the drawer view, coupled with a pan gesture recogniser to support dragging the drawer up and down. There is also a tap recogniser to dismiss the drawer when the user taps outside of it.
There are several configuration parameters and you're urged to play with the demo app to explore them.Speaking of the demo app, it's still incomplete in terms of supporting some of those configurations, although the library itself does support them. For instance, the library lets you set which animation timing curve to use but the demo app doesn't support that yet.I've since simplified the demo app to its bare minimum. In order to play with different configuration parameters, you'll need to change the code, recompile, rebuild, and rerun the app.