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

Added mas_remakeConstraints #63

Merged
merged 2 commits into from
Apr 29, 2014
Merged

Conversation

nickynick
Copy link
Contributor

When working with Masonry, sometimes there is a need to update existing constraints for a view to do some kind of animation or whatever. mas_updateConstraints is only capable of updating constraint constants, so for more advanced changes we are forced to store created constraints in a separate array and then call uninstall on each prior to creating new ones.

This is quite tedious, so I had an idea for a new method, mas_remakeConstraints. It is very simple: it removes all constraints previously defined and installed for the view, allowing you to provide replacements without hassle.

Under the hood, it stores a MASViewConstraint in a private associated set of its firstViewAttribute.view, so it's quite simple implementation-wise too.

TODO list:

  • tests :)

@cloudkite
Copy link
Contributor

This would be very handy no doubt! however there are a few pitfalls with this.

[superview.subview remakeConstraints:^(MASConstraintMaker *make){
   make.top.equalTo(100); //this constraint actually gets added to superview
}];

// superview constraints will get removed, meaning the above constraint gets deleted
[superview remakeConstraints:^(MASConstraintMaker *make){
   make.width.equalTo(10);
}];

The other reservation I have is that this adds a strong reference between the view and constraint. Where as before this relationship is managed by UIKit. This means constraints which would normally be deallocated when UIKit automatically removes constraints ie in response to removeFromSuperview will now not be deallocated.

@cloudkite
Copy link
Contributor

Whoops ignore the last comment assumed you where holding onto NSLayoutConstraints not MASViewConstraints. Nice work looks good 👍

@cloudkite
Copy link
Contributor

I guess one thing to think about, is there is now a little bit of extra memory overhead. As MASViewConstraint and MASCompositeConstraint instances will now hang around, previously they would have been deallocated by ARC after a makeConstraints, updateConstraints etc block has been completed.

However they are quite light-weight so don't think it would be a big deal?

@nickynick
Copy link
Contributor Author

Well yeah, each constraint is merely a hundred bytes, so I don't think it is a problem. There should be literally thousands of constraints for them to take up any considerable amount of memory, and if you have a thousand of constraints, then you are likely screwed in other ways already :)

@cloudkite
Copy link
Contributor

Yeah sounds good :)

@nickynick
Copy link
Contributor Author

Added some tests, guess it should be fine :)

@cloudkite
Copy link
Contributor

Nice 👍 thanks for the contribution :)

cloudkite added a commit that referenced this pull request Apr 29, 2014
@cloudkite cloudkite merged commit e529574 into SnapKit:master Apr 29, 2014
@nickynick nickynick deleted the remake-constraints branch April 29, 2014 19:25
dan-rodrigues pushed a commit to dan-rodrigues/Masonry that referenced this pull request Oct 18, 2014
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.

2 participants