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

Add support for update existing constraint from install(). #55

Closed
wants to merge 3 commits into from
Closed

Add support for update existing constraint from install(). #55

wants to merge 3 commits into from

Conversation

devxoul
Copy link
Contributor

@devxoul devxoul commented Jan 9, 2015

Concept

1. Update constant

If the new constraint is exactly the same with existing, layout() will just update a constant of existing constraint.

layout(view) { view in
    view.top == view.superview!.top + 10
    view.left == view.superview!.left + 10
    view.width == 200
}

// just update constants
layout(view) { view in
    view.top == view.superview!.top + 20
    view.width == 300
}

2. Replace existing constraint

If the new constraint has different relation from existing, layout() will uninstall existing and install the new one.

layout(view) { view in
    view.left == someView.top.left + 10
}

// replace existing constraint with new one
layout(view) { view in
    view.left == anotherView.top.left + 50
}

Animations

Cartography doesn't support animations currently. Because Cartography immediately layout subviews after constraints are installed.

Use constrain() instead of layout() for animations, as @robb mentioned. What we need to do is to call layoutIfNeeded() in animation block.

For example:

// use `constrain()` instead of `layout()`
constrain(view) { view in
    view.left == view.superview!.left + 200
}

// layout in animation block
UIView.animateWithDuration(0.5, animations: {
    view.layoutIfNeeded()
})

Or you can pass view.layoutIfNeeded to an animation block:

UIView.animateWithDuration(0.5, animations: view.layoutIfNeeded)

I'll make a PR for this as soon as possible.

Related Issues

@robb
Copy link
Owner

robb commented Jan 9, 2015

Thanks for the pull request 💖

However, I've looked into this before but I considered the implicit replacement of constraints to yield to many edge cases. Consider this spec:

import Cartography
import XCTest

class UpdateTests2: XCTestCase {
    var superview: View!
    var view1: View!
    var view2: View!

    override func setUp() {
        superview = View(frame: CGRectMake(0, 0, 400, 400))

        view1 = View(frame: CGRectZero)
        superview.addSubview(view1)

        view2 = View(frame: CGRectZero)
        superview.addSubview(view2)
    }

    func testUpdate() {
        layout(view2) { view2 in
            view2.height == 300
            view2.width  == 300
        }

        layout(view1, view2) { view1, view2 in
            view1.center == view2.center

            view1.width  == view2.width
            view1.height == view2.height
        }

        layout(view1) { view1 in
            // This should replace `view1.height == view2.height`
            view1.width == 200; return
        }

        XCTAssert(CGRectGetWidth(view1.frame) == 200, "It should update existing constraint")
    }
}

You would expect view1.width == 200; to replace the earlier view1.height == view2.height constraint, but since that one is attached to view1.superview, we get unsatisfiable constraints – a symbolic breakpoint in UIViewAlertForUnsatisfiableConstraints confirms this.

(Note that the spec may actually pass if UIKit decides to break the right constraint. For me it breaks view2.width == 300 which is definitely not what I wanted)

Because Cartography immediately layout subviews after constraints are installed.

Have you tried the new (and undocumented 😔) constrain function?

You should be able to do something like

// this won't call `setNeedsLayout()`
constrain(view) { view in
    view.left == view.superview!.left + 200
}

// layout in animation block
UIView.animateWithDuration(0.5, animations: view.layoutIfNeeded)

@devxoul
Copy link
Contributor Author

devxoul commented Jan 9, 2015

@robb

Oh, that isn't what I want to do. What I wanted to make layout function to do was 'Create new constraints or update existing'. Not replace whole constraints.

For example:

layout(view1, view2) { (view1, view2) in
    view1.top == view2.top + 10 // (1)
    view1.left == view2.left + 10 // (2)
}
// at this time:
// (1) view1.top = view2.top + 10
// (2) view1.left = view2.left + 10

layout(view1, view2, anotherView) { (view1, view2, anotherView) in
    view1.top == anotherView.bottom + 20 // (3) update
    view1.height == view2.height - 10 // (4) create new one
}
// then:
// (1) -> (3) view1.top = anotherView.bottom + 20 (replaced)
// (2) view1.left = view2.left + 10 (existing, not changed)
// (4) view1.height = view2.height - 10 (newely created)

I've just tried constrain function, it works perfectly as I expected :D

@robb
Copy link
Owner

robb commented Jan 9, 2015

What I wanted to make layout function to do was 'Create new constraints or update existing'. Not replace whole constraints.

Hmm, not sure I follow. Let's stick with width and height since those actually allow referencing constants, where I think most of the confusion comes into play. Consider this example:

layout(view1, view2) { (view1, view2) in
    view1.width  == view2.width  - 10
    view1.height == view2.height - 10
}

layout(view1, view2, anotherView) { (view1, view2, anotherView) in
    view1.width  == anotherView.width + 20
    view1.height == view2.height - 20
}

Assuming view1, view2 and anotherView and all share the same common superview, this works as expected.

However, just from looking at it, I would expect this to work as well:

layout(view1, view2) { (view1, view2) in
    view1.width  == view2.width  - 10
    view1.height == view2.height - 10
}

layout(view1, view2) { (view1, view2) in
    view1.width  == 100
    view1.height == view2.height - 20
}

which doesn't because of the aforementioned reasons.

I've just tried constrain function, it works perfectly as I expected :D

👌

@devxoul
Copy link
Contributor Author

devxoul commented Jan 9, 2015

@robb, you're right. I didn't catch that 😲

Then, how about 'create new constraint or update constant of existing'?

@robb
Copy link
Owner

robb commented Jan 9, 2015

Hmm, I'm not sure this will do more harm than good. What part of an expression ends up being updatable is more of an implementation detail of Auto Layout. I think allowing constraints to be (partially) updated is going to be very confusing.

E.g. this would work:

constrain(view1) { view1 in
    view1.width  == 100
    view1.height == 100
}

constrain(view1) { view1 in
    view1.width  == 200
    view1.height == 200
}

and this would work:

constrain(view1, view2) { view1, view2 in
    view1.width  == view2.width  + 100
    view1.height == view2.height + 100
}

constrain(view1, view2) { view1, view2 in
    view1.width  == view2.width  + 200
    view1.height == view2.height + 200
}

but this wouldn't

constrain(view1) { view1 in
    view1.width  == 100
    view1.height == 100
}

constrain(view1) { view1 in
    view1.width  == view2.width  + 200
    view1.height == view2.height + 200
}

Then there's stuff like:

let leftColumnHeight: Double = // …

constrain(marginView) { marginView in
    marginView.height >= leftColumnHeight
}


let rightColumnHeight: Double = // …

constrain(marginView) { marginView in
    marginView.height >= rightColumnHeight
}

Would this update the height constraint or create a second one? I have no clue….

I think a better way to update constraints is capturing them, e.g.:

var width: NSLayoutConstraint? = nil

constrain(view1) { view1 in
    width = view1.width == 100
}

// Later

width.constant == 200

which can get unwieldy but it overall simpler, imho.

@devxoul
Copy link
Contributor Author

devxoul commented Jan 9, 2015

Totally agree with that, but how about this approaches?

  1. Capture referencing views:
// block A (init block)
constrain(view1) { view1 in
    view1.width  == view2.width  + 200
    view1.height == view2.height + 200
}

// block B (update block)
constrain(view1) { view1 in
    view1.width  == 100
    view1.height == 100
}

At block A, constraints installed on view1.superview. We can capture view1.superview from view1 at install time, so at block B, we can use captured view to find existing constraints. I just wrote some prototype and it seems to be working.

  1. Update layout explicitly:
let leftColumnHeight: Double = // …

// force create
layout(marginView) { marginView in
   marginView.height >= leftColumnHeight
}


let rightColumnHeight: Double = // …

// force update
updateLayout(marginView) { marginView in
   marginView.height >= rightColumnHeight
}

Capturing each constraints is the most simple way to update constants, but it's too messy if there are many UI elements and various exceptional cases.

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

Successfully merging this pull request may close these issues.

None yet

2 participants