Move section index to property #671
Conversation
|
@amonshiz updated the pull request - view changes |
Tests/IGListAdapterE2ETests.m
Outdated
| // [expectation fulfill]; | ||
| // }]; | ||
| // [self waitForExpectationsWithTimeout:15 handler:nil]; | ||
| //} |
| XCTAssertEqual(section11.sectionIndex, 0); | ||
| XCTAssertEqual(section12.sectionIndex, 0); | ||
| XCTAssertEqual(section21.sectionIndex, 1); | ||
| XCTAssertEqual(section22.sectionIndex, 1); |
There was a problem hiding this comment.
add more complex cases here?
There was a problem hiding this comment.
My preference would just be to 86 the stack controller ...
Tests/IGListAdapterTests.m
Outdated
| XCTAssertEqual([self.adapter sectionControllerForObject:@3].sectionIndex, 3); | ||
| XCTAssertTrue([self.adapter sectionControllerForObject:@3].isLastSection); | ||
| } | ||
|
|
There was a problem hiding this comment.
we should probably test: (1) append, (2) insert in middle
|
|
|
let's make sure we handle the cases that @Adlai-Holler mentioned in the issue |
|
All the existing tests pass, so at least we have those mutations covered. Would it be worth adding a list adapter mutations specific testing file? This one test was to cover the case mentioned in the task. However, speaking with Ryan it seems this shouldn't be passing currently so I will need to debug a bit. |
|
@amonshiz updated the pull request - view changes |
amonshiz
force-pushed
the
move-section-index-to-property
branch
from
9508684
to
e0f6a65
Compare
|
@amonshiz updated the pull request - view changes |
| @@ -38,7 +38,7 @@ IGLK_SUBCLASSING_RESTRICTED | |||
| @param objects The objects in the collection. | |||
| @param sectionControllers The section controllers that map to each object. | |||
| */ | |||
| - (void)updateWithObjects:(NSArray <id <NSObject>> *)objects sectionControllers:(NSArray <id <NSObject>> *)sectionControllers; | |||
| - (void)updateWithObjects:(NSArray <id <NSObject>> *)objects sectionControllers:(NSArray <IGListSectionController *> *)sectionControllers; | |||
There was a problem hiding this comment.
at some point this method was not updated to the IGListSectionController<IGListSectionType> * type and was missed in a few refactorings. the next method down, sectionControllerForSection: returned an IGListSectionController * so the types were a significant mismatch and needed to be updated.
| @@ -23,39 +23,39 @@ @implementation IGListSectionMapTests | |||
|
|
|||
| - (void)test_whenUpdatingItems_thatArraysAreEqual { | |||
| NSArray *objects = @[@0, @1, @2]; | |||
| NSArray *sectionControllers = @[@"a", @"b", @"c"]; | |||
| NSArray *sectionControllers = @[[IGListTestSection new], [IGListTestSection new], [IGListTestSection new]]; | |||
There was a problem hiding this comment.
all of these are just so that the test actually respects the types that IGListSectionMap require.
Generated by 🚫 Danger |
|
@amonshiz updated the pull request - view changes |
|
@amonshiz -- looks like this is good to go, except for resolving conflicts? |
|
will do! |
amonshiz
force-pushed
the
move-section-index-to-property
branch
from
f489471
to
ff5c938
Compare
|
@amonshiz updated the pull request - view changes |
|
@amonshiz has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@amonshiz updated the pull request - view changes - changes since last import |
|
@amonshiz has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
…eplace with sectionIndex property on IGListSectionController.
- Tests pass - Used Xcode 8.3, not sure if that impacts changes in `project.pbxproj` files.
… inserting at beginning. Also, tests `isFirstSection` and `isLastSection` across a head insert.
…to the IGListSectionMap update method.
…s associated section map) have a section index of NSNotFound. Added tests to ensure that too.
amonshiz
force-pushed
the
move-section-index-to-property
branch
from
0dc49b6
to
fd78480
Compare
|
@amonshiz updated the pull request - view changes - changes since last import |
|
@amonshiz has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Changes in this pull request
This pull request removes the
sectionForSectionController:method from theIGListCollectionContextprotocol so that the protocol is exclusively for presentation methods.This should not add new functionality, but rather makes the index directly accessible on the section controllers themselves. This change makes sense because at no time will there be an update to the list that the list adapter is unaware of and so it will always be able to set and update any indexes for a section controller that has changed.
Issue fixed: #609
Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.