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
Performance imporvement in UIView fillToSuperview() #540
Conversation
Generated by 🚫 Danger |
Codecov Report
@@ Coverage Diff @@
## master #540 +/- ##
=========================================
- Coverage 93.94% 93.9% -0.04%
=========================================
Files 67 67
Lines 2822 2823 +1
=========================================
Hits 2651 2651
- Misses 171 172 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rightAnchor.constraint(equalTo: superview.rightAnchor).isActive = true | ||
topAnchor.constraint(equalTo: superview.topAnchor).isActive = true | ||
bottomAnchor.constraint(equalTo: superview.bottomAnchor).isActive = true | ||
let left = leftAnchor.constraint(equalTo: superview.leftAnchor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @viktart :))
Can you explain how this implementation is more performant that the previous?
I believe the code is more clear this way, but can't see the performance gain here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The performance gain is because you're not supposed to activate constraints individually. It's better to activate them all at once using the NSLayoutConstraint.activate
API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🥇
@@ -29,6 +29,8 @@ The changelog for **SwifterSwift**. Also see the [releases](https://github.com/S | |||
- `removeFirst(where:)` array extension now is more generic `RangeReplaceableCollection` extensions. [#516](https://github.com/SwifterSwift/SwifterSwift/pull/516) by [LucianoPAlmeida](https://github.com/LucianoPAlmeida). | |||
- **RandomAccessCollection**: | |||
- `indices(of:)` array extension now is more generic `RandomAccessCollection` extensions. [#516](https://github.com/SwifterSwift/SwifterSwift/pull/516) by [LucianoPAlmeida](https://github.com/LucianoPAlmeida). | |||
- **UIView**: | |||
- `fillToSuperview()` should be more performant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, post merge regret. We should clean up this CHANGELOG entry in another PR 😕 My bad guys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, since I also approve my bad too :((
But it's simple I can fix it right :))
According to Apple's documentation: activating constraints in a bunch is typically more performant than activating them one by one.
Checklist