Adding support to UILayoutGuide #269

corujautx · 2017-10-16T14:46:54Z

This pull-request adds support in Cartography to use UILayoutGuides and improves Cartography to work with iOS 11.0 new features.

There's a breaking change regarding UILayoutSupport as I made it simpler to constrain elements such as safeAreaLayoutGuide and general views. This breaking change reflects that you no longer will use topLayoutGuideCartography or bottomLayoutGuideCartography inside the constrain block but instead you pass a property of the view controller to the method (e.g. now named both car_topLayoutGuide and car_bottomLayoutGuide) so you can attach constraints to its height, top or bottom as anchors are specified by the documentation.

There's also a convenience property in the new ViewProxy that allows you to directly access a brand new LayoutProxy corresponding to the safeAreaLayoutGuide in iOS 11.0 as suggested by @gsampaio .

This should be able to close issues #268 and #267, and improve upon #207.

I look forward to suggestions in how to improve this pull request. Thanks.

This also adds a major refactoring to Cartography, supporting UILayoutGuide and adding a proper support to UILayoutSupport in face of the release of iOS 11.0

Adding support to UILayoutGuide

corujautx · 2017-10-16T14:59:25Z

Also, because I extended the number of constrain overloads this should also fix #263

corujautx · 2017-10-16T17:38:40Z

The breaking change that this PR introduces is that code that today looks like this

constrain(view) { view in 
    view.top == vc.topLayoutGuideCartography
}

will change to:

constrain(view, vc.car_topLayoutGuide) { view, guide in 
     view.top == guide.bottom
}

This adds more flexibility as you can also now constrain to the top/height attributes of an UILayoutSupport.

The whole idea in this PR is to unify UIView (or NSView in macOS), UILayoutGuide and UILayoutSupport as constrainable elements, represented by the protocol LayoutElement.

inamiy

Great work! 👏👏👏

I like LayoutProxys splitted into multiple directions, and separating ViewProxy and LayoutGuideProxy so that ViewProxy.safeAreaLayoutGuide / LayoutGuideProxy.owningView is possible (I think we need test cases for this).

A bit downside is the use of as! casts in many places, but I guess it is needed unless we introduce new messy type-erased types.

For terminology, I personally prefer items over elements since "item" is used throughout Auto Layout system (e.g. NSLayoutConstraint(item:...)).

inamiy · 2017-10-17T00:53:21Z

Cartography/LayoutGuideProxy.swift

+
+@available(iOS, introduced: 9.0)
+@available(tvOS, introduced: 9.0)
+public final class LayoutGuideProxy: SupportsPositioningLayoutProxy {


UILayoutGuide doesn't support firstBaseline/ lastBaseline, so this should not conform to SupportsBaselineLayoutProxy (super protocol).

I'll be patching up that. Thanks @inamiy

Also, owningView has been implicitly tested. I think the missing test cases are for safeAreaLayoutGuide which I'll also add. I'll also rename LayoutElement to LayoutItem.

I'll try to figure out something about the IUOs.

corujautx · 2017-10-17T13:05:46Z

@inamiy I added the requested fixes and I also think this PR also deprecates #267 as it also solves that problem.

ps: #265 is also a duplicate of this

inamiy

I think overall now LGTM 👍

Since this is quite a big breaking change, I think #267 can be merged first for soft-landing ver 2.1, and then this PR for ver 3.0 (should have more reviewers).

@orta What do you think?

orta · 2017-10-18T01:38:15Z

OK, so this will need a bit of a rebase - I've released #267 as a 2.1.0 release

The idea behind this all seems 👍 to me, I'm happy to 3.0 it

corujautx · 2017-10-18T03:30:30Z

Okay, I merged the current master with my fork. Please check if everything is okay and we can merge and release Cartography 3.0 :)

corujautx · 2017-10-24T02:23:52Z

Can you check it @orta , @inamiy ?

orta · 2017-10-24T20:47:53Z

Oh yeah, my bad 👍

orta · 2017-10-24T20:47:57Z

I'll give this a one over

orta · 2017-10-24T20:48:47Z

Ah yeah, can I get an example of how to migrate from 2.x to 3.0? To add to the release notes. Then this is good to go from my side 👍

corujautx · 2017-10-25T00:12:11Z

I posted above but:

The properties topLayoutGuideCartography and bottomLayoutGuideCartography have been replaced by car_topLayoutGuide and car_bottomLayoutGuide respectively. Those now are to be injected in constrain instead of the block that is supplied to the method.

Code that today looks like this:

constrain(view) { view in 
    view.top == self.topLayoutGuideCartography
    view.bottom == self.bottomLayoutGuideCartography
}

now will look like this:

constrain(view, self.car_topLayoutGuide, self.car_bottomLayoutGuide) { view, topLayoutGuide, bottomLayoutGuide in
    view.top == topLayoutGuide.bottom
    view.bottom == bottomLayoutGuide.top 
}

inamiy · 2017-10-25T01:14:17Z

Cartography/View.swift

@@ -14,8 +14,6 @@ import Foundation

    extension UIView: LayoutItem {
        public func asProxy(context: Context) -> ViewProxy {
-            self.translatesAutoresizingMaskIntoConstraints = false


In 767821e...da5e483, why did translatesAutoresizingMaskIntoConstraints = false move to other methods?
It seems scattered than before 😕

Yeah, sorry about that.

Here's the problem tho: this introduces a bug/breaking change on the library depending on how people use it, and I kinda discovered that by pointing out my fork to a project we are developing at our company.

When I centralized the translatesAutoresizingMaskIntoConstraints in the proxy generation step I was thinking "Hey this is going to make the library a lot more simple", but the issue is that lot of people use the syntax as in:

constrain(view, superview) { view, superview in ... }

instead of:

constrain(view) { view in view.top == view.superview!.top }

in order to avoid IUOs

When we moved to the proxy code, the first case generated a side effect of setting up translatesAutoresizingMaskIntoConstraints in the superview, and this broke the layout of several root views in UINavigationController or any container view controller.

I couldn't predict this side effect being generated by the code I made, so I changed back into the original behavior of the library, even if it looks ugly, to avoid more breaking changes.

Thanks for explanation!
So, translatesAutoresizingMaskIntoConstraints = false impl is actually back to the original code now 🙄

corujautx · 2017-10-29T19:24:55Z

@orta I posted the explanation above, please check.

orta · 2017-10-31T11:51:48Z

Ah yeah, thanks, OK, let's get the 3.0 train going

ghost · 2017-11-11T01:23:14Z

There is an issue with this if we are adding more than five views we have to use let views:[LayoutItem] = [view1,view2,self.car_topLayoutGuide, self.car_bottomLayoutGuide ]

error
"Protocol 'LayoutItem' can only be used as a generic constraint because it has Self or associated type requirements"

corujautx · 2017-11-11T14:19:26Z

@pprevalon That is why I added more overloads to constrain to support more than 5 elements (now it supports up to 10 elements).

The reason why this happens is because Swift doesn't allow us to use covariant generics, but instead all are invariant, making the array conversion a problem. e.g. the signature is func constrain<T: LayoutElement>(elements: [T], block: ([T.LayoutProxy]) -> Void) which basically means that if you use an array of View it'll generate a constrain method that takes necessarily a block that uses [ViewProxy] instead. Perhaps I could use LayoutProxy instead, but I feel that'd be more of a hassle than a solution, given that it'll create a casting hell and messes with type-safety.

If anyone has any solution to this problem, or a way to work around it, it'd be nice to hear, but I can't think of a way in Swift to create a working interface for that.

NachoSoto · 2017-11-16T22:51:59Z

I'm confused about this change. With #267 we could do view.edges == view.superview!.safeArea.edges. That doesn't seem possible anymore?

NachoSoto · 2017-11-16T23:01:32Z

Never mind, I'm silly, view.edges == superview.safeAreaLayoutGuide.edges works great :)

This is actually what I'm doing:

if #available(iOS 11.0, *) {
    constrain(self.scrollView, self) { scroll, superview in
        scroll.edges == superview.safeAreaLayoutGuide.edges
    }
} else {
    constrain(self.scrollView, self) { scroll, superview in
        scroll.edges == superview.edges
    }
}

This happens to be enough in my case cause this view isn't affected by top or bottom layout guides (it's in the middle of the screen).

corujautx · 2017-11-17T01:28:48Z

@NachoSoto if you need to use top layout guide and bottom layout guide you could use:

constrain(self.scrollView, self.car_topLayouGuide, self.car_bottomLayoutGuide) { scroll, topGuide, bottomGuide in 
    scroll.top == topGuide.bottom
    scroll.bottom == bottomGuide.top
    scroll.leading == scroll.superview!.leading
    scroll.trailing == scroll.superview!.trailing
}

corujautx added 7 commits October 11, 2017 14:06

Adding support to UILayoutGuide in Cartography

2732065

This also adds a major refactoring to Cartography, supporting UILayoutGuide and adding a proper support to UILayoutSupport in face of the release of iOS 11.0

Removing elements from constrain method signature

462273b

Adding tests for LayoutGuide support

6bcf956

Adding convenience support to safeAreaLayoutGuide

6421a8c

Fixing documentation replacing references to view

6729905

Merge pull request #1 from corujautx/feature/layout-guide-support

a91c01a

Adding support to UILayoutGuide

Changing access control of proxy properties

b6eb9c9

corujautx added 2 commits October 16, 2017 13:47

Fixing issues with macOS

e48af31

Fixing issue with top margin

bcf9c93

inamiy reviewed Oct 17, 2017

View reviewed changes

corujautx added 4 commits October 17, 2017 10:10

Removing baseline properties from layout guide proxy

a31a7e2

Removing IUOs & adding type-erasure

878f609

Adding safeAreaLayoutGuide tests

1bb14b9

Renaming LayoutElement to LayoutItem

e4585a3

Fixing type-erasures not being linked for the macOS target

f2a8b73

corujautx closed this Oct 17, 2017

corujautx reopened this Oct 17, 2017

inamiy approved these changes Oct 17, 2017

View reviewed changes

Merge branch 'master' of https://github.com/robb/Cartography

767821e

corujautx added 5 commits October 24, 2017 15:23

Fixing autoresizing mask to match older behavior

5cd25b2

Fixing generation of view proxies

4cdc8fa

Fixing issue with Context not disabling autoresizing mask translation

143d9fc

Changing podspec version to 3.0.0

da5e483

Adding AutoresizingMaskLayoutProxy to macOS and tvOS targets

fc476fe

inamiy reviewed Oct 25, 2017

View reviewed changes

inamiy approved these changes Oct 25, 2017

View reviewed changes

orta merged commit bc78595 into robb:master Oct 31, 2017

orta mentioned this pull request Oct 31, 2017

README needs updating for UILayout things #273

Open

inamiy mentioned this pull request Nov 9, 2017

UILayoutGuide support #268

Closed

NachoSoto mentioned this pull request Nov 16, 2017

Add safeAreaLayoutGuide support #265

Closed

inamiy mentioned this pull request Feb 8, 2018

Add Safe Area Layout support #267

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support to UILayoutGuide #269

Adding support to UILayoutGuide #269

corujautx commented Oct 16, 2017

corujautx commented Oct 16, 2017 •

edited

Loading

corujautx commented Oct 16, 2017 •

edited

Loading

inamiy left a comment

inamiy Oct 17, 2017

corujautx Oct 17, 2017

corujautx Oct 17, 2017

corujautx commented Oct 17, 2017 •

edited

Loading

inamiy left a comment

orta commented Oct 18, 2017

corujautx commented Oct 18, 2017

corujautx commented Oct 24, 2017

orta commented Oct 24, 2017

orta commented Oct 24, 2017

orta commented Oct 24, 2017

corujautx commented Oct 25, 2017

inamiy Oct 25, 2017

corujautx Oct 25, 2017 •

edited

Loading

inamiy Oct 25, 2017

corujautx commented Oct 29, 2017

orta commented Oct 31, 2017

ghost commented Nov 11, 2017 •

edited by ghost

Loading

corujautx commented Nov 11, 2017 •

edited

Loading

NachoSoto commented Nov 16, 2017

NachoSoto commented Nov 16, 2017

corujautx commented Nov 17, 2017

Adding support to UILayoutGuide #269

Adding support to UILayoutGuide #269

Conversation

corujautx commented Oct 16, 2017

corujautx commented Oct 16, 2017 • edited Loading

corujautx commented Oct 16, 2017 • edited Loading

inamiy left a comment

Choose a reason for hiding this comment

inamiy Oct 17, 2017

Choose a reason for hiding this comment

corujautx Oct 17, 2017

Choose a reason for hiding this comment

corujautx Oct 17, 2017

Choose a reason for hiding this comment

corujautx commented Oct 17, 2017 • edited Loading

inamiy left a comment

Choose a reason for hiding this comment

orta commented Oct 18, 2017

corujautx commented Oct 18, 2017

corujautx commented Oct 24, 2017

orta commented Oct 24, 2017

orta commented Oct 24, 2017

orta commented Oct 24, 2017

corujautx commented Oct 25, 2017

inamiy Oct 25, 2017

Choose a reason for hiding this comment

corujautx Oct 25, 2017 • edited Loading

Choose a reason for hiding this comment

inamiy Oct 25, 2017

Choose a reason for hiding this comment

corujautx commented Oct 29, 2017

orta commented Oct 31, 2017

ghost commented Nov 11, 2017 • edited by ghost Loading

corujautx commented Nov 11, 2017 • edited Loading

NachoSoto commented Nov 16, 2017

NachoSoto commented Nov 16, 2017

corujautx commented Nov 17, 2017

corujautx commented Oct 16, 2017 •

edited

Loading

corujautx commented Oct 16, 2017 •

edited

Loading

corujautx commented Oct 17, 2017 •

edited

Loading

corujautx Oct 25, 2017 •

edited

Loading

ghost commented Nov 11, 2017 •

edited by ghost

Loading

corujautx commented Nov 11, 2017 •

edited

Loading