search

DroidsOnRoids / let-swift-app

Let Swift Developers Meetup app
https://itunes.apple.com/us/app/let-swift/id1265027315?l=pl&ls=1&mt=8
Apache License 2.0
34 stars 10 forks source link

Use iOS 11 UISearchController to make it compatible with iPhone X #208

Open karolus opened 6 years ago

karolus commented 6 years ago

There were an issues with search bar on iPhone X, and the easiest way to fix it was to use the newest UISearchController. It looks nice and behaves well, plus it allowed to remove some offset-jugglery with header view.

It also needs to use iOS 11 as a minimum, but I don't think that's a problem with this kind of app.

Unfortunately it also introduced some warning due to deprecations in iOS 11, but I didn't want to make this PR bigger with unrelated to the main theme changes.

@m-chojnacki could we have Issues tab enabled so we can track know issues?

m-chojnacki commented 6 years ago

Thank you for your next contribution! I have enabled Issues tab. In my opinion, using UISearchController seems like a good idea because as you said it behaves better with current header view.

However, I would like to keep this pull request open for a little bit longer, because our "rule of thumb" regarding iOS versions support is "support the most recent iOS version, and one version back", so I'd rather wait with merging it till the iOS 12 preview is out. Keep in mind that almost half of our device test farm is still running iOS 10, so by merging it now I won't be able to test it on many of our physical devices, which is a crucial process in our QA workflow.

This shouldn't be a problem for now, because I've already fixed the search bar on iPhone X, as well as other related bugs, so the app is now usable on iPhone X.

sunshinejr commented 6 years ago

What exactly is needed by iOS 11, besides safeArea? @karolus @m-chojnacki

karolus commented 6 years ago

@sunshinejr searchController property on UINavigationItem. Now I see that PR title is misleading! Sorry! @m-chojnacki sure! Let's wait if that's necessary. Didn't know about your testing farm ;)

sunshinejr commented 6 years ago

No worries @karolus. We could probably use if #available to support iOS >= 11 and iOS < 11 differently, if you have time to do it. I agree that we should wait with withdrawing iOS 10 support until iOS 12.