-
Notifications
You must be signed in to change notification settings - Fork 87
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
Remove reference to functions as selections #32
Conversation
d7d0ad2
to
584cb2f
Compare
], footer: "This is a section footer."), | ||
Section(header: "Accessories", rows: [ | ||
Row(text: "None"), | ||
Row(text: "Disclosure Indicator", accessory: .DisclosureIndicator), | ||
Row(text: "Detail Disclosure Button", accessory: .DetailDisclosureButton(detailDisclosureButtonSelected)), | ||
Row(text: "Detail Disclosure Button", accessory: .DetailDisclosureButton({ | ||
[unowned self] in |
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.
We put this line on the same line as the opening brace in our app. I don’t really mind whichever way, but we should be consistent ✨
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.
Fixed!
This was causing memory leaks as the closure referenced self, without declaring it's ownership of self. From the Swift docs: "A strong reference cycle can also occur if you assign a closure to a property of a class instance, and the body of that closure captures the instance...[this] strong reference cycle occurs because closures, like classes, are reference types" By adding the [unowned self] to each selection, we remove any retain cycles.
584cb2f
to
c67a77f
Compare
Yay! Thanks for this, and the reference to the Swift docs. Wanna update the README too so other people don’t run into the same issues that we did? :D |
🍏 |
@ayanonagon Is there something specific in the README that you think should get updated? |
Section(header: "Money", rows: [
Row(text: "Balance", detailText: "$12.00", accessory: .DisclosureIndicator, selection: {
// Show statement
}),
Row(text: "Transfer to Bank…", cellClass: ButtonCell.self, selection: {
// Show transfer to bank modal
})
], footer: "Transfers usually arrive within 1-3 business days.") I guess that doesn’t reference |
@ayanonagon How's that? 812f8d3 |
It's minor but would you mind putting that example of presenting a view controller in the transfer to bank row instead? |
@hyperspacemark done. |
🍏 |
Remove reference to functions as selections
This was causing memory leaks as the closure referenced self, without declaring it's ownership of self.
From the Swift docs: "A strong reference cycle can also occur if you assign a closure to a property of a
class instance, and the body of that closure captures the instance...[this] strong reference cycle occurs
because closures, like classes, are reference types"
By adding the [unowned self] to each selection, we remove any retain cycles.