YLTableView 2.0 - YLTableViewCell as protocol #10
Conversation
…iewDataSource can use estimated heights provided by the cells, if the cells conform to the protocol (+ an alternative method to gather the height from a subclasses table view data source). Add caching mechanism for estimated heights.
| env: | ||
| - PROJECT=YLTableView.xcodeproj | ||
| - BUILD_SCHEME=YLTableView | ||
| - TEST_SCHEME=YLTableViewTests |
There was a problem hiding this comment.
These variables need to be under a global key: https://docs.travis-ci.com/user/environment-variables/#Global-Variables
Otherwise it'll matrix this and split it into 3 separate builds, one for each environment variable.
|
A travis configuration was added in #11, could you update this branch with master to get the build working? |
| @interface YLTableViewBaseTestCase : XCTestCase | ||
|
|
||
| //! Redeclaring to add the compiler warning if super is missing | ||
| -(void)setUp NS_REQUIRES_SUPER; |
There was a problem hiding this comment.
Can you add this for + (void)setUp and + (void)tearDown
…d definitions and declarations
| } | ||
|
|
||
| -(void)tearDown { | ||
| _tableView.delegate = nil; |
There was a problem hiding this comment.
i don't think you need to do this, if the @property references are cleaned up across test runs
… and used constant ints instead, removed unnecessary -tearDown in YLTableViewDataSourceTests.m
| #import <XCTest/XCTestCase.h> | ||
|
|
||
| //! New tests should subclass from this class as it contains contains all common test methods/properties. | ||
| @interface YLTableViewBaseTestCase : XCTestCase |
There was a problem hiding this comment.
I'd recommend against having a base test case class here. A base test case makes sense in our codebase and the super warnings are pretty convenient, but for a small open-source project that we want people to be able to pick up quickly and contribute to I feel like this just might add confusion. Especially since we only have one test case now, for now I'd say YAGNI (you ain't gonna need it)!
| they can override this method. | ||
| For more information, see tableView:estimatedHeightForRowAtIndexPath. | ||
| */ | ||
| return -1; |
There was a problem hiding this comment.
Let's just have this return UITableViewAutomaticDimension and get rid of the branching done in the calling method
…on by default instead of -1 to eliminate some unnecessary branching
…ace, and removed iOS 7.0 stuff
Ushhud
changed the title
| #import "YLTableViewDataSource.h" | ||
|
|
||
| @class YLTableViewCell; |
There was a problem hiding this comment.
this can be replaced with @protocol YLTableViewCell; and thus avoid the extra #import, if you're so inclined
|
A few very, very minor issues, but overall looks good – this is much lighter-weight than the previous version and I'm super excited for that. Fix and ship! |
YLTableView 2.0 - YLTableViewCell as protocol
Change YLTableViewCell to a protocol from an interface.
Add caching mechanism for estimated heights.
Remove code used for iOS 7.