robb / Cartography

A declarative Auto Layout DSL for Swift :iphone::triangular_ruler:
Other
7.35k stars 525 forks source link

Adding support to UILayoutGuide #269

Closed corujautx closed 6 years ago

corujautx commented 6 years ago

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.

corujautx commented 6 years ago

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

corujautx commented 6 years ago

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.

corujautx commented 6 years ago

@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

orta commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Can you check it @orta , @inamiy ?

orta commented 6 years ago

Oh yeah, my bad 👍

orta commented 6 years ago

I'll give this a one over

orta commented 6 years ago

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 commented 6 years ago

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 
}
corujautx commented 6 years ago

@orta I posted the explanation above, please check.

orta commented 6 years ago

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

ghost commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

NachoSoto commented 6 years ago

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 commented 6 years ago

@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
}