Skip to content
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

Extended whisper background height to embrace the notch #193

Merged

Conversation

gilserrap
Copy link
Contributor

Well, I guess there's no perfect solution, but what would you think of having the whistles display all the way from the top of the screen on the iPhone X?

design proposal

@onmyway133 onmyway133 merged commit 154b24c into hyperoslo:master Nov 21, 2017
@onmyway133
Copy link
Contributor

onmyway133 commented Nov 21, 2017

@gilserrap Thanks for the PR, I think this is an acceptable solution for now

This was referenced Nov 21, 2017
@gilserrap
Copy link
Contributor Author

You're welcome!

@gilserrap gilserrap deleted the iPhoneX-whisper-cover-top-space branch November 21, 2017 13:53
public struct Dimensions {

static var notchHeight: CGFloat {
if UIApplication.shared.statusBarFrame.height > 20 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use safeAreaInsets here, if available?

@jondwillis
Copy link
Contributor

This has the disadvantage of covering the status bar on iPhone X, which is not necessary.

@gilserrap
Copy link
Contributor Author

@jondwillis You're right, I am not comfortable with this solution.
Actually I would prefer a solution like the one in the Snapchat app as you commented here: #173
But at the moment this was the most obvious fix for me, I am assuming that this is a temporary solution.

@jondwillis
Copy link
Contributor

See my comment on:
jondwillis@afcd471

@onmyway133
Copy link
Contributor

@jondwillis Hi, I've made the change regarding the window level, you can check https://github.com/hyperoslo/Whisper/releases/tag/6.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants