search

pointfreeco / isowords

Open source game built in SwiftUI and the Composable Architecture.
https://www.isowords.xyz
Other
2.7k stars 222 forks source link

use iPhone metrics when the iPad app is running in compact width (closes #128) #129

Open chucks opened 3 years ago

chucks commented 3 years ago

This PR changes the majority of the code that is checking for iPad UI idiom to code that checks for "effectively using iPad metrics", which I've defined as "iPad idiom and horizontal size class not compact". With this change in place, the app will select UI element sizes and layout values (e.g. padding) that are intended for iPhone when running on iPad in compact width.

This fix contains a shortcut that is probably fine for an app such as isowords, but is incorrect in the general case. Namely, the horizontal size class is tracked in the DeviceState struct, which captures the horizontal size class high up in the view hierarchy, whereas it is OK for intermediate views to manually override this value. Thus, the horizontalSizeClass value in the SwiftUI Environment on the view where you want to apply the metrics is the real "truth" and could diverge from what is in DeviceState. However, that makes the code a lot messier since it introduces another environment property into each of these views, plus each test about what metrics to apply becomes idiom == pad && horizontalSizeClass != compact.

This change affects metrics and layout in the broader app, it's possible I've introduced some visual regressions since I've not tested all the flows extensively.

I have noticed some visual glitching when doing some rotation and the app is in compact width. I'm not certain if this is a regression or some artifact of running in the simulator. I was not successful getting the code signing to be generic enough to run a local build of the app on my devices.

Simulator Screen Shot - iPad Pro (11-inch) (2nd generation) - 2021-08-17 at 13 12 21

If this problem is present when running on device, I don't mind working on it a little further if you have hints about where I should look.

An alternative "fix" for this problem would be to enable "requires full screen" in info.plist, which would stop supporting the multitasking scenarios completely.

stephencelis commented 3 years ago

Thanks for submitting this! We haven't had a chance to look into it yet, but will try to soon!