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-18062] iOS: AttributedString Parity with Android #6463
Conversation
839ec27
to
89042bd
Compare
-Using property accessors in AttributedProxy for Range -Using property accessors in Label Proxy for AttributedString -Using property accessors in AttributedStringProxy for AttributedString
89042bd
to
7354449
Compare
-Added attributed string for TextArea and TextField on Android -Added docs for TextArea and TextField
@@ -98,97 +98,6 @@ -(NSNumber*)CLIP_MODE_DISABLED | |||
return NUMINT(-1); | |||
} | |||
|
|||
#ifdef USE_TI_UIIOSATTRIBUTEDSTRING | |||
MAKE_SYSTEM_PROP(ATTRIBUTE_FONT, AttributeNameFont); |
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.
These properties should not be deleted. Instead they should be marked DEPRECATED REPLACED
TiUIAttributedStringProxy.m is only a member of Target |
Code reviewed. Please address comments. Will do FR only after CR passes. As it stands right now this PR is rejected |
For the question about duplicate properties, I meant: var attr = Titanium.UI.createAttributedString({ Which text (TEXT1 or TEXT2), font (FONT1 or FONT2) and backgroundColor (COLOR1 or COLOR2) will be respected? |
@@ -0,0 +1,58 @@ | |||
/** | |||
* Appcelerator Titanium Mobile | |||
* Copyright (c) 2009-2014 by Appcelerator, Inc. All Rights Reserved. |
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.
Fixed from 2013 to 2014
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 should be "Copyright (c) 2014 by Appcelerator, Inc. All Rights Reserved" because it's added in 2014.
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.
Probably should be 2014-2015, since 2014 is coming to an end.
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.
@hieupham007 , you are farsighted 👍
if (attributedString instanceof AttributedStringProxy) { | ||
Spannable spannableText = AttributedStringProxy.toSpannable(((AttributedStringProxy)attributedString), getProxy()); | ||
if (spannableText != null) { | ||
((TextView)getNativeView()).setText(spannableText); |
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.
Use tv instead of ((TextView)getNativeView()).
In TextField and TextArea, if you set attributedString, font and backgroundColor initially, which properties will be applied for the user input text (the text input by soft keyboard)? |
The input text will use font and backgroundColor that is set if the input text is not in between the attributedString. |
Sample Code. Tab 2:- Second TextField has an AttributedString as input and background set to red, font color to yellow and size to 20. If you erase the AttributedString or type after it, the input will be yellow, font size 20 with red background. Background color affects both AttributedString and the input. Third TextField has AttributedString and AttributedString hint. Erase it to reveal the hint. Tab 3:- TextArea has attributedString and font, color and backgroundColor set. If you erase the Attributed String or type after it, all the input text will be font 20, bold, yellow with red background. Background color affects both AttributedString and the input.
|
48a912c
to
447119d
Compare
…ingProxy was added and other changes
447119d
to
d1cecab
Compare
Code reviewed. Left one comment about parity. |
@ashcoding can you finish the review? |
Will do. |
iOS code needs to change for the color property. Sample code to show issue as follows:
|
After some verification, existing iOS "attributedString"' behavior on UILabel is DIFFERENT from what the existing documents say. |
I'll go change the Android behavior of the attributedString to match iOS when other properties is set. |
7e4b0d3
to
d1cecab
Compare
I looked into this further. In Android, if you set an attributed string, when you use methods such as setFont or setColor, natively it sets it to the Text in the TextView to those properties. If the AttributedString (natively it is SpannableString), those methods do not seem to change the properties of the AttributedString. setColor, setFont and etc would affect the whole Text when used normally while normally when you use AttributedStrings, you would specify at which part does it have a certain color. Thus, I don't think it should be used in that way too. So in this case, I would not recommend for parity in this case when AttributedString, text, color and font is used. Instead I would recommend a document change to specify that you should not use AttributedString with the other properties, eg text, font and color in Android. Either use AttributedString or use the other properties. Shadow properties work with no issue. |
2716f1d
to
124852b
Compare
CR & FR passed for Android. |
I am going to accept this PR as is. There are some problems with the iOS implementation (formatting, un-required checks etc) but that will be fixed in a subsequent PR. APPROVED |
[TIMOB-18062] iOS: AttributedString Parity with Android
Custom fonts don't appear to be working: https://jira.appcelerator.org/browse/AC-70 |
Jira: https://jira.appcelerator.org/browse/TIMOB-18062
This PR is a continuation of #6409 which came from #6358.
It addresses the comments by @pingwang2011 for the Android portion of the code. The doc is still not completed yet as Titanium.UI.iOS.Attribute and Titanium.UI.iOS.AttributedString yml files are missing the "deprecated".
The propertyAccessors is now used in AttributedStringProxy.java, AttributeProxy.java and LabelProxy.java.