Skip to content
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

Merged
merged 13 commits into from
Jan 15, 2015
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Appcelerator Titanium Mobile
* Copyright (c) 2014-2015 by Appcelerator, Inc. All Rights Reserved.
* Licensed under the terms of the Apache Public License
* Please see the LICENSE included with this distribution for details.
*/
package ti.modules.titanium.ui;

import org.appcelerator.kroll.KrollProxy;
import org.appcelerator.kroll.annotations.Kroll;
import org.appcelerator.titanium.TiC;

@Kroll.proxy(propertyAccessors = {
TiC.PROPERTY_TYPE,
TiC.PROPERTY_VALUE,
TiC.PROPERTY_ATTRIBUTE_RANGE,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)

From pingwang2011:
Add TiC.PROPERTY_ATTRIBUTE_RANGE to propertyAccessors. Those properties will be processed/changed in processProperties(KrollDict d) / handleCreationDict(KrollDict dict) and propertyChanged(String key, Object oldValue, Object newValue, KrollProxy proxy) / public void onPropertyChanged(String name, Object value). And you can get access to them using proxy.getProperty(String property) at any time.

TiC.PROPERTY_ATTRIBUTE_RANGE was added to propertyAccessors.

})
public class AttributeProxy extends KrollProxy
{
protected AttributeProxy()
{
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)
Removed this method:
"@Kroll.method @Kroll.setProperty
public void setRange (int[] range)"

From pingwang2011:
Don't need this method if TiC.PROPERTY_ATTRIBUTE_RANGE is added to propertyAccessors.

#6409 (diff)
Removed this method:
"@Kroll.method @Kroll.getProperty
public int[] getRange()"

From pingwang2011:
Don't need this method if TiC.PROPERTY_ATTRIBUTE_RANGE is added to propertyAccessors.

#6409 (diff)
Removed this method:
"@OverRide
public void handleCreationDict(KrollDict options)"

From pingwang2011:
Don't need this method if TiC.PROPERTY_ATTRIBUTE_RANGE is added to propertyAccessors.

@Override
public String getApiName()
{
return "Ti.UI.Attribute";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
/**
* Appcelerator Titanium Mobile
* Copyright (c) 2014-2015 by Appcelerator, Inc. All Rights Reserved.
* Licensed under the terms of the Apache Public License
* Please see the LICENSE included with this distribution for details.
*/
package ti.modules.titanium.ui;

import java.util.HashMap;

import org.appcelerator.kroll.KrollDict;
import org.appcelerator.kroll.KrollProxy;
import org.appcelerator.kroll.annotations.Kroll;
import org.appcelerator.kroll.common.Log;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.proxy.TiViewProxy;
import org.appcelerator.titanium.util.TiConvert;
import org.appcelerator.titanium.util.TiUIHelper;

import android.app.Activity;
import android.text.Spannable;
import android.text.SpannableString;
import android.text.TextUtils;
import android.text.style.AbsoluteSizeSpan;
import android.text.style.BackgroundColorSpan;
import android.text.style.ForegroundColorSpan;
import android.text.style.StrikethroughSpan;
import android.text.style.StyleSpan;
import android.text.style.TypefaceSpan;
import android.text.style.URLSpan;
import android.text.style.UnderlineSpan;

@Kroll.proxy(creatableInModule=UIModule.class, propertyAccessors = {
TiC.PROPERTY_ATTRIBUTES,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)

TiC.PROPERTY_ATTRIBUTES was added to propertyAccessors.

TiC.PROPERTY_TEXT
})
public class AttributedStringProxy extends KrollProxy
{
private static final String TAG = "AttributedString";

public AttributedStringProxy()
{
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)

Removed method:
"@Kroll.getProperty @Kroll.method
public AttributeProxy[] getAttributes()"

From pingwang2011:
Don't need this method if TiC.PROPERTY_ATTRIBUTES is added to propertyAccessors.

#6409 (diff)
Removed method:
"@Kroll.setProperty @Kroll.method
public void setAttributes(AttributeProxy[] values)"

From pingwang2011
Don't need this method if TiC.PROPERTY_ATTRIBUTES is added to propertyAccessors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)

Removed method:
"public void handleCreationDict(KrollDict options)"

From pingwang2011
values!=null is unnecessary since you have checked instanceof. Here is a little strange, if you already casted it to Object[], it cannot be null. Anyway, please put the instanceof check at the very beginning.

@Kroll.method
public void addAttribute(Object attr)
{
AttributeProxy attribute;
KrollDict attributeDict = null;
if (attr instanceof HashMap) {
attributeDict = new KrollDict((HashMap)attr);
}
if (attributeDict != null) {
attribute = new AttributeProxy();
attribute.setCreationUrl(getCreationUrl().getNormalizedUrl());
attribute.handleCreationDict(attributeDict);
Object obj = getProperty(TiC.PROPERTY_ATTRIBUTES);
AttributeProxy[] attributes = null;
if (obj instanceof Object[]) {
Object[] objArray = (Object[])obj;
attributes = new AttributeProxy[objArray.length + 1];
for (int i = 0; i < objArray.length; i++) {
attributes[i] = AttributedStringProxy.attributeProxyFor(objArray[i], this);
}
attributes[objArray.length] = attribute;
} else {
attributes = new AttributeProxy[1];
attributes[0] = attribute;
}
setProperty(TiC.PROPERTY_ATTRIBUTES, attributes);
}
}

public static AttributeProxy attributeProxyFor(Object obj, KrollProxy proxy)
{
AttributeProxy attributeProxy = null;
if (obj instanceof AttributeProxy) {
return (AttributeProxy)obj;
} else {
KrollDict attributeDict = null;
if (obj instanceof KrollDict) {
attributeDict = (KrollDict)obj;
} else if (obj instanceof HashMap) {
attributeDict = new KrollDict((HashMap)obj);
}
if (attributeDict != null) {
attributeProxy = new AttributeProxy();
attributeProxy.setCreationUrl(proxy.getCreationUrl().getNormalizedUrl());
attributeProxy.handleCreationDict(attributeDict);
}
if(attributeProxy == null) {
Log.e(TAG, "Unable to create attribute proxy for object, likely an error in the type of the object passed in.");
}

return attributeProxy;
}
}

public static Spannable toSpannable(AttributedStringProxy attrString, Activity activity)
{
if (attrString != null && attrString.hasProperty(TiC.PROPERTY_TEXT)) {
String textString = TiConvert.toString(attrString.getProperty(TiC.PROPERTY_TEXT));
if (!TextUtils.isEmpty(textString)) {
Spannable spannableText = new SpannableString(textString);
AttributeProxy[] attributes = null;
Object obj = attrString.getProperty(TiC.PROPERTY_ATTRIBUTES);
if (obj instanceof Object[]) {
Object[] objArray = (Object[])obj;
attributes = new AttributeProxy[objArray.length];
for (int i = 0; i < objArray.length; i++) {
attributes[i] = AttributedStringProxy.attributeProxyFor(objArray[i], attrString);
}
}
if(attributes != null) {
for (AttributeProxy attr : attributes) {
if (attr.hasProperty(TiC.PROPERTY_TYPE)) {
Object type = attr.getProperty(TiC.PROPERTY_TYPE);
int[] range = null;
Object inRange = attr.getProperty(TiC.PROPERTY_ATTRIBUTE_RANGE);
if (inRange instanceof Object[]) {
range = TiConvert.toIntArray((Object[])inRange);
}
Object attrValue = attr.getProperty(TiC.PROPERTY_VALUE);
if(range != null && (range[0] < (range[0]+range[1]))) {
switch (TiConvert.toInt(type)) {
case UIModule.ATTRIBUTE_FONT:
KrollDict fontProp = null;
if (attrValue instanceof KrollDict) {
fontProp = (KrollDict) attrValue;
} else if (attrValue instanceof HashMap) {
fontProp = new KrollDict((HashMap<String, Object>) attrValue);
}
if(fontProp != null) {
String[] fontProperties = TiUIHelper.getFontProperties((KrollDict) fontProp);
if(fontProperties != null) {
if (fontProperties[TiUIHelper.FONT_SIZE_POSITION] != null) {
spannableText.setSpan(
new AbsoluteSizeSpan((int) TiUIHelper.getRawSize(
fontProperties[TiUIHelper.FONT_SIZE_POSITION], activity)),
range[0], range[0] + range[1], Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
}
if (fontProperties[TiUIHelper.FONT_WEIGHT_POSITION] != null) {
int typefaceStyle = TiUIHelper.toTypefaceStyle(
fontProperties[TiUIHelper.FONT_WEIGHT_POSITION],
fontProperties[TiUIHelper.FONT_STYLE_POSITION]);
spannableText.setSpan(new StyleSpan(typefaceStyle), range[0], range[0] + range[1],
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
}
if (fontProperties[TiUIHelper.FONT_FAMILY_POSITION] != null) {
spannableText.setSpan(new TypefaceSpan(fontProperties[TiUIHelper.FONT_FAMILY_POSITION]),
range[0], range[0] + range[1], Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
}
}
}
break;
case UIModule.ATTRIBUTE_BACKGROUND_COLOR:
spannableText.setSpan(
new BackgroundColorSpan(TiConvert.toColor(TiConvert.toString(attrValue))), range[0],
range[0] + range[1], Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
break;
case UIModule.ATTRIBUTE_FOREGROUND_COLOR:
spannableText.setSpan(
new ForegroundColorSpan(TiConvert.toColor(TiConvert.toString(attrValue))), range[0],
range[0] + range[1], Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
break;
case UIModule.ATTRIBUTE_STRIKETHROUGH_STYLE:
spannableText.setSpan(new StrikethroughSpan(), range[0], range[0] + range[1],
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
break;
case UIModule.ATTRIBUTE_UNDERLINES_STYLE:
spannableText.setSpan(new UnderlineSpan(), range[0], range[0] + range[1],
Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
break;
case UIModule.ATTRIBUTE_LINK:
if(attrValue != null) {
spannableText.setSpan(new URLSpan(TiConvert.toString(attrValue)), range[0], range[0]
+ range[1], Spannable.SPAN_EXCLUSIVE_EXCLUSIVE);
}
break;
}
}
}
}
}
return spannableText;
}
}
return null;
}

@Override
public String getApiName()
{
return "Ti.UI.AttributedString";
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the method "addAttribute"?

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package ti.modules.titanium.ui;

import org.appcelerator.kroll.KrollDict;
import org.appcelerator.kroll.KrollProxy;
import org.appcelerator.kroll.annotations.Kroll;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.TiContext;
Expand All @@ -18,6 +19,7 @@

@Kroll.proxy(creatableInModule=UIModule.class, propertyAccessors = {
TiC.PROPERTY_AUTO_LINK,
TiC.PROPERTY_ATTRIBUTED_STRING,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6409 (diff)

From pingwang2011:
Add TiC.PROPERTY_ATTRIBUTED_STRING to propertyAccessors.

TiC.PROPERTY_COLOR,
TiC.PROPERTY_ELLIPSIZE,
TiC.PROPERTY_FONT,
Expand All @@ -35,6 +37,9 @@
})
public class LabelProxy extends TiViewProxy
{
private static final int MSG_FIRST_ID = KrollProxy.MSG_LAST_ID + 1;
protected static final int MSG_LAST_ID = MSG_FIRST_ID + 999;

public LabelProxy()
{
defaultValues.put(TiC.PROPERTY_TEXT, "");
Expand All @@ -58,7 +63,7 @@ public TiUIView createView(Activity activity)
{
return new TiUILabel(this);
}

@Override
public String getApiName()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import android.os.Message;

@Kroll.proxy(creatableInModule=UIModule.class, propertyAccessors = {
TiC.PROPERTY_ATTRIBUTED_STRING,
TiC.PROPERTY_AUTOCAPITALIZATION,
TiC.PROPERTY_AUTOCORRECT,
TiC.PROPERTY_AUTO_LINK,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import android.os.Message;

@Kroll.proxy(creatableInModule=UIModule.class, propertyAccessors = {
TiC.PROPERTY_ATTRIBUTED_STRING,
TiC.PROPERTY_ATTRIBUTED_HINT_TEXT,
TiC.PROPERTY_AUTOCAPITALIZATION,
TiC.PROPERTY_AUTOCORRECT,
TiC.PROPERTY_AUTO_LINK,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ public class UIModule extends KrollModule implements Handler.Callback
@Kroll.constant public static final int URL_ERROR_TIMEOUT = WebViewClient.ERROR_TIMEOUT;
@Kroll.constant public static final int URL_ERROR_UNKNOWN = WebViewClient.ERROR_UNKNOWN;
@Kroll.constant public static final int URL_ERROR_UNSUPPORTED_SCHEME = WebViewClient.ERROR_UNSUPPORTED_SCHEME;

@Kroll.constant public static final int ATTRIBUTE_FONT = 0;
@Kroll.constant public static final int ATTRIBUTE_FOREGROUND_COLOR = 1;
@Kroll.constant public static final int ATTRIBUTE_BACKGROUND_COLOR = 2;
@Kroll.constant public static final int ATTRIBUTE_STRIKETHROUGH_STYLE = 3;
@Kroll.constant public static final int ATTRIBUTE_UNDERLINES_STYLE = 4;
@Kroll.constant public static final int ATTRIBUTE_LINK = 5;
@Kroll.constant public static final int ATTRIBUTE_UNDERLINE_COLOR = 6;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why attributedString does not have an attribute to specify a text color?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foreground color is text color.


protected static final int MSG_SET_BACKGROUND_COLOR = KrollProxy.MSG_LAST_ID + 100;
protected static final int MSG_SET_BACKGROUND_IMAGE = KrollProxy.MSG_LAST_ID + 101;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
import org.appcelerator.kroll.KrollDict;
import org.appcelerator.kroll.KrollProxy;
import org.appcelerator.kroll.common.Log;
import org.appcelerator.titanium.TiApplication;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.proxy.TiViewProxy;
import org.appcelerator.titanium.util.TiConvert;
import org.appcelerator.titanium.util.TiUIHelper;
import org.appcelerator.titanium.view.TiUIView;

import ti.modules.titanium.ui.AttributedStringProxy;
import android.graphics.Color;
import android.text.Html;
import android.text.InputType;
Expand Down Expand Up @@ -215,6 +217,15 @@ public void processProperties(KrollDict d)
if (needShadow) {
tv.setShadowLayer(shadowRadius, shadowX, shadowY, shadowColor);
}
if (d.containsKey(TiC.PROPERTY_ATTRIBUTED_STRING)) {
Object attributedString = d.get(TiC.PROPERTY_ATTRIBUTED_STRING);
if (attributedString instanceof AttributedStringProxy) {
Spannable spannableText = AttributedStringProxy.toSpannable(((AttributedStringProxy)attributedString), TiApplication.getAppCurrentActivity());
if (spannableText != null) {
tv.setText(spannableText);
}
}
}
// This needs to be the last operation.
TiUIHelper.linkifyIfEnabled(tv, d.get(TiC.PROPERTY_AUTO_LINK));
tv.invalidate();
Expand Down Expand Up @@ -276,6 +287,11 @@ 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) && newValue instanceof AttributedStringProxy) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user set "attributedString" and "text" initially, then "attributedString" is respected while "text" is ignored. But if the user later change the property "text", "text" will be respected and "attributedString" will be ignored based on the current implementation of propertyChanged. Is that expected? How about the "font" and "backgroundColor"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be the behavior. If a user wants to change the "text" while an "attributeString" is currently being used, the user should create a new "attributeString" with "text" as on of it's value and replace the old attributeString with the new attributeString.

Especially in a case where, for example, where an attributeString is already set with font and backgroundColor in a Label. If a text is set, it the set font and backgroundColor in attributeString will not be set and used in the Label. This will result in a plain text. If the user wants to change the text and maintain the same font and color, user should replace attributedString and not text.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the iOS behavior. If attributedString is set and a change is font is done, it does affect the attributedString and change it's font size or color.

Android's behavior matches this.

The only thing that is different right now is in iOS, if "attributedString" and "text" is set initially. iOS respects "text". Android respects "attributedString".

I think I'll change this so that Android respects "text" if both are set initially to match iOS parity.

From the docs http://docs.appcelerator.com/titanium/3.0/#!/api/Titanium.UI.Label, it says "Specify an attributed string for the label. If set, the label ignores the text, color, and shadow properties". So I'll leave it as attributedString first for android.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhatfield, @vishalduggal, should we resolve this parity issue or just document it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to resolve the parity issue if we can.

| Neeraj Gupta | SVP, Product and Engineering
| Appcelerator, Inc | e: ngupta@appcelerator.commailto:%20ngupta@appcelerator.com | o:+1 650-528-2931 |
| 440 N. Bernardo Ave, Mtn View, CA 94043

From: pingwang2011 <notifications@github.commailto:notifications@github.com>
Reply-To: appcelerator/titanium_mobile <reply@reply.github.commailto:reply@reply.github.com>
Date: Monday, January 5, 2015 at 2:57 PM
To: appcelerator/titanium_mobile <titanium_mobile@noreply.github.commailto:titanium_mobile@noreply.github.com>
Subject: Re: [titanium_mobile] [TIMOB-18062] iOS: AttributedString Parity with Android (#6463)

In android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.javahttps://github.com//pull/6463#discussion-diff-22496623:

@@ -276,6 +286,11 @@ 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) && newValue instanceof AttributedStringProxy) {
    

@bhatfieldhttps://github.com/bhatfield, @vishalduggalhttps://github.com/vishalduggal, should we resolve this parity issue or just document it?

Reply to this email directly or view it on GitHubhttps://github.com//pull/6463/files#r22496623.

Spannable spannableText = AttributedStringProxy.toSpannable(((AttributedStringProxy)newValue), TiApplication.getAppCurrentActivity());
if (spannableText != null) {
tv.setText(spannableText);
}
} else {
super.propertyChanged(key, oldValue, newValue, proxy);
}
Expand All @@ -284,5 +300,4 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP
public void setClickable(boolean clickable) {
((TextView)getNativeView()).setClickable(clickable);
}

}