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

Fix isZoomed method #195

Closed
wants to merge 1 commit into from
Closed

Fix isZoomed method #195

wants to merge 1 commit into from

Conversation

vvit
Copy link

@vvit vvit commented Aug 10, 2019

iOS 12.4 iPhone6 Plus Simulator reports UIScreen.main.nativeScale = 3 for non-zoomed display.
Possbile resolution: add < 3 checking.

@vvit vvit mentioned this pull request Aug 10, 2019
@devicekit-danger-bot
Copy link

devicekit-danger-bot commented Aug 10, 2019

4 Warnings
⚠️ Plist changed, don’t forget to localize your plist values
⚠️ Source/Device.generated.swift#L625 - Prefer non-optional booleans over optional booleans.
⚠️ Source/Device.generated.swift#L627 - TODOs should be resolved (Longterm we need a better solu…).
⚠️ Source/Device.generated.swift#L1200 - Prefer empty collection over optional collection.

SwiftLint found issues

Warnings

File Line Reason
Device.generated.swift 625 Prefer non-optional booleans over optional booleans.
Device.generated.swift 1200 Prefer empty collection over optional collection.
Device.generated.swift 627 TODOs should be resolved (Longterm we need a better solu...).

Generated by 🚫 Danger

@Zandor300
Copy link
Member

Sorry for the long wait.

When I try it on my iPhone 6s Plus, the current version of DeviceKit (without the fix) works just fine. It is running iOS 13 Beta 8 though...

Here is my test project: https://git.zsinfo.nl/Zandor300/devicekitiszoomedtest

@vvit
Copy link
Author

vvit commented Aug 28, 2019

@Zandor300 thanks, maybe my simulator installation is corrupted. I'll check. You may close the PR for now without merging. Have a nice day.

@vvit vvit closed this Aug 28, 2019
@Zandor300
Copy link
Member

@vvit I want to reopen this but can't since you've removed your fork.

Your change actually removes the need to explicitly force the variable to return false for iPhone X and iPhone XS, so if it doesn't affect the rest (and maybe fix your issue), I would actually like to have this merged to master.

@Zandor300
Copy link
Member

I've updated the test project to print some values. These are the results from my end:

device:      iPhone 6s Plus
isZoomed:    true (actually true)
nativeScale: 2.88
scale:       3.0

device:      iPhone 6s Plus
isZoomed:    false (actually false)
nativeScale: 2.6086957
scale:       3.0

device:      iPhone X
isZoomed:    false (only allows false)
nativeScale: 3.0
scale:       3.0

The thing in brackets behind is isZoomed is what I actually set the interface to.

I've locally changed the source of the DeviceKit pod and recompiled with the following:

    public var isZoomed: Bool? {
      guard isCurrent else { return nil }
      if Int(UIScreen.main.scale.rounded()) == 3 {
        // Plus-sized
        return UIScreen.main.nativeScale > 2.7 && UIScreen.main.nativeScale < 3
      } else {
        return UIScreen.main.nativeScale > UIScreen.main.scale
      }
    }

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