Make height/width inputs safer

[gsbernstein]

Anchor versions: constraining one view's dimension to another view's dimension
Constant version: constrain a view's dimension to a constant

• reverts 9c055fb: inferring superview on the anchor versions was a huge mistake because it allows the anchor version of width/height to overload the constant version of width/height. First noticed when Vir was onboarding and it was calling the wrong one unexpectedly.
• de-anonymizes constant versions' Float input params for clarity
• remove default values from constant versions' Float input params (why would zero be the default??)

These are breaking changes, but worth it.

[ericgroom]

reverts 9c055fb: inferring superview on the anchor versions was a huge mistake because it allows the anchor version of width/height to overload the constant version of width/height. First noticed when Vir was onboarding and it was calling the wrong one unexpectedly.

I'm a bit confused on the implications of this, does this mean that there are height/width constraints that weren't working [as intended]?

[gsbernstein]

I'm trying to remember the exact case the caused the confusion, maybe vir can help. I think it was that using the pattern of the other ones like top() or bottom() you'd expect width() to match the superview, but it was setting it to zero. I guess removing the default value from the dimensions version and making that param non-anonymous is enough, I could leave the superview stuff on the anchor version. [image: Screen Shot 2020-06-23 at 6.06.50 PM.png]

…

On Wed, Aug 19, 2020 at 3:44 PM Eric Groom ***@***.***> wrote: reverts 9c055fb <9c055fb>: inferring superview on the anchor versions was a huge mistake because it allows the anchor version of width/height to overload the constant version of width/height. First noticed when Vir was onboarding and it was calling the wrong one unexpectedly. I'm a bit confused on the implications of this, does this mean that there are height/width constraints that weren't working [as intended]? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABSY7NDCWF27BULJBVFKVOLSBRITFANCNFSM4P56AX4Q> .

[ericgroom]

I'm all for removing optionals where unnecessary, and default arguments (as those have bitten us more than once) but I think we need to find out how much of a breaking change this is. If I'm reading this correctly pretty much every height/width/size usage would have to be edited?

[ericgroom]

You should be able to omit the argument label, I think the bigger issue was that the anchor version had a default value of nil right?

[gsbernstein]

I don't think we ever use the default of zero anymore, I think I migrated away from that a while ago since it's very confusing. Changing the external parameter label would be an easy migration when I bump the pod to this commit, but maybe I could also make a deprecated wrapper for Axel's repo so anyone else using it could have time to migrate?

…

On Tue, Aug 25, 2020 at 3:20 PM Eric Groom ***@***.***> wrote: ***@***.**** commented on this pull request. I'm all for removing optionals where unnecessary, and default arguments (as those have bitten us more than once) but I think we need to find out how much of a breaking change this is. If I'm reading this correctly pretty much every height/width/size usage would have to be edited? ------------------------------ In constrain/Classes/BaseTypes.swift <#2 (comment)>: > @discardableResult - func height(_ constant: CGFloat = 0.0, by relationship: Relationship = .equal, priority: UILayoutPriority = .required) -> Constraints { + func height(to constant: CGFloat, by relationship: Relationship = .equal, priority: UILayoutPriority = .required) -> Constraints { You should be able to omit the argument label, I think the bigger issue was that the anchor version had a default value of nil right? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABSY7NFM7FLZNIXWU3V7HZDSCQ2L3ANCNFSM4P56AX4Q> .