slackhq / PanModal

An elegant and highly customizable presentation API for constructing bottom sheet modals on iOS.
MIT License
3.69k stars 533 forks source link

SwiftUI intrinsic content height #135

Open AndrewSB opened 3 years ago

AndrewSB commented 3 years ago

Description

i've been using PanModal to present SwiftUI modals on top of UIKit views for a while, and it's been working great! i'm noticing an issue that occurs when the height of my SwiftUI view changes, when using a pan modal height of .intrinsicContentSize, recalculating the height is

  1. non-deterministic: as in recalculations of the same SwiftUI height result in different final UIKit heights
  2. and requires a DispatchQueue.async, presumably to wait for the SwiftUI layout to finish before UIKit calculates sizeThatFits, but I'm unsure

here's some sample code for how I setup my PanModal view

class ToyPickerController: UIHostingController<ToyPickerView> {

    init(didTapClose: @escaping () -> Void, didSelect: @escaping (Toy, Toy.Option?) -> Void) {
        super.init(rootView: ToyPickerView(didTapClose: didTapClose, didSelect: didSelect))
        self.rootView.invalidateHeight = { [weak self] in
            DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(10)) { // <- RELEVANT
                self?.panModalSetNeedsLayoutUpdate()
                self?.panModalTransition(to: .shortForm)
            }
        }
    }

}

extension ToyPickerController: PanModalPresentable {
    var panScrollable: UIScrollView? { .none }

    var shortFormHeight: PanModalHeight { .intrinsicHeight }
    var longFormHeight: PanModalHeight { .intrinsicHeight }

    var cornerRadius: CGFloat { 24 }

    var showDragIndicator: Bool { false }
}

I call invalidateHeight() right after I make a change that will cause the height to become different. In the following example, it's when I click on the puzzle button

and here's a screen recording showing non-deterministic height for the view: you can see I change the height of the modal after presenting it 3 times in succession: the first time the height decides to shrink, the second time it grows, and the third time it stays the same. In SwiftUI, the heights of both states are the same

https://user-images.githubusercontent.com/3814772/108647668-1b68a700-746e-11eb-8237-ba5f3b868e05.mp4

i'd love to figure out how to remove the DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(10)), and maybe that will solve the non-deterministic intrinsic content size as well? happy to provide more code/create an example. thanks!

lucanicoletti commented 2 years ago

I'm facing a similar issue when having a tableView inside the PanModal. May I ask @AndrewSB what's the rationale behind the + .milliseconds(10) when dispatching the event?

AndrewSB commented 2 years ago

rationale was to give UIKit enough time to layout to it's intrinsic size before PanModal asks the view for it's height