-
Notifications
You must be signed in to change notification settings - Fork 576
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
Fixed handling of BFIncludeStatusBarInSizeAlways #129
Fixed handling of BFIncludeStatusBarInSizeAlways #129
Conversation
XCTAssert(sizeThatFitsIncludingStatusBar.height > sizeThatFitsNotIncludingStatusBar.height); | ||
} | ||
|
||
|
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: Remove unneeded whitespace.
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.
The usage of blank lines is similar in the other tests of that file. Wouldn't want to break the coding style here.
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.
Somewhat. There are 2 blank lines here, compared to a single line between every other test here.
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.
Thanks. Got it now – after believing first you meant the blank lines within the tests, not the 2 after it.
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.
Do you consider it best to restore the blank lines within the tests?
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.
It's ok if you remove them. Looks good enough 😁
LGTM, passing to @mingflifb for review. |
f32670b
to
45bc71f
Compare
45bc71f
to
eda11b9
Compare
Fixed handling of BFIncludeStatusBarInSizeAlways
@widescape updated the pull request. |
When includeStatusBarInSize was set to BFIncludeStatusBarInSizeAlways, it didn't actually include the status bar in its size.