Conversation
…he readableLayoutGuide instead of the scroll view.
| } | ||
| } | ||
|
|
||
| override open var contentInset: UIEdgeInsets { |
There was a problem hiding this comment.
I actually ran into problems with the contentInset. In the example app, there is a StackViewController inside another StackViewController. When the size class changed, the contentInset for the scroll view for the child StackViewController was reset to zero. The best I could figure out was that UIKit was reseting the contentInset. Even when I resized the window to be back to its original size, it was at zero. I put breakpoints in the didSet and it never hit the breakpoint. I spent a good amount of time trying to figure it out. I never did figure it out. I decided to move on since I had already spent so much time on it and it wasn't made worse w/ my changes. It had always been like that, we just never noticed because we weren't changing the window size.
Note: I never tested to see if this was a problem with the outer stack VC/scroll view. I don't think it ever had a contentInset so if the same thing happens there, it never would have surfaced there.
There was a problem hiding this comment.
All that being said, I can see how different insets for each size class would be beneficial. We don't need it and as far as I know there haven't been any requests for it though.
Since you gave the approval, I'm going to assume it's good to merge as is. If you'd like me to work on any changes to the contentInset, let me know and I can work on it in a separate PR.
There was a problem hiding this comment.
One last note @louoso, if the content inset issue I mentioned above seems worth fixing to you, I have ideas on how to fix it. As far as I know, the bug as it exists now shouldn't actually impact Formalist or our own apps because I don't think we make use of contentInset.
|
The test failed due to permission issues but when I run the tests locally, they pass. Going to merge. |
This will allow the content to be aligned according to the
readableLayoutGuideinstead of the scroll view's frame.