[TIMOB-19802] Android: CardView should use standard property names
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Low |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2015-12-02T22:58:24.000+0000 |
Affected Version/s | Release 5.1.0 |
Fix Version/s | Release 5.1.2 |
Components | Android |
Labels | n/a |
Reporter | Fokke Zandbergen |
Assignee | Ashraf Abu |
Created | 2015-10-27T12:05:36.000+0000 |
Updated | 2015-12-03T02:15:05.000+0000 |
Description
The following sample shows that both
backgroundColor
and cardBackgroundColor
work, but the one with backgroundColor
doesn't show a border/corner radius. Because of TIMOB-19801 I cannot verify, but I guess when I would set both I would see a rectangle without radius behind one that has?
Is there an actual use case to have both (working, if we resolve TIMOB-19801)? If so, then we need to document the difference. If not, then I'd say we rename cardBackgroundColor
to backgroundColor
.
var win = Ti.UI.createWindow({
layout: 'vertical'
});
var card1 = Ti.UI.Android.createCardView({
top: 20,
left: 20,
right: 20,
contentPadding: 20,
cardBackgroundColor: '#F00'
});
card1.add(Ti.UI.createLabel({
text: 'cardBackgroundColor',
color: '#000'
}));
win.add(card1);
var card2 = Ti.UI.Android.createCardView({
top: 20,
left: 20,
right: 20,
contentPadding: 20,
backgroundColor: '#F00'
});
card2.add(Ti.UI.createLabel({
text: 'backgroundColor',
color: '#000'
}));
win.add(card2);
win.open();
*UPDATE:* I'm not convinced deviating from standard properties like [backgroundColor](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-backgroundColor), [borderRadius](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-borderRadius), [elevation](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-elevation) and [*Padding](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.TextField-property-paddingLeft).
* *Pro:* Easier to translate what you read in the [Android reference](https://developer.android.com/reference/android/support/v7/widget/CardView.html) to your Titanium app.
* *Con:* Titanium developers are used to our naming conventions throughout what is after all a cross-platform SDK, even for platform-specific components.
Given my experience that most Titanium developers do not read the target platform references but rely on our docs and - lazy as developers are - the predictability of our API I think we should:
* Remove the card
prefix from the CardView specific properties.
* Rename (card)CornerRadius
to borderRadius
.
* Remove the content
prefix from the contentPadding*
properties.
Attachments
File | Date | Size |
---|---|---|
android_20151027-125702.png | 2015-10-27T12:05:27.000+0000 | 44848 |
I see that right now we follow the [original API](https://developer.android.com/reference/android/support/v7/widget/CardView.html) exactly. I think that might be better indeed, but then we should make sure that
backgroundColor
is not there in the API reference or document the difference (use cases).Confirmed the PR fixes it. However, I'm not convinced deviating from standard properties like [backgroundColor](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-backgroundColor), [borderRadius](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-borderRadius), [elevation](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.View-property-elevation) and [*Padding](https://appcelerator.github.io/appc-docs/latest/#!/api/Titanium.UI.TextField-property-paddingLeft). * *Pro:* Easier to translate what you read in the [Android reference](https://developer.android.com/reference/android/support/v7/widget/CardView.html) to your Titanium app. * *Con:* Titanium developers are used to our naming conventions throughout what is after all a cross-platform SDK, even for platform-specific components. Given my experience that most Titanium developers do not read the target platform references but rely on our docs and - lazy as developers are - the predictability of our API I think we should: * Remove the
card
prefix from the CardView specific properties. * Rename(card)CornerRadius
toborderRadius
. * Remove thecontent
prefix from thecontentPadding*
properties.I agree with your recommendations.
Master reviewed and merged. Backport 5_1_X https://github.com/appcelerator/titanium_mobile/pull/7518
Backport merged.
Verified the fix. If the properties with the prefix 'card' & 'content' are used we see depreciation warnings in the console along with suggestions for the new property names. Also, the new property names works as expected. e.g:
Closing. Environment: Appc Studio : 4.4.0.201511241829 Ti SDK : 5.1.2.v20151202061227 Ti CLI : 5.0.5 Alloy : 1.7.26 MAC Yosemite : 10.10.5 Appc NPM : 4.2.2 Appc CLI : 5.1.0 Node: v0.12.27 Nexus 6P - Android 6.0
Reopening to edit comment