Add config to disable dismissal of collapsed drawer #65
Conversation
georgmay
changed the title
|
I really want this too! |
| self.presentedViewController.dismiss(animated: true) | ||
| if !self.configuration.neverDismissCollapsedDrawer { | ||
| self.presentedViewController.dismiss(animated: true) | ||
| } |
There was a problem hiding this comment.
This looks strange. Why not add the condition directly to the local variable shouldDismiss? Also, I'd prefer not to have two negations, ie, to have something like canDismissCollapsedDrawer instead of !neverDismissCollapsedDrawer.
| @@ -15,7 +15,10 @@ extension PresentationController { | |||
| guard tapY < currentDrawerY else { return } | |||
| NotificationCenter.default.post(notification: DrawerNotification.drawerExteriorTapped) | |||
| tapGesture.isEnabled = false | |||
| presentedViewController.dismiss(animated: true) | |||
|
|
|||
| if !configuration.neverDismissCollapsedDrawer { | |||
There was a problem hiding this comment.
I'd prefer not to have two negations, ie, to have something like canDismissCollapsedDrawer instead of !neverDismissCollapsedDrawer.
| @@ -6,4 +6,8 @@ public protocol DrawerPresentable: class { | |||
| /// The height at which the drawer must be presented when it's in its | |||
| /// partially expanded state. If negative, its value is clamped to zero. | |||
| var heightOfPartiallyExpandedDrawer: CGFloat { get } | |||
|
|
|||
| /// The height at which the drawer should be presented when it's in its | |||
| /// collapsed expanded state. If negative, its value is clamped to zero. | |||
There was a problem hiding this comment.
collapsed expanded state makes no sense. It should be collapsed state.
| @@ -82,6 +82,9 @@ public struct DrawerConfiguration { | |||
| /// When `false`, the presentation is always to full screen and there is no | |||
| /// partially expanded state. The default value is `true`. | |||
| public var supportsPartialExpansion: Bool | |||
|
|
|||
| /// When `true`, collapsed drawer is never gets dismissed. | |||
| public var neverDismissCollapsedDrawer: Bool | |||
There was a problem hiding this comment.
I'd prefer something like canDismissCollapsedDrawer instead of neverDismissCollapsedDrawer. Also, the default value in the initialiser should then change to true.
| @@ -82,6 +82,9 @@ public struct DrawerConfiguration { | |||
| /// When `false`, the presentation is always to full screen and there is no | |||
| /// partially expanded state. The default value is `true`. | |||
| public var supportsPartialExpansion: Bool | |||
|
|
|||
| /// When `true`, collapsed drawer is never gets dismissed. | |||
There was a problem hiding this comment.
Grammar needs fixing. Should be "When true, collapsed drawer is never dismissed." Also, a note about the default value is missing.
| @@ -37,6 +37,7 @@ private extension PresenterViewController { | |||
| configuration.upperMarkGap = 100 // default is 40 | |||
| configuration.lowerMarkGap = 80 // default is 40 | |||
| configuration.maximumCornerRadius = 15 | |||
| configuration.neverDismissCollapsedDrawer = true | |||
There was a problem hiding this comment.
If changing from neverDismissCollapsedDrawer to canDismissCollapsedDrawer, then the value here should be false.
| containerViewHeight: CGFloat, | ||
| drawerFullY: CGFloat) -> CGFloat { | ||
| switch state { | ||
| case .collapsed: | ||
| return containerViewHeight | ||
| return containerViewHeight - drawerCollapsedHeight |
There was a problem hiding this comment.
If I understand correctly, I think the subtraction here should only happen when canDismissCollapsedDrawer is false. If it's true, then the previous behaviour (ie, no subtraction) should be the one that takes effect. In other words, I think this should be
let offset: CGFloat = canDismissCollapsedDrawer ? 0 : drawerCollapsedHeight
return containerViewHeight - offsetPlease make sure to test the behaviour for both values of canDismissCollapsedDrawer and make sure that the behaviour for canDismissCollapsedDrawer == true matches the current behaviour.
| @@ -82,6 +82,9 @@ public struct DrawerConfiguration { | |||
| /// When `false`, the presentation is always to full screen and there is no | |||
| /// partially expanded state. The default value is `true`. | |||
| public var supportsPartialExpansion: Bool | |||
|
|
|||
| /// When `false`, collapsed drawer is never dismissed. | |||
There was a problem hiding this comment.
This needs a comment about the default value (which is true). Something like:
/// When false, collapsed drawer is never dismissed. The default value is true.
|
I believe this is addressed now with #75 though without a configuration to not dismiss collapsed drawer |
|
I'm going to close this PR since the changes haven't been addressed, there are conflicts, and - as @ilyapuchka has pointed out - this is fixed by another PR. |
Hello!
Here's a PR to allow users disable dismissal of a drawer view and keep some height of it always on screen.
I think i can reference this issue here.