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

ReportbackItemDetailView -(CGFloat)heightForWidth:width #706

Merged
merged 4 commits into from
Dec 23, 2015

Conversation

aaronschachter
Copy link
Contributor

Closes #697 - renders Reportback Items with dynamic height in the Campaign Detail view controller.

Instead of using iOS self sizing cells, we manually calculate the height based on xib values in a new -(CGFloat)heightForWidth:width method.

I've been fighting with getting a sizing cell to return the appropriate height (#687, #688, #694), but it keeps returning sizes that are too large (and the results vary upon each load, making it even trickier).

Reviewed with @pixelrevision -- using good ol' math (similar to what we're already doing in the LDTCampaignDetailViewController sizeForCellAtIndexPath: iimplementation) seems just way more understandable and maintable at this point, vs fumbling around in the dark trying to get autolayout to magically calculate the numbers via layoutSubviews, layoutIfNeeded, etc.

@aaronschachter
Copy link
Contributor Author

Would make sense to remove the displayShareButton functionality and related hide/collapse to keep things tidy, as we're just manually determining whether or not to show it based on the height we return in -(CGFloat)heightForWidth:width

@aaronschachter
Copy link
Contributor Author

Going to keep the hide/collapse functionality in for now, as it could come in handy for kudos/props (#621)

aaronschachter added a commit that referenced this pull request Dec 23, 2015
ReportbackItemDetailView -(CGFloat)heightForWidth:width
@aaronschachter aaronschachter merged commit 6f2baff into develop Dec 23, 2015
@aaronschachter aaronschachter deleted the heightForWidth branch December 23, 2015 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant