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

Presented view with `intrinsicHeight` will change height after initial interaction #105

Open tunidev opened 4 years ago

tunidev commented 4 years ago

Description

Describe your issue here.


Bug Report

When using intrinsicHeight the first time we drag up or down, it makes the presented view to change position (height). After that it will keep the new adjusted height.

Reproducible in:

PanModal version: 1.2.7

iOS version: 13.5

Steps to reproduce:

  1. Run sample project
  2. Open User Groups (Stacked)
  3. Tap any cell
  4. Drag up or slightly down the presented view

Expected result:

I expect that the presented view should keep the same height and only bounce back to the initial position or dismiss. But not changing the height the first time we drag it

Actual result:

The height of the presented view will change as soon as you drag up or down.

Attachments:

ezgif com-resize

rorybainfreetrade commented 4 years ago

I've just come across this same issue and I think it's safe area related. It seems the height before the interaction is too high; it stretches your view have a height that is equivalent to its intrinsic height plus two safe areas, then after interaction, it adjusts the height to be instrinsic height plus one safe area, but that results in your content moving below the safe area.

Edit: confirmed somewhat, if you constrain to the superview bottom and not its' safe area then it doesn't change the height. However, when you do this, the modal appears, your content sits on the bottom side of the modal, leaving space at the top of itself that is equivalent to the safe area height.

Further Edit: I've got a workaround where if you call CATransaction.flush() at the end of PanModalPresentationController.adjustPresentedViewFrame() then it works, albeit a little jumpy. (I found this whilst trying to debug the layout)

I've found the issue is that adjustPresentedViewFrame correctly adjusts the views frame to the its instrinsicContentSize plus the safeArea.bottom, however, later your view updates its constraints to include the safe area, which makes its intrinsic content size bigger, but topMargin(from: PanModalHeight) still adds a safe area offset. I'm not sure if there's a way that the intrinsic content size reading in topMargin() can calculate the view size without a safe area.

If you call presentedViewController.setNeedsLayout() and presentedViewController.layoutIfNeeded() at the end of PanModalPresentationController.adjustPresentedViewFrame(), then you get a correctly laid out view. However, upon interacting with the view, it resolves to a height that is overly extended.

Here's what I've been using to investigate, just re-writing BasicViewController.swift from the demo. Expected outcome is that the red label shows a height of 100 at all times, and that interacting with the modal does not adjust the y position after the interaction ends.

BasicViewController.txt

rorybainfreetrade commented 4 years ago

@tunidev FYI I added a workaround here. I'm not 100% sure it works for all cases, but in my usage I've been constraining my views to the safeArea.bottom, so if you're doing that this might be worth a shot.

tunidev commented 4 years ago

Thank you for the investigation. I confirm that your workaround is fixing the issue in my layout with is similar to the demo project.

malonehedges commented 4 years ago

@ste57 any chance this could be merged in?

ApoorvKhatreja commented 4 years ago

This obviously depends on your implementation, but I was also facing this issue and I was able to fix it without forking the library, instead I forced a view layout before the transition begins by implementing the following in my view controller:

override func viewWillAppear(_ animated: Bool) {
    super.viewWillAppear(animated)
    view.layoutIfNeeded()
}

This forces the view to layout with safeAreaInsets included, and therefore the updated frames are used by the library for animations. Give it a shot, it might resolve your issue as well.

kodakvn commented 1 year ago

This works for me and UI will not be flash while updating layout

override func viewWillAppear(_ animated: Bool) {
    super.viewWillAppear(animated)

    DispatchQueue.main.async { [weak self] in
        self?.panModalSetNeedsLayoutUpdate()
        self?.panModalTransition(to: .shortForm)
    }
}