-
Notifications
You must be signed in to change notification settings - Fork 919
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
Add "atPosition:" variations to UICollectionView waitForCellAtIndexPa… #1109
Add "atPosition:" variations to UICollectionView waitForCellAtIndexPa… #1109
Conversation
Thanks, @harleyjcooper ! I've been pretty lousy with finding find to add the test. Btw, I'm wondering why tests fail on iPad simulator. Doesn't seem to be connected to this change. |
KIF Tests/CollectionViewTests.m
Outdated
@@ -59,6 +59,27 @@ - (void)testTappingLastItemAndSectionWithoutAnimation | |||
} | |||
|
|||
|
|||
- (void)testTappingLastAndFirstRowAtPositionWithAccessiblityIdentifier |
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 would rename this and the other test to testScrolling
since you're not really tapping anything
KIF Tests/CollectionViewTests.m
Outdated
{ | ||
UICollectionViewCell *lastCell = [tester waitForCellAtIndexPath:[NSIndexPath indexPathForRow:-1 inSection:-1] inCollectionViewWithAccessibilityIdentifier:@"CollectionView Tests CollectionView" atPosition:UICollectionViewScrollPositionBottom]; | ||
CGPoint lastPosition = [lastCell.superview convertPoint:lastCell.frame.origin toView:nil]; | ||
XCTAssertEqual(lastCell.frame.origin.x, lastPosition.x); |
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.
Is this a vertically scrolling collection view? If so, shouldn't you be checking here the y
position (whether it aligns to the top/bottom of the collection view bounds)?
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.
OK I'm comparing the y now. I'm a bit fuzzy on UIView coordinate conversion, but this seems to work.
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.
LGTM
This all looks totally reasonable to me. Thanks Harley! |
Taking over from @gaperlinski for #1061 :