babylonhealth / DrawerKit

DrawerKit lets an UIViewController modally present another UIViewController in a manner similar to the way Apple's Maps app works.
https://github.com/Babylonpartners
MIT License
781 stars 44 forks source link

Configurable initial state and collapsed state height #75

Closed ilyapuchka closed 6 years ago

ilyapuchka commented 6 years ago

This PR adds support for configurable initial state and collapsed state height. Currently it's only possible to completely hide drawer in collapsed state which also results in dismissing view controller. With this change it's possible to make drawer visible not just in partial or fully expanded state, but also in collapsed state without introducing any additional state.

20181024211114

When collapsed state is chosen as initial and it has non-zero height then dismissing on collapsed state becomes unneeded so it is not happening in this case. ~Also added ability to not animate corner radius as with current implementation it will be animated in between collapsed and partially expanded state which also seems to be undesirable when collapsed state has non-zero height. Though it can be made configurable with minimumCornerRadius which will correspond to corner radius in collapsed state. But this can be done as a separate PR.~ This is now part of #77

ilyapuchka commented 6 years ago

This may intersect with #65 but it is stale.

ilyapuchka commented 6 years ago

The change is proposed because it is needed for the new requirements on the map screen we are implementing in the app. DrawerKit unlike Pulley does not ATM make it possible to display the drawer in 3 states (collapsed, partially expanded and fully expanded), as demonstrated in the gif. Collapsed state is ATM means that nothing is displayed and when reaching it presented controller is dismissed. Alternatively we could introduce yet another state but I think it will unnecessary complicate implementation.

wltrup commented 6 years ago

DrawerKit unlike Pulley does not ATM make it possible to display the drawer in 3 states (collapsed, partially expanded and fully expanded), as demonstrated in the gif

Huh? That's exactly what the partially expanded state is for. It's been there from day one.

ilyapuchka commented 6 years ago

@wltrup I'll explain again, currently the sates are: collapsed - nothing is visible partially expanded - part of drawer is visible fully expanded - full drawer is visible

With this change: collapsed - small portion of drawer is visible partial expanded - bigger part of drawer is visible fully expanded - full drawer is visible

This effectively replicates behaviour of iOS Maps app where search screen displayed in drawer can be collapsed to only display search field without being dismissed. And this is behaviour we need to implement.

wltrup commented 6 years ago

The problem I see with that is that it's using a state with a clear semantic meaning (collapsed) in two semantically different and distinct situations (an actual dismissal and a "fake" dismissal). The correct way to handle this is to have an additional state for the "fake" dismissal.

ilyapuchka commented 6 years ago

@wltrup is there dismissed state? There is collapsed state that is currently corresponds to dismissed state because in this state nothing is visible on the screen. With this change these states are separate and dismiss only happens when 0 is provided for collapsed height. So there are effectively 4 states with this change: collapsed, partially expanded, fully expanded and dismissed which is just not represented by the state enum. Do you suggest to add it? What value it will bring?

wltrup commented 6 years ago

Sorry. I just corrected my last post. The state I meant is collapsed, not dismissed.

wltrup commented 6 years ago

Yes, I'm suggesting that we should have 4 actual states in the enumeration, if we are to support this correctly: fully expanded, partially expanded, collapsed (which can only be transitioned into from fully expanded or partially expanded), and dismissed (meaning done, the drawer is gone for good).

ilyapuchka commented 6 years ago

Ok, I'll add dismissed state

wltrup commented 6 years ago

Well, it's more like renaming the current collapsed to dismissed and adding a new collapsed state, I think.

ilyapuchka commented 6 years ago

Why not just keep collapsed as collapsed and add dismissed that will be transitioned to only when dismiss happens? seems like a simpler change to me.

wltrup commented 6 years ago

No, because the current collapsed state has the semantic meaning of a dismissal. If you create a new dismissed state, you'll need to migrate a lot of code from collapsed to it. I think it's easier to rename collapsed to dismissed, create a new collapsed state and move into it the few lines of code that are in the current collapsed state, having to do with non-dismissal behaviour.

ilyapuchka commented 6 years ago

@wltrup it's done

bryan1anderson commented 5 years ago

I'm so confused by how I'm supposed to achieve this now

gtsiap commented 5 years ago

@bryan1anderson You have to implement DrawerPresentable.heightOfCollapsedDrawer.

You can find an example here

https://github.com/Babylonpartners/DrawerKit/blob/c2b3b1fb205e28cb90c57baae151383ac54d6944/DrawerKitDemo/DrawerKitDemo/PresentedViewController.swift#L33

bryan1anderson commented 5 years ago

@gtsiap Thanks I was stuck with what I thought was a problem of not "setting" the state. Now I understand if you provide heightOfCollapsedDrawer then it works.