search

chadaustin / is-it-snappy

iOS App for measuring input-to-output latency
Other
106 stars 8 forks source link

Fix support for iPhone X and newer and iOS 14 #9

Closed lhecker closed 3 years ago

lhecker commented 3 years ago

Please let me know what you think about this PR. 🙂👍

chadaustin commented 3 years ago

Great, thanks! I pushed three of your four commits, with a couple tweaks. I'm not opposed to the binary search, but I have to think about it some before pushing.

lhecker commented 3 years ago

Thanks for merging my commits! I know that some of them weren't quite necessary.

The binary search finds the rightmost Element which is not greater than time. This is equal to your previous code. Although maybe one could add a check ensuring that index isn’t negative, considering that it can theoretically be -1 if time is negative.

I have also migrated your project to Swift 5 by the way. But I also went a step further and "modernized" it to fully support dark mode and non-fullscreen modals. If you're fine with it, I'd be happy to contribute my changes to you! Unfortunately though, they're a bit more involved, due to the changes necessary for supporting dark mode.

You can find my branch here: https://github.com/lhecker/is-it-snappy/tree/modernize Changing the build target to iOS 14 isn't strictly necessary - I just did it, to see the deprecation warnings. Most of the changes to project.pbxproj happened simply, because I copied the current default Xcode 12 project settings over to your project to see what has changed in the last couple years. Due to that the branch contains some further changes for the Swift 5 migration to fix those pesky warnings (second to last commit). The last commit adds support for dark mode. At least it should. I only split up the commits retroactively. 😅

P.S.: In your recent commits you added helper functions with the following description:

// Helper function inserted by Swift 4.2 migrator.

If you look at them closely you'll notice that they're actually not necessary at all. I don't even understand why the migrator adds them in the first place. 🤨