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

iOS 11 & iPhone X #173

Closed
Droppix opened this Issue Sep 25, 2017 · 12 comments

Comments

Projects
None yet
8 participants
@Droppix

Droppix commented Sep 25, 2017

Don't work with iOS 11 and iPhone X.
New release ?

@AndrewGable

This comment has been minimized.

AndrewGable commented Sep 26, 2017

I agree, we are having issues with this on the iPhone X simulator. Here is one example of when we show aMurmur

30352968-132948fe-97d8-11e7-9e8c-e41d7348d346

Do you have any ides on how to fix this?

@onmyway133

This comment has been minimized.

Member

onmyway133 commented Sep 26, 2017

@Droppix @AndrewGable Hi, this is known issue. We 'll try to fix this during the swift 4 migration

@AndrewGable

This comment has been minimized.

AndrewGable commented Sep 26, 2017

Awesome, is the proposed fix to extend the Murmur completley bellow the notch?

@derikflanary

This comment has been minimized.

derikflanary commented Sep 26, 2017

May I ask when the Swift 4 migration is expected to be completed?

@wowlocal

This comment has been minimized.

Contributor

wowlocal commented Oct 7, 2017

Hey guys, i fix this here #179

@zenangst

This comment has been minimized.

Contributor

zenangst commented Oct 17, 2017

Hey folks, I just merged #179 into master. This seems to do the trick when I tested, please confirm that it works for you guys :) I also med #183 to reduce some code-duplication, that will be merged shortly.

@zenangst

This comment has been minimized.

Contributor

zenangst commented Oct 17, 2017

@derikflanary I think when #183 is merged we can make a new 5.x version and then merge one of the swift4 PR's and make a 6.x release.

@onmyway133

This comment has been minimized.

Member

onmyway133 commented Oct 30, 2017

@Droppix @wowlocal @derikflanary Hi, you can check https://github.com/hyperoslo/Whisper/releases/tag/6.0.0. This is mostly language update, and small fixes to make it look nice on iPhone X. There's some other issues and PRs that we will try to deal with soon

@barnaclejive

This comment has been minimized.

barnaclejive commented Oct 31, 2017

Was this tested on non-iPhone X? I just pulled tag 6.0.0 and ran it on an iPhone 8 Simulator and the Murmur will drop down below the status bar. I assume this is how it has to work on iPhone X, but on other devices it should still just drop into the status bar area, not below it.

simulator_screen_shot_-iphone_8-_2017-10-31_at_12_16_52

@jondwillis

This comment has been minimized.

Contributor

jondwillis commented Nov 4, 2017

@barnaclejive

screen shot 2017-11-04 at 2 11 47 pm

I have implemented Whistles in my own branch such that they appear under the status bar on iPhone X, but extend all the way to the top of the screen. Additionally, I have also preserved the behavior where they appear over the status bar on other devices.

See commit: jondwillis@afcd471

Someone should disentangle and clean up my fork and do a pull request if they wish to see this behavior in the mainline. In my application, safeAreaInsets do not change, but to be production-ready, the final version of this should observe safeAreaInsetsDidChange

Though I subjectively like extending from the top edge on iPhone X, doing this may be against Apple's guidelines, and may result in a App Store review rejection. I have noticed that snapchat changed its Whistle-like implementation on iPhone X to look kind of like a push notification does; it is below the status bar, and does not touch any edges.

Shouts/Announcements also need to be fixed for iPhone X and AFAIK that hasn't been done.

For reference here is what Snapchat is doing:

img_0029

@jondwillis

This comment has been minimized.

Contributor

jondwillis commented Nov 14, 2017

Update: I was able to publish an app with this Whistle behavior to the App Store. So, anecdotally, it is acceptable under Apple's Guidelines.

@onmyway133

This comment has been minimized.

Member

onmyway133 commented Nov 21, 2017

@barnaclejive Hi, can you check https://github.com/hyperoslo/Whisper/releases/tag/6.0.2 which merged #193
@jondwillis We will go with full notch extend for now 👍
I will close this in favor of #188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment