[TIMOB-15998] implemented support for attributed string on Android #6358
Conversation
ingo
changed the title
| @@ -58,7 +69,46 @@ public TiUIView createView(Activity activity) | |||
| { | |||
| return new TiUILabel(this); | |||
| } | |||
|
|
|||
| @Kroll.getProperty | |||
There was a problem hiding this comment.
Please put the @Kroll annotations on the same line
|
Please merge with master and add documentation |
| @@ -276,6 +295,10 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP | |||
| } else if (key.equals(TiC.PROPERTY_SHADOW_COLOR)) { | |||
| shadowColor = TiConvert.toColor(TiConvert.toString(newValue)); | |||
| tv.setShadowLayer(shadowRadius, shadowX, shadowY, shadowColor); | |||
| } else if (key.equals(TiC.PROPERTY_ATTRIBUTED_STRING)) { | |||
| if (newValue instanceof AttributedStringProxy) { | |||
There was a problem hiding this comment.
you can merge these ifs
|
Hi. Tried the pull request. Is it missing the class file "AttributedStringProxy" and "AttributeProxy"? I don't seem to have it. |
|
Sorry, I forgot the proxy files to upload, uploaded now. |
|
Took a look at everything and done testing. Code reviewed and function tested to be okay. Things to change for code are the things mentioned by Hieu:- |
|
Addressed comments and merged with master |
|
@salachi @hieupham007 @ashcoding Shouldn't Ashraf be the merger here? He is listed as the reviewer. |
|
@ingo It's actually merging the branch with master so the conflicts are settled and that the pull request can be done easily. |
| type: Titanium.UI.iOS.AttributedString | ||
| platforms: [iphone, ipad] | ||
| since: 3.2.0 | ||
| type: Titanium.UI.AttributedString |
There was a problem hiding this comment.
For iOS, it is still type "Titanium.UI.iOS.AttributedString". Until iOS changes this, perhaps the document should list iOS as Titanium.UI.iOS.AttributedString and Android as Titanium.UI.AttributedString.
There was a problem hiding this comment.
I don't know how to add both types here, when I add both types with coma separated, documentation validation fails.
There was a problem hiding this comment.
@salachi Since Chee Kiat created another ticket to handle the iOS parity, could you just write the documentation for Android separately and leave the iOS as it was?
Chee Kiat will handle the iOS part and fix the documentation to show parity later.
There was a problem hiding this comment.
As I mentioned above, I don't know how to specify both types (generic and IOS) for the same property in the documentation.
There was a problem hiding this comment.
For the doc in UI Label, you could write:-
description: |
If set, the label ignores the `text`, `color`, and `shadow` properties.
For iOS, use <Titanium.UI.iOS.AttributedString>. For Android, use <Titanium.UI.AttributedString>
type: [Titanium.UI.iOS.AttributedString, Titanium.UI.AttributedString]
Once this pull request is done with this fix, Chee Kiat can fix the docs related to this with the iOS Jira Ticket he is going to do to address the parity issue.
|
I think the documents for this should have one for Android and one for iOS since for iOS, you still need to use the "Titanium.UI.iOS" prefix. This probably needs another ticket for the fix and not within scope of this ticket. Code reviewed and tested to work for android. Docs need to change. |
|
I would recommend copying Titanium.UI.iOS.AttributedString implementation to Titanium.UI.AttributedString so that it is now a cross platform property. We can then deprecate Titanium.UI.iOS.AttributedString. |
|
JIRI ticket created to modify iOS AttributedString such that this becomes cross platform. |
|
Will update this pull request with iOS changes, including documentation and submit a new pull request under https://jira.appcelerator.org/browse/TIMOB-18062. No further actions needed on this pull request. |
|
Sunil, please address @ashcoding comments regarding docs. |
|
@hieupham007 The docs don't need changes done in this PR. Chee Kiat's PR has taken over this PR which includes the changes done here, iOS changes and docs. His PR #6409 |
|
Code reviewed, looks good. Waiting on doc review to merge |
| since: 3.2.0 | ||
| type: Titanium.UI.AttributedString | ||
| platforms: [iphone, ipad, android] | ||
| since: {iphone: "3.2.0", ipad: "3.2.0", android: "3.5.0"} |
|
Reviewed and ran validate. Docs are missing Titanium.UI.ATTRIBUTE_* constants. |
|
Closing this PR in favor of #6409 |
|
Attention: The contributor has signed the CLA |
https://jira.appcelerator.org/browse/TIMOB-15998
Implemented support for attributedstring