updateConstraints installs two width constraints instead of updating it about masonry HOT 13 CLOSED

This is an expected behaviour. mas_updateConstraints: is suitable for lightweight updates where you only need to change a constraint constant. However, equalTo(@760) translates to NSLayoutConstraint with second item = nil and constant = 760, and equalTo(self) translates to second item = self and constant = 0. Therefore, these constraints cannot be matched.

There are two approaches to this in Masonry. First, you can use a MASConstraint property to store created constraint and uninstall it when you need to update:

@property (nonatomic, strong) MASConstraint *containerWidthConstraint;
...

[self.containerView mas_makeConstraints:^(MASConstraintMaker *make) {
        [self.containerWidthConstraint uninstall];

        if (something) {
            self.containerWidthConstraint = make.width.equalTo(@760);
        } else {
            self.containerWidthConstraint = make.width.equalTo(self);
        }
}];

Another approach is to simply use mas_remakeConstraints: method which uninstall all constraints previously created for this view:

[self.containerView mas_remakeConstraints:^(MASConstraintMaker *make) {
        if (something) {
            make.width.equalTo(@760);
        } else {
            make.width.equalTo(self);
        }
}];

from masonry.

Aah.. thanks for clearing this up Nick. Also, i didn't know your new remakeConstraints function, really useful, just what i needed so many times :)

I am closing the issue, although i kinda still think it is an issue. When remaking a new constraint for e.g. width, you expect that it overrides the old one regardless of the parameter given to equalTo. It's an implementation specific behaviour, that a wrapper API like Masonry should not expect the user to know about.

from masonry.

Yeah, it's been added not so long ago (in v0.5). I personally love it because it covers a lot of simple update cases :)

from masonry.

Oh, and I agree that this is a bit confusing. @cloudkite Maybe we should consider a change?

from masonry.

@nickynick I think it would be difficult to achieve without possibly creating more edge cases. Mainly due to composite constraints

consider the following all create a constraint with width == 760

make.size.equalTo(CGSizeMake(760, 760));
make.width.height.equalTo(760)
make.width.equalTo(@[ @760, otherview, someView.width ]);
make.width.equalTo(760);

however would you expect the following to update the only the last OR all of the above?

make.width.equalTo(otherview.width);

Happy to be convinced otherwise :)

from masonry.

@cloudkite I feel like composite constraints are nothing more but a shorthand way of doing things, e.g. make.size.equalTo(CGSizeMake(100, 200) is identical to make.width.equalTo(100) + make.height.equalTo(200).

I think it would make sense to match constraints by the attribute only. E.g., consider this example:

// constraints before:
make.size.equalTo(@[ someView, [NSValue valueWithCGSize:CGSizeMake(100, 200)] ]);
make.width.greaterThanOrEqualTo(50);

// update:
make.width.equalTo(42);

// constraints after:
make.height.equalTo(@[ someView, @200 ]);
make.width.equalTo(42);

The way things are now, I never find myself using updateConstraints: because of its limitations. It also doesn't feel intuitive, i.e. I made the same mistake first time I tried to use it.

from masonry.

@nickynick what do you think about making updateConstraints similar to remakeConstraints except that it first finds constraints it can update using layoutConstant. After which it removes the remaining existing constraints and installs constraints which are not eligible to be updated?

from masonry.

@cloudkite Wouldn't it be the same as remakeConstraints: then? I probably missed your point, an example, maybe? :)

from masonry.

ok yeah i misunderstood what you suggested in:

// constraints before:
make.size.equalTo(@[ someView, [NSValue valueWithCGSize:CGSizeMake(100, 200)] ]);
make.width.greaterThanOrEqualTo(50);

// update:
make.width.equalTo(42);

// constraints after:
make.height.equalTo(@[ someView, @200 ]);
make.width.equalTo(42);

So in this case update would reduce three constraints referencing width into one?

from masonry.

Yep, basically when you want to update a constraint for a certain attribute, you'd want to throw away all the previous constraints for this attribute because they'd mess up your updated layout. If you need them, you can provide them again. Like, in the example above, you can write make.width.equalTo(@[ someView, @42 ]); to get 2 constraints instead of 3, one of which would be the same.

from masonry.

I'm not sure if this is the right thing to do™, but it's definitely the easier way to do updates, and probably more intuitive.

from masonry.

Yeah sounds reasonable, I think its more intuitive. In most cases you wouldn't have more than one constraint referencing a particular attribute anyway.

I do very occasionally use something lie make.width.greaterThanOrEqualTo(50) to specify minimums.
However I guess you can supply these again to keep them around.

Would still want updateConstraints to attempt to use layoutConstant when possible for better performance

// constraints before:
make.size.equalTo(52);
make.width.greaterThanOrEqualTo(50);

// update:
make.width.equalTo(42); //use layoutConstant
make.width.greaterThanOrEqualTo(40); //use layoutConstant
make.height.equalTo(otherview); //uninstall existing, and install new constraint

from masonry.

Yeah, it's a nice optimization. We should try to reuse existing stuff if it's possible.

Okay then, looks like an upcoming change for v0.6? Worth to note that this is a breaking change, however it's probably not a severe one.

from masonry.