hyperoslo / Sugar

:coffee: Something sweet that goes great with your Cocoa
https://github.com/hyperoslo
Other
1.07k stars 69 forks source link

Fix the use of insets #101

Closed onmyway133 closed 7 years ago

onmyway133 commented 7 years ago

Trailing and bottom anchors should be using negative, that's what we want :)

mention-bot commented 7 years ago

@onmyway133, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vadymmarkov to be a potential reviewer.

vadymmarkov commented 7 years ago

It kinda makes sense, but I'm not sure. It's a super breaking change for me.

onmyway133 commented 7 years ago

@vadymmarkov yeah I see. But then we should have another struct instead of UIEdgeInsets, the behaviour here is different from what we know from CGRect

onmyway133 commented 7 years ago

Or we can have insets like

extension UIEdgeInsets {

  init(constraintInsets value: CGFloat) {
    top = value
    left = value
    bottom = -value
    right = -value
  }
}
zenangst commented 7 years ago

@onmyway133 so with this change, you would never set negative values on your constraints?

onmyway133 commented 7 years ago

@zenangst I think for using insets like this, most of the cases, users expect it to behave like insets in CGRect, like an inner padding. So we try to help them by proactively adding - for them. But I'm biased on this too

zenangst commented 7 years ago

@onmyway133 that all depends, the normal case for adding constraints is to actually do - when constraining it to the bottom with a bit of margin isn't it?

onmyway133 commented 7 years ago

@zenangst you're right. But this is a helper, so it tries to help :)

zenangst commented 7 years ago

@onmyway133 remember the convenience methods in Spots? They don't always help if you have to question how they work 😁

zenangst commented 7 years ago

I do agree that constraints are backwards to begin with, but using a wrapper for constraints, I would assume that they would still use the same logic. Just my 50 cents :)

onmyway133 commented 7 years ago

@zenangst Right. I think I will close this for now. It is a bit too clever