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.

Sign up for GitHub

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

Make Device.swift usable from objc #224

Merged
merged 4 commits into from
Feb 1, 2018
Merged

Make Device.swift usable from objc #224

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4d8a8fb
Device.swift: make it a class usable from objc
manuroe Jan 29, 2018
4a3052d
Device.swift: Add human readable platform names for iPhone 8, 8 Plus …
manuroe Jan 29, 2018
27ed125
Device.swift: Make nativeScreenSize return CGSize.zero if the value i…
manuroe Jan 30, 2018
d6e196a
Merge remote-tracking branch 'origin/develop' into feature/make_devic…
manuroe Jan 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions PiwikTracker/Device.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
public struct Device {
import Foundation

final public class Device: NSObject {
/// Creates an returns a new device object representing the current device
public static func makeCurrentDevice() -> Device {
@objc public static func makeCurrentDevice() -> Device {
let platform = currentPlatform()
let humanReadablePlatformName = humanReadablePlatformNameForCurrentDevice()
let os = osVersionForCurrentDevice()
Expand All @@ -14,20 +16,30 @@ public struct Device {
}

/// The platform name of the device i.e. "iPhone1,1" or "iPad3,6"
public let platform: String
@objc public let platform: String

/// A human readable version of the platform name i.e. "iPhone 6 Plus" or "iPad Air 2 (WiFi)"
/// Will be nil if no human readable string was found.
public let humanReadablePlatformName: String?
@objc public let humanReadablePlatformName: String?

/// The version number of the OS as String i.e. "1.2" or "9.4"
public let osVersion: String
@objc public let osVersion: String

// The screen size
public let screenSize: CGSize
@objc public let screenSize: CGSize

// The native screen size
public let nativeScreenSize: CGSize?
@objc public let nativeScreenSize: CGSize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You changed the nativeScreenSize to non-optional, but for macOS it is not defined. Correct me if I am wrong on that point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My issue was that optional non class parameter like CGSize? cannot be represented in objc. I got a build error when prefixing it with @objc.
As a Swift rookie, I decided to remove the optionality. Could you advice what is the best thing to do in that situation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see your issue there. Yeah. Only pointers to objects can be represented as optionals in Objective-C (null pointer then).

Since making this a pointer would be very weird I think there would be two solutions. Either define "no native screen size is defined" as CGSize.zero or as "is equivalent to screen size". I think the first would be better, since a screen size with no width or height could easily be understood as "not defined". Either way, please add a short comment in the documentation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely. Thanks!


required public init(platform: String, humanReadablePlatformName: String? = nil, osVersion: String, screenSize: CGSize, nativeScreenSize: CGSize) {
self.platform = platform
self.humanReadablePlatformName = humanReadablePlatformName
self.osVersion = osVersion
self.screenSize = screenSize
self.nativeScreenSize = nativeScreenSize

super.init()
}
}

extension Device {
Expand Down Expand Up @@ -69,6 +81,12 @@ extension Device {
case "iPhone9,2": return "iPhone 7 Plus (GSM+CDMA)"
case "iPhone9,3": return "iPhone 7 (Global)"
case "iPhone9,4": return "iPhone 7 Plus (Global)"
case "iPhone10,1": return "iPhone 8 (GSM+CDMA)"
case "iPhone10,2": return "iPhone 8 Plus (GSM+CDMA)"
case "iPhone10,3": return "iPhone X (GSM+CDMA)"
case "iPhone10,4": return "iPhone 8 (Global)"
case "iPhone10,5": return "iPhone 8 Plus (Global)"
case "iPhone10,6": return "iPhone X (Global)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thanks!


// iPod
case "iPod1,1": return "iPod Touch 1G"
Expand Down