-
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
Bugfix: SectionFooterView AutoDimension Support #110
Bugfix: SectionFooterView AutoDimension Support #110
Conversation
Hi Neal, is this change friendly to your issue you saw? Care to test with this branch? |
@hyperspacemark @eliperkins @rhaining for further review |
Static/Section.swift
Outdated
@@ -13,6 +13,9 @@ public struct Section: Hashable, Equatable { | |||
/// Custom view for the header or footer. The height will be the view's `bounds.height`. | |||
case view(UIView) | |||
|
|||
// View Sized with autolayout | |||
case autoLayoutView(UIView) |
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.
I'm definitely not tied to this naming since in my current case name, but wanted to convey this would handle automatic dimensioning rather than use the views set bounds.height. Only added a new case to prevent previous integration breaks.
My current use case adds stackViews without constraints, but ended up using this to not break existing .view(_) sizing.
@rhaining @hyperspacemark Finally found my error. This PR works in supporting Autolayout FooterViews in iOS10/11. BUT: Example Working: |
Static/Section.swift
Outdated
@@ -13,6 +13,9 @@ public struct Section: Hashable, Equatable { | |||
/// Custom view for the header or footer. The height will be the view's `bounds.height`. | |||
case view(UIView) | |||
|
|||
// View Sized with autolayout | |||
case autoLayoutView(UIView) |
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.
If using this view, must set estimated header/footer height to > 0, if using tableView's before iOS11
2d8c055
to
285bcf2
Compare
Static/DataSource.swift
Outdated
headerHeight = UITableViewAutomaticDimension | ||
} | ||
return headerHeight | ||
return section(at: sectionIndex)?.header?.viewHeight ?? UITableViewAutomaticDimension |
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.
i think the thing we lose here is that if there's no header defined for this section, we're returning 'automatic dimension' now, versus in the previous code, we're returning 0
in that case.
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.
maybe this returns 0
if the table style is 'plain', and auto dimension if the table style is 'grouped' ??
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.
Double checking plain vs grouped once more
Static/DataSource.swift
Outdated
footerHeight = UITableViewAutomaticDimension | ||
} | ||
return footerHeight | ||
return section(at: sectionIndex)?.footer?.viewHeight ?? UITableViewAutomaticDimension |
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.
similar question here
Problem: 1. I noticed our app was manually laying out subviews in our sectionFooters when I didn't see the need to. 2. After recent changes, Group tableView's which had nil section extremity defaulted to zero, which drops the standard spacing between sections. Note in attatchment. Notable info: Statics currently has deploy target of iOS8, which did not yet support automatic sectionFooterView dimensioning. Solution: - Offer autosizing option which uses UITableViewAutomaticDimension rather than view's original bounds height. Retain original .view(_) case in the event some clients were already sizing beforehand limiting integration breaks. This also offers automatic margin adherence. - Bump deploy target to iOS9 for clean integration. - Revert 2 previous PRs - Default height for plain/grouped styles to play friendly with tableView's defaults.
285bcf2
to
58fcb2f
Compare
Followed up with a change for default heights. By returning autodimension height by default, plain tableViews were taking non-standard heights with nil extremities. Update to use a default height between styles which performs equivalent to default handling if nil vs filled extremities Also updated Example code with example of how to create an autosized extremity |
Problem:
eg.. (Unnecessarily maintaining and calculation including insets/frame)
Notable info:
Statics currently has deploy target of iOS8, which did not yet support automatic sectionFooterView dimensioning.
Solution:
Testing:
Tested with simulators
Did not test on iOS 9