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
Timob 12176: Implementation of ListView #3935
Conversation
…e into timob-12176
…e into timob-12176
…e into timob-12176
…e into timob-12176
…e into timob-12176 Conflicts: android/titanium/src/java/org/appcelerator/titanium/view/TiUIView.java
…e into timob-12176
…e into timob-12176 Conflicts: android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUIImageView.java
} else if (additionalEventData != null) { | ||
data.putAll(additionalEventData); | ||
} | ||
return proxy.fireEvent(TiC.EVENT_CLICK, data); |
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.
Shouldn't this be eventName?
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.
haha nice catch :)
@@ -1,4 +1,5 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<resources> | |||
<string name="app_name">Titanium UI Module</string> | |||
<string name="accessoryType">One of the following images: isCheck, hasChild, or custom image.</string> |
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.
Don't use camel case here because some OS are case insensitive and some are case sensitive and this may introduce name conflict. BTW, in order to avoid the name conflict, it is better to add the package name to the string name, eg. titanium_ui_widget_listview_accessory_type.
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.
Actually, for all the resources used by titanium, like layout and drawable, we should add the package name to the res name/id to avoid name conflict.
//Recursively calls for all childTemplates | ||
if (properties.containsKey(TiC.PROPERTY_CHILD_TEMPLATES)) { | ||
if(item == null) { | ||
Log.e(TAG, "ITEM SHOULDN'NT BE NULL"); |
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.
Should be debug only, and message needs to be reworded
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.
done
…e into timob-12176
…e into timob-12176
Code reviewed and functionally tested. Ran through the test cases attached to the ticket, along with the bubbling events test case, and a KS pass. Also verified that this matches the yaml spec for the most part. Noted my concerns at https://gist.github.com/bryan-m-hughes/4677555. Going to merge this now, and other list view issues/comments will be addressed in https://jira.appcelerator.org/browse/TIMOB-13035. Request Accepted |
Timob 12176: Implementation of ListView
Amazing job on that PR |
Testing procedure pending.
https://jira.appcelerator.org/browse/TIMOB-12176