-
Notifications
You must be signed in to change notification settings - Fork 12
Add unit tests verifying that the presented controller's frame matches the expected frame #58
base: develop
Are you sure you want to change the base?
Conversation
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.
Some minor feedback on naming.
} | ||
} | ||
|
||
final class PresentationTransition: NSObject, TransitionWithPresentation { |
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.
Rename to "TestingPresentationTransition"?
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.
Done.
waitForExpectations(timeout: 0.1) | ||
|
||
XCTAssertEqual(window.rootViewController!.presentedViewController, presentedViewController) | ||
XCTAssertEqual(window.rootViewController!.presentedViewController?.view.bounds, window.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.
Does make sense to check the view's frame against the window's bounds? Also, this test method name should be updated to reflect frame/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.
Done.
func testNoFramesModifiedWhenThereIsAPresentationView() { | ||
let presentedViewController = UIViewController() | ||
let transition = PresentationTransition() | ||
let presentationView = 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.
Nit: designated iniitalizer initWithFrame?
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.
Done.
override var frameOfPresentedViewInContainerView: CGRect { | ||
if let presentationFrame = presentationFrame { | ||
return presentationFrame | ||
} |
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.
This hurts my head. I need to learn more swift.
This is a follow-up to #55.
Also fixes an existing test that said it was testing the non-animated route.