Add automatic sizeThatFits computation for views

[antoinelamy]

Disclaimer

This is a preliminary PR for discussion purpose. The implemented functionality has barely been tested and is not guaranteed to be bug free.

Motivation

Implementing sizeThatFits(_ size: CGSize) as part of the manual layout process has always been cumbersome. You always end up writing the same code twice, a first time for the layout and the second time for sizing. Using PinLayout to compute the resulting size like showcased in the AdjustToContainer exemple is not recommended either because the view coordinates are modified during sizing. The sizeThatFits method documentation state clearly:

This method does not resize the receiver.

Proposal

Build an autosizing mechanism on top of PinLayout without modifying the view's coordinates in the process.

• This PR add the AutoSizeCalculable interface that defines autosizing related functions and properties.
• An internal flag (Pin.autoSizingInProgress) is also needed for the layout system to know if it should compute an additional rect including the margins.
• Implementing sizeThatFits(_ size: CGSize) using automatic sizing requires the following:
  • Layout code is preferably located in a separate function than layoutSubviews() to be sure things like setting the content size on a scroll view are not executed during the sizing process. In the provided exemple, that function would be layout().
  • The sizeThatFits implementation is as simple as calling return autoSizeThatFits(size) { layout() } and even takes into account the outer margins (ie: the bottom margin applied to the last view on y axis)

Limitations

• Sadly I don't see any way of adding this capability directly on Layoutable because of the need to define stored properties.
• All layout related code must be done using PinLayout only, otherwise the views that uses another layout system would be ignored in the resulting computed size.

Discussion

I would very much like feedback on this PR discussing the concept and exposing the potential flaws if any. I think that if we can get this thing to work properly in every situation it would be a great addition to PinLayout.

[lucdion]

Salut Antoine 🙂

I like the general idea of your PR.

Global variable Pin.autoSizingInProgress

The only thing that bothers me is Pin.autoSizingInProgress. I know that all layouts related code should be executed from the main thread, so, a well-programmed app should not have 2 views that are layouted simultaneously. So having a global variable that contains that state shouldn't be an issue. But still, it's a global variable.

I'm not sure what could be a nicer solution thought, or if there is such a solution. But here is some thought on it (maybe this will trigger other ideas on your side 🤞).

Solution 1: Add a layout context
Suppose we add a method pin(_ context: LayoutContext?) and we add a context parameter to the layoutClosure, this context could then be passed to all pin calls. Ex:

override func sizeThatFits(_ size: CGSize) -> CGSize {
        return autoSizeThatFits(size) { (ctx: LayoutContext) in 
        layout(ctx) }
}

private func layout(_ ctx: LayoutContext?) {
   subview.pin(ctx).top().left(10).width(200);
}

Cons:

• A little verbose and not as nice.
• It's easy to forget to call pin(_ context: LayoutContext?) instead of .pin.

Solution 2: Check view's parents
Check in the view hierarchy if there is a parent view with an autoSizingRect value, in that case, autoSizing is in progress. For this to work, we need to set to nil the property autoSizingRect when leaving autoSizeThatFits(...).

Cons:

• Need to scans all view's parents to detect that there is no autoSizing in progress 😞

Solution 3: Global variable
Keep the global variable 😕

Experimental feature

In all cases I would keep that feature as experimental, i.e. we don't document immediately until you have played with the feature and you are happy with the result. During that time, you could note anything that would be useful to eventually document.

Non main thread calls warnings

I would also add a warning when .pin is called from a thread other than the main thread (https://github.com/layoutBox/PinLayout#pinlayouts-warnings).
Ex:
⚠️ PinLayout should be used only from the main thread. UIKit calls must be called from the app's main thread.

This warning could be disabled Pin.activeWarnings.mainThread.

...

I will continue to think about this feauture, so I may add other comments later

[antoinelamy]

I really would like to extract that function out of the UIView conformance extension but the only thing that prevent me to do so is the call to let adjustedRect = Coordinates<View>.adjustRectToDisplayScale(rect). I think it would make sense to add a displayScale property on the Layoutable protocol and perform that transformation before calling setRect. We really want the resulting rect to be the same in both layout and auto sizing.

[antoinelamy]

I have perform the changes to do this but I believe it would make more sense in a separate PR as it also fix an issue in the current production code.

[antoinelamy]

I agree that a global feature flag is probably not the most elegant solution but at the same time it's the less invasive way to implement it. I started trying to implement it creating a Pin object for a different object type acting as a view proxy but the code was overly complex and I ended up having the same need for a state variable.

Solution 1
It would require all the existing layout code to be refactored to opt-in to the feature plus it loose a bit of its simplicity that makes PinLayout so appealing IMO and like you said it would be easy to forget passing in the context object during pin creation.

Solution 2
I fear that the performance would take a huge hit on that one.

Solution 3
Well, I think it's the less bad solution and layout code should not be called from non main thread anyway, setting bounds or position on a background thread is forbidden. It might be a problem for another unknown Layoutable type that could potentially be layouted on multiple threads. In that case, my advise would be to not make that type conform to AutoSizeable.

The API is a bit similar to UIView.animate that wraps a beginAnimation() / endAnimation() behind the scene and a global flag is used to know wether or not the change should be performed animated or not. The fact that the API uses a closure where the layout code is expected to be called makes it less error prone I think.

[lucdion]

I agree @antoinelamy, that your solution is probably the best, even if there is one flaw, its the one that has a minimal impact on code

[antoinelamy]

Non main thread calls warnings

I would also add a warning when .pin is called from a thread other than the main thread (https://github.com/layoutBox/PinLayout#pinlayouts-warnings).
Ex:
⚠️ PinLayout should be used only from the main thread. UIKit calls must be called from the app's main thread.

This warning could be disabled Pin.activeWarnings.mainThread.

From what I see there is already such warning in PinLayout+Warning.displayLayoutWarnings() and it already covers the case of autosizing as we enter the apply() function the same way regular layout does:

if !Thread.isMainThread {
    warn("Layout must be executed from the Main Thread!")
}

[antoinelamy]

Should be good to go now @lucdion, anything else comes to mind?

[lucdion]

Two more things:

1. Could you duplicate the sample that you had previously modified? At least it shows an example.
2. You won't like that, but even if its an experimental feature. We would need a minimalist documentation that describes the feature and how to use it. I would add that new section below https://github.com/layoutBox/PinLayout#justify--align
   Thanks

[antoinelamy]

@lucdion I thought you preferred to silent release this feature, my mistake. I added a proper sample that fetches a random text and a random sized image. That content is then layout in a container that uses autosizing to determine its proper size. I also included setting a scroll view content size to illustrate the fact that we should not call non PinLayout code in the layout closure passed to autoSizeThatFits. I also added some documentation in the Readme for it, not sure if it's good enough but I'll leave it to you for review.

[lucdion]

Ready to go 🎉
Thanks @antoinelamy for this nice addition