danielsaidi / SwiftUIKit

SwiftUIKit is a Swift SDK that adds extra functionality to Swift & SwiftUI.
MIT License
1.42k stars 56 forks source link

Undimmed Sheet Detents - Dim reappears after sheet height changes. #22

Closed anqus closed 1 year ago

anqus commented 1 year ago

Hello, thanks for the undimmed sheet detents, I find it surprising this isn't included in SwiftUI by default, but great work figuring out how to implement it!

I've spotted that the dim reappears if the sheet height is changed programmatically. Here's a quick sample:

import SwiftUI
import SwiftUIKit

struct ContentView: View {
  @State var sheetHeight: CGFloat = 100
  @State var sheetIsPresented = false

  var body: some View {
    VStack {
      Button(
        action: {
          sheetIsPresented.toggle()
          sheetHeight = 100
        },
        label: {
          Text("Show Sheet")
        }
      )
    }
    .sheet(
      isPresented: $sheetIsPresented
    ) {
        VStack {
          Text("\(sheetHeight)")
          Button(
            action: {
              sheetHeight += 10
            },
            label: {
              Text("Increase Sheet Size")
            }
          )
        }
      .presentationDetents(
        undimmed: [ UndimmedPresentationDetent.height(sheetHeight)]
      )
    }
  }
}

Before tapping "increase sheet size", and after: IMG_0007 IMG_0008

danielsaidi commented 1 year ago

Hi @anqus

Yes, it seems like there are some problems with this approach when changing the height programmatically.

I guess the code needs to call some events whenever the height changes?

I can take a look at this when I'm using it the next time. Until then, I'd be happy to take a look at any PR that fixes this.

danielsaidi commented 1 year ago

Hi again @anqus

I have refactored the code quite a bit, and added your sample code to try it out. There are still occasional glitches, but I think it behaves a lot better now than before.

Can you try it out in the master branch and let me know what you think?

anqus commented 1 year ago

Hey, sorry for taking a while to come back to you.

I have tried out the changes, it seems like it works 50% of the time, but still frequently applies the dim.

Not sure I fully understand your changes, but I've tried to fix the problem and not got anywhere 😅

https://github.com/danielsaidi/SwiftUIKit/assets/54314568/55e4e1d4-0770-4438-8e8f-e72a11613d5c

danielsaidi commented 1 year ago

Hmmm, in that case I think I don't have anything else to add :) I use this approach with a fixed set of heights, which works well.

Perhaps you could add a height-based .id() view modifier to force a redraw whenever the height changes?

zmeyc commented 9 months ago

This workaround seems to work:

    private class UndimmedDetentViewController: UIViewController {
        override func viewDidLayoutSubviews() {
            super.viewDidLayoutSubviews()
            avoidDimmingParent()
            avoidDisablingControls()

            Task { @MainActor in
                self.avoidDimmingParent()
                self.avoidDisablingControls()
            }
        }

I'm not sure why the state is getting reset after viewDidLayoutSubviews is called.

danielsaidi commented 9 months ago

Hi @zmeyc

Have you confirmed if this works? In that case, I could add it to the library.

zmeyc commented 7 months ago

@danielsaidi Unfortunately, it turned out that it also doesn't work reliably and has timing issues. I called these functions again after a small delay and it does seem to work, but the screen is grayed for a moment.

danielsaidi commented 7 months ago

@zmeyc I see, too bad. I deprecated this in the latest version, since iOS 16 has native support.