wojtek717 / bottom-sheet

Custom Bottom Sheet view for SwiftUI allowing to recreate sheet from Apple's Map, Find My apps. iOS Maps-like, Find My-like bottom sheet.
MIT License
15 stars 2 forks source link

Add `selectedDetent` bindings #15

Closed samdogg7 closed 4 weeks ago

samdogg7 commented 1 month ago

Background

Once the general fixes and improvements in #8 are merged, the package will include the concept of a selectedDetent, which persists the detent position when navigating to another screen.

Proposed Enhancement

While this is a step in the right direction, it would be even more flexible if the parent view could:

  1. Set selectedDetent: Allowing the parent to define the position.
  2. Listen to changes in the detent state: Enabling the parent to respond to detent updates, which could impact other UI states.

Benefits

wojtek717 commented 1 month ago

Sounds like a perfect extension of this functionality, especially the ability to select detention from code,

samdogg7 commented 1 month ago

@wojtek717 I can take this on today!

samdogg7 commented 1 month ago

@wojtek717 it has proven to be quite a complex change to support both isPresented and a Binding<Detent>. It is definitely possible, but will require a bit of boiler plate code.

My lean is to remove isPresented entirely in favor of requiring a Binding<Detent> + adding Detent.hidden case. This aligns with the keeping the implementation lightweight, opposed to blowing up package scope to support multiple initializers. An added benefit of this is the removal of initialDetent and setInitialDetent 😄

I believe isPresented as a Bool would be deprecated in #19 in favor of Binding<Bool>; so one way or another the instantiator will need to create a @State to pass in (similar to Apple's sheet).

What are your thoughts?

Again, it is definitely possible to support both but will require some complex handling in BottomSheet

wojtek717 commented 1 month ago

@samdogg7 I think that a more necessary and useful option is to support Binding isPresented to manage the display state of the sheet. It is also needed in my personal project for which this package was created :D. If you still want to continue working on selectedDetent binding then go ahead and tell me how I can help.

samdogg7 commented 1 month ago

Okay I'll investigate supporting both 👍🏼

samdogg7 commented 1 month ago

@wojtek717 would I be able to have access to this repo? Or should I continue to use forks? I do not need push access to main, just to create a new branch / push to it.

wojtek717 commented 1 month ago

@samdogg7 you should get an invitation