Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setting trailing edge to inset has opposite of expected (and previous) behavior #316

Closed
2 tasks done
colinta opened this issue Sep 19, 2016 · 8 comments
Closed
2 tasks done

Comments

@colinta
Copy link
Contributor

colinta commented Sep 19, 2016

New Issue Checklist

Issue Info

Info Value
Platform ios
Platform Version 10.0
SnapKit Version 3.0.0
Integration Method cocoapods

Issue Description

This might be because

UIEdgeInsets are now have their right and bottom value inverted
(PS "are now have")

But the example that immediately follows that warning uses all positive values make.edges.equalTo(UIEdgeInsets(top: 10, left: 10, bottom: 10, right: 10)).inset(), which leads me to believe that I should continue to use positive values. Plus that makes sense, given that UIEdgeInsets use positive values to make rects smaller than their enclosing rect.

So given all that - should I be using negative values here? I hope that's not the case, I am re-using this UIEdgeInset in other calculations in my app, which all assume that positive values make the rect smaller.

@robertjpayne
Copy link
Member

@colinta for edges (left/right/top/bottom) UIEdgeInsets now behave as you described, this is a change from 0.22.0 and earlier in which the right/bottom were not inverted so you had to pass negative values.

Are you saying when doing like: make.leading.trailing.top.bottom.equalTo(UIEdgeInsets) it's not resulting in the same expected behaviour?

@colinta
Copy link
Contributor Author

colinta commented Sep 19, 2016

Interesting, so it sounds like they're supposed to work how I would expect. I just updated an app to use SnapKit 3, and one of my views was placed incorrectly. Here's the relevant code:

class MainScreen: UIView {
    struct Size: {
        static let tableMargins = UIEdgeInsets(top: 90, left: 20, bottom: 45, right: 20)
    }

        table.snp.makeConstraints { make in
            make.top.leading.trailing.equalTo(overlay).inset(Size.tableMargins)

What I expected (and this was the case in 0.22) was for the left and right edges to be 20pt from the overlay edges. Instead, the left side is placed correctly, but the right is not (it's 20pt off-screen).

I haven't tried the usage equalTo(insets).inset(), but in my case table is not a direct subview of overlay, so I'm not sure I can use that anyway.

@robertjpayne
Copy link
Member

@colinta the issue is probably leading/trailing are broken as a regression!

@colinta
Copy link
Contributor Author

colinta commented Sep 19, 2016

Ok, I'll see if I can help! I'll take a look at ConstraintConstantTarget.swift. At first glance, I noticed this:

if let value = self as? ConstraintInsets
...
    switch layoutAttribute {
    ...
    case .right, .rightMargin:
        return -value.right
    case .trailing, .trailingMargin:
        return (ConstraintConfig.interfaceLayoutDirection == .leftToRight) ?
            value.right :
            -value.left
    }
}

I see that .right uses -value.right, but .trailing uses value.right.

@robertjpayne
Copy link
Member

@colinta yea so this is tricky… It should probably be inverted for both if trailing edge?

@robertjpayne
Copy link
Member

e.g. UIEdgeInsets(left: 10, right: 10) in both cases the constant for the right edge should be -10

@robertjpayne
Copy link
Member

@colinta should be fixed in 3.0.1 release that just went out. Let me know if it persists!

@colinta
Copy link
Contributor Author

colinta commented Sep 20, 2016

Verified working! Thanks, Robert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants