Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BottomSheet v3 #79

Merged
merged 337 commits into from
Aug 7, 2022
Merged

BottomSheet v3 #79

merged 337 commits into from
Aug 7, 2022

Conversation

lucaszischka
Copy link
Owner

@lucaszischka lucaszischka commented Jul 4, 2022

The goals

I have been recoding the project to achieve

  • full macOS support
  • full iPad support
  • true landscape support
  • a more SwiftUI like feeling of the project

ViewModifiers

First I dropped the options: [BottomSheet.Options] parameter and instead switched to ViewModifiers. For example:

...
    .bottomSheet(...)
    .showCloseButton(self.isCloseButtonShown)
    .enableAppleScrollBehaviour()
    .customBackground(
        // No need for AnyView and works just like .background from SwiftUI
        Color.red
    )
    .onDismiss {
        // Custom dismiss action
    }

I think the concept is pretty self explanatory and after the readme is updated with a full list of modifiers all question regarding this should be resolved.

BottomSheetPosition

I also tried to make the BottomSheetPosition easier to use and to implement new features; instead of subclassing I switched to a predefined enum that represents all different possibilities:

case hidden

case dynamicBottom
case relativeBottom(CGFloat)
case absoluteBottom(CGFloat)

case dynamicTop
case relativeTop(CGFloat)
case absoluteTop(CGFloat)

case dynamic
case relative(CGFloat)
case absolute(CGFloat)

New cases explanation

...Bottom represents the state where the main content is hidden and ...Top the state where the BottomSheet can be scrolled when using .enableAppleScrollBehaviour(true).
The last new option is dynamic... which represents the state where the sheet has a "dynamic" height / wraps its content height.

Consequences

However this new BottomSheetPosition requires you to tell the BottomSheet which positions can be switched out / into (for example by swiping down/up or tapping the tap indicator). Therefore I added a switchablePositions: [BottomSheetPosition] parameter, where you can declare the cases you want to have. For example:

..., switchablePositions: [
    .relativeBottom(0.125),
    .relative(0.4),
    .absolute(600),
    .relative(0.8),
    .absoluteTop(800)
], ...

As you may realize, this looks very similar to the BottomSheetPosition enum in v2. Converting to it should therefore be easy.

Pros and Cons

Pros of the new implementation:

Cons of the new implementation:

  • the need to declare the switchablePositions: [BottomSheetPosition] makes it not clearer or easier to use

What do you think?

  • Is the syntax worse than before?
  • Do you have any ideas how the syntax could be made clearer and easier to use?
  • Or do you even like the new syntax?
  • Are the new features for the BottomSheetPosition 'worth it'?
    It would be nice if you could provide feedback regarding this questions.

Other new features

About macOS and iPad support

appleScrollBehaviour on iPad and Mac now 'only' packs the view inside of a basic ScrollView. Why?
Because the sheet is "up side down" so you are pulling it down. And it is in the top left not on the bottom. Therefore the appleScrollBehaviour behaviour of the iPhone doesn't make sense.

@lucaszischka lucaszischka added the enhancement New feature or request label Jul 4, 2022
@lucaszischka lucaszischka added this to the v3 milestone Jul 4, 2022
@lucaszischka lucaszischka linked an issue Jul 22, 2022 that may be closed by this pull request
@lucaszischka lucaszischka marked this pull request as ready for review August 7, 2022 01:59
@lucaszischka lucaszischka merged commit 75b0143 into main Aug 7, 2022
@lucaszischka lucaszischka deleted the v3 branch August 7, 2022 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants