SwiftKickMobile / SwiftMessages

A very flexible message bar for UIKit and SwiftUI.
MIT License
7.31k stars 743 forks source link

SwiftMessagesSegue with preferredContentSize and safeAreaInsets #336

Closed patmalt closed 4 years ago

patmalt commented 5 years ago

I am presenting a view controller via SwiftMessagesSegue and configuring it with a layout of .bottomMessage. This view controller needs to have a dynamic height, based on its content. I was happy to see the view controller's preferredContentSize is respected and the view is sized correctly, with one exception...

The content of my view controller does not respect the safe area inset and the additional space must be manually calculated. I want the containerView of the segue to be constrained to the bottom just as it is for .bottomMessage, however the toView's (view controller content) bottom should be constrained to the safe area bottom. This will allow my content to be above the safe area and allow the containerView to block the un-safe area.

I thought this would be an easy fix, just add safeAreaInsets.bottom to the preferredContentSize and it would just work. However, at the time SwiftMessagesSegue asks for preferredContentSize, the view controller does not know its own safeAreaInsets because it is not yet added to the view hierarchy.

There are a few options to get this to work as I want, and I am looking for some feedback on how I might best accomplish this with SwiftMessages.

  1. Get the safeAreaInsets via UIApplication.shared.keyWindow?.safeAreaInsets.
    • This seems like a code smell to me. I do not like reaching out to UIApplication if it can be avoided.
  2. Add the safeAreaInsets.bottom from transitionContext.containerView to toVC?.preferredContentSize.height if the height is greater than 0
    • This would work, but might break existing consumers of SwiftMessages.
  3. Constrain segue.containerView.bottomAnchor to safeAreaLayoutGuide.bottomAnchor instead of toView.bottomAnchor
    • This would work, but might break existing consumers of SwiftMessages.

To continue being a consumer of SwiftMessages what approach should I take? I can submit any code changes via a pull request if that is necessary, depending on what we decide is the best approach.

wtmoose commented 5 years ago

Thanks for bringing this up. I don't know if this is an iOS bug or if I'm doing something wrong, but the main issue is that the presented view controller's safe area insets are zero, whereas you would expect them to be inherited from the superview, which are not zero. I've implemented a potential workaround that you can try on the head master.

And here is a sample project where you can see proper safe area insetting in action:

SwiftMessagesTest.zip

Since your case involves safe area, I recommend not using preferredContentSize. Instead, allow Auto Layout to determine the dynamic height for you. You just need to specify sufficient Auto Layout constraints in your view controller such that the height is deterministic.

I want the containerView of the segue to be constrained to the bottom just as it is for .bottomMessage, however the toView's (view controller content) bottom should be constrained to the safe area bottom.

The toView's bottom is constrained to the container's bottom so that it can provide a full-bleed background if needed. You are expected to constrain the toView's contents to the safe area if you want to avoid the safe area.

Let me know if this solves your problem.

patmalt commented 5 years ago

Thanks for the quick response and helping me out!

To be clear, I believe this is neither an iOS bug or the fault of SwiftMessages. A view's safeAreaInsets are not set until it is added to the view hierarchy. According to the Apple documentation:

If the view is not currently installed in a view hierarchy, or is not yet visible onscreen, the edge insets in this property are 0.

From what I can tell, at the time SwiftMessagesSegue's TransitioningPresenter asks for toViewController's preferredContentSize, toView has not been added to the view hierarchy.

The changes in master, in addition to the pointer to constrain the contents to the safe area of toView made it work as expected.

Until I wrap toViewController in a navigation controller...

Since UINavigationController's view's (specifically UINavigationTransitionView and UIViewControllerWrapperView) subviews do not have sufficient constraints (i.e. none) to determine the height of the navigation controller. This activates the height constraint of 350 😩

Any pointers on how to get a dynamically sized UINavigationController presented via SwiftMessages?

patmalt commented 5 years ago

One caveat I forgot to mention when I stated that I got toView's contents to float above the un-safe area.

Setting messageView.bounceAnimationOffset to 0 (which is done in configure(layout:)) made the padding at the bottom larger than the safe area. This was the case even when messageView.layoutMarginAdditions is set to UIEdgeInsets.zero.

I found that to be extremely confusing, and I am still not sure why this is the case, even after digging around the code for most of my work day.

wtmoose commented 5 years ago

To be clear, I believe this is neither an iOS bug or the fault of SwiftMessages. A view's safeAreaInsets are not set until it is added to the view hierarchy.

Yep, I'm aware of this. However, the safe area insets were never being set. I verified after the view had appeared and the transition animation had completed.

Since UINavigationController's view's (specifically UINavigationTransitionView and UIViewControllerWrapperView) subviews do not have sufficient constraints (i.e. none) to determine the height of the navigation controller. This activates the height constraint of 350 😩

Yep, nav controllers don't work with self-sizing. The only way I know how to size nav controller is to set an explicit height through preferredContentSize or adding a height constraint.

So is your top-level view the nav controller's view? And you want this view to be above safe area? Is your situation similar to one of the examples in the Demo app? Being able to visualize what you're trying to achieve would be helpful.

I found that to be extremely confusing, and I am still not sure why this is the case, even after digging around the code for most of my work day.

Sorry to hear that. I may have time to look at this in a day or two, but I don't have time right now, unfortunately.

wtmoose commented 4 years ago

As of the Xcode 11 GM, the safe area bug went away. So I removed the workaround (which wasn't working for me in some apps). If you need further help. please send me a demo app or a drawing of what you're trying to accomplish.

patmalt commented 4 years ago

Sorry for not getting back to you. Our team decided the .bottomCard style was good enough for us. Therefore, we can go ahead a close this issue.

I really appreciate your support. Thanks!