-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add device class field for iOS #945
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change looks good, but would recommend getting a sign-off from @eduardo-camacho as well as he's the resident iOS/Obj-C expert.
@@ -68,3 +68,19 @@ | |||
return std::string { [[[UIDevice currentDevice] systemVersion] UTF8String] }; | |||
} | |||
|
|||
std::string GetDeviceClass() { | |||
#if TARGET_IPHONE_SIMULATOR | |||
return "iOS.Emulator"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually Apple refers to this as "Simulator", not Emulator. Please change
#else | ||
switch (UIDevice.currentDevice.userInterfaceIdiom) { | ||
case UIUserInterfaceIdiomPhone: | ||
return "iOS.Phone"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove "iOS." prefix, it's not actually needed since tablets, phones, TV and car are iOS only.
@@ -83,3 +83,6 @@ | |||
return GetDeviceOsVersion(); | |||
} | |||
|
|||
std::string GetDeviceClass() { | |||
return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set this as "Mac"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments
Hi @eduardo-camacho |
I don't agree with documented naming, but not much to do now if it was already defined that way before, let's not break backward compatibility. Approving. |
Co-authored-by: Bang Lee <[email protected]>
https://1dsdocs.azurewebsites.net/schema/PartA/device.html#deviceclass