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
added getPhysicalSizeCategory() Ti.Platform.displayCaps for Android #2610
Conversation
YML docs should be updated to show new method |
return "xlarge"; | ||
case Configuration.SCREENLAYOUT_SIZE_UNDEFINED: | ||
default : | ||
return "medium"; |
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.
Why return "medium" here rather than "unknown" or some value that is representative of the value not being available? If the value is unavailable for some reason on a large device then returning "medium" (as in this case) would be a bug.
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.
shoot, meant to return 'normal' there.
I can return the 'unknown' constant. I was following the example set forth in getDensity() in DisplayCapsProxy.java where a medium density is returned if the density cannot be determined. Let me know how you'd like me to proceed and I'll modify the PR.
Per the discussion in the architecture council meeting, this change is Android only. Before inclusion though we should update the docs and replace the category values with exposed constants. |
OK, before I dive in and adjust the PR and add docs and constants, let me get a couple things clear:
|
@@ -14,3 +14,56 @@ properties: | |||
[API levels](http://developer.android.com/guide/appendix/api-levels.html). | |||
type: Number | |||
permission: read-only | |||
|
|||
- name: PHYSICAL_SIZE_CATEGORY_LARGE |
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.
It may be overly picky, but I feel like these constants should be listed in a more natural way. IE: small, normal, large, xlarge, undefined
It can be easy for a user to look at values until they hit an error or undefined value and then stop since they assume they have hit the end of the list of "good" values.
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.
I think alphabetical listings are the standard as I that's how it is throughout the rest of the docs
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.
Fair enough. I think the presentation would be better served by a usage type organization rather alphabetical but this is consistent so I will give on this.
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.
If it makes you feel any better, I originally typed it the way you are suggesting then changed it when I looked at the other constants in the docs. :)
Functional test passed and validate / docgen passed. Awaiting update to docs then will revalidate docs and accept. |
Accepted |
added getPhysicalSizeCategory() Ti.Platform.displayCaps for Android
This will allow us to get the physical size of an Android device. This will provide a reliable way to determine if Android devices are small, normal, large, or xlarge form factor devices.
Details are in the corresponding Jira ticket: https://jira.appcelerator.org/browse/TIMOB-10043