[TIMOB-1618] Android: Expose "minimumFontSize" on Android (Parity) #8724

hansemannn · 2016-12-30T11:38:19Z

JIRA: https://jira.appcelerator.org/browse/TIMOB-1618

Continuing from #5909, thank you @lgolu!

# Conflicts: # android/modules/ui/src/java/ti/modules/titanium/ui/LabelProxy.java

Added minimumFontSize property with the same behaviour as on iPhone

Added property minimumFontSize

Implemented parity behaviour for the property minimumFontSize on Android.

Correction: replaced missing font padding implementation

Added import declarations.

Renamed PROPERTY_MINIMUM_FONT_SIZE

garymathews · 2016-12-31T02:22:14Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+			if (tv != null) {
+				if (autoshrinkSetFontSize != null) {
+					if (tv.getTextSize() == TiConvert.toFloat(autoshrinkSetFontSize)) {
+						if (propertySetFontSize != null ) {


Could use getFontProperties() instead? Making propertySetFontSize redundant.

String[] fontProperties = TiUIHelper.getFontProperties(proxy.getProperties()); if (fontProperties.length > TiUIHelper.FONT_SIZE_POSITION && fontProperties[TiUIHelper.FONT_SIZE_POSITION] != null) { tv.setTextSize(TiUIHelper.getSizeUnits(fontProperties[TiUIHelper.FONT_SIZE_POSITION]), TiUIHelper.getSize(fontProperties[TiUIHelper.FONT_SIZE_POSITION])); } else { tv.setTextSize(TiUIHelper.getSizeUnits(null), TiUIHelper.getSize(null)); }

garymathews · 2016-12-31T02:29:04Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+
+			    TextPaint textPaint = tv.getPaint();
+				if (textPaint != null) {
+					float stringWidth = textPaint.measureText((tv.getText()).toString());


Unnecessary brackets around (tv.getText())

garymathews · 2016-12-31T02:32:45Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+				if (textPaint != null) {
+					float stringWidth = textPaint.measureText((tv.getText()).toString());
+					int textViewWidth = tv.getWidth();
+					if (textViewWidth < stringWidth && stringWidth != 0) {


... && stringWidth > 0

garymathews · 2016-12-31T02:35:04Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+					int textViewWidth = tv.getWidth();
+					if (textViewWidth < stringWidth && stringWidth != 0) {
+						float fontSize = (textViewWidth / stringWidth) * tv.getTextSize();
+						autoshrinkSetFontSize = fontSize > TiConvert.toFloat(minimumFontSize) ? String.valueOf(fontSize) : minimumFontSize;


TiConvert.toFloat(minimumFontSize, 0)

hansemannn · 2016-12-31T08:02:17Z

Comments addressed! Not sure about the fontProperties change, but the reason makes sense.

garymathews · 2016-12-31T12:43:25Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

@@ -49,6 +51,9 @@
 	private float shadowX = 0f;
 	private float shadowY = 0f;
 	private int shadowColor = Color.TRANSPARENT;
+	private String minimumFontSize = null;
+	private String propertySetFontSize = null;


This can be removed

garymathews · 2016-12-31T12:43:42Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

@@ -207,6 +257,9 @@ public void processProperties(KrollDict d)
 			tv.setHighlightColor(TiConvert.toColor(d, TiC.PROPERTY_HIGHLIGHTED_COLOR));
 		}
 		if (d.containsKey(TiC.PROPERTY_FONT)) {
+			if ((d.getKrollDict(TiC.PROPERTY_FONT)).containsKey("fontSize")) {


This can be removed

garymathews · 2016-12-31T12:44:00Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

 		} else if (key.equals(TiC.PROPERTY_FONT)) {
+			if (((HashMap) newValue).containsKey("fontSize")) {


This can be removed

hansemannn · 2017-01-01T12:12:45Z

@garymathews PR updated. Happy new year!

frankieandone

Important note: I suggest we add the same fix to TextField

frankieandone · 2017-01-05T18:58:49Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+
+	/**
+	 * Method used to decrease the fontsize of the text to fit the width
+	 * fontsize should be >= than the property minimumFontSize


keeping in format with other descriptions and docs:

change >= into greater than or equal to like for maxAge and other examples. This is the description so it's supposed to be wordy?

frankieandone · 2017-01-05T19:02:25Z

apidoc/Titanium/UI/Label.yml

    type: Number
-    platforms: [iphone, ipad]
+    since: {iphone: "0.8.0", ipad: "0.8.0", android: "6.1.0"}


place android part first in since: {...}.

other examples always have android first. I think it's in alphabetical order

Nope, it's random. Usually the API added first comes first and the new one is added to it. But there're really no guidelines for that one.

frankieandone · 2017-01-05T19:08:18Z

apidoc/Titanium/UI/Label.yml

@@ -166,8 +166,13 @@ properties:
    summary: Minimum font size when the font is sized based on the contents. 
    description: |
        Enables font scaling to fit and forces the label content to be limited to a single line.
+
+        On Android, setting this property will force the `ellipsize` property to be


keeping in format with existing docs:

make ellipsize into link to property link?

either edit into or append default: <Titanium.UI.TEXT_ELLIPSIZE_TRUNCATE_END> on Android...and so and so behavior on iOS, etc...

Yes, we can link it. But the default value suggestion should not be done, since the default is undefined, so behaving as not set. Otherwise it confuse the developers if they should set TEXT_ELLIPSIZE_* constants instead of font-sizes.

sorry my bad, did review twice, first time added this comment. "lost" first review so inputted comments again then it looked like I had two reviews. Removed new and updated second review and this was left I guess from first time... 👍

frankieandone · 2017-01-05T19:33:07Z

android/titanium/src/java/org/appcelerator/titanium/TiC.java

+	/**
+	 * @module.api
+	 */
+	public static final String PROPERTY_MINIMUM_FONT_SIZE = "minimumFontSize";


keeping in format with TiC:

properties are in alphabetical order so place after PROPERTY_MIN_AGE.

refactor to PROPERTY_MIN_FONT_SIZE like PROPERTY_MIN_AGE. not sure why PROPERTY_MIN_AGE, PROPERTY_MIN_UPDATE_DISTANCE, PROPERTY_MIN_UPDATE_TIME do not have @module.api tag.

It has to be PROPERTY_MINIMUM_FONT_SIZE. The constants are based on the camel case name that injects _ chars instead of the camel-case style.

frankieandone · 2017-01-05T19:33:43Z

apidoc/Titanium/UI/Label.yml

    type: Number
-    platforms: [iphone, ipad]
+    since: {iphone: "0.8.0", ipad: "0.8.0", android: "6.1.0"}
+


remove new line.

frankieandone · 2017-01-05T19:40:58Z

android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java

+	 * fontsize should be >= than the property minimumFontSize
+	 * @param view
+	 */
+	private void adjustTextFontSize(View v){	


refactor variable to view or change @param view to @param v

hansemannn · 2017-01-05T21:10:22Z

Good idea to also implement it on the text field. @fmerzadyan do you want to take-over the PR? I just did some cleanup, the deep-dive should be done by an Android Pro :-)

frankieandone · 2017-01-05T21:17:36Z

@hansemannn haha ~ not that good but sure 👍

frankieandone · 2017-01-11T20:49:36Z

CR and FT passed

lokeshchdhry · 2017-01-27T23:30:23Z

FR Passed.
The minimumFontSize property works fine.
Checked it by code below:

var win = Ti.UI.createWindow({});

var label = Ti.UI.createLabel({
	text:"Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.",
	textColor:'white',
	minimumFontSize:8
});

win.add(label);
win.open();

Checked with different values for minimumFontSize.
Checked it in portrait & landscape mode.
Checked it for alloy app.
The text scales automatically to the minimum font size specified for the minimumFontSize & works as expected.

Appc Studio : 4.8.1.201612050850
SDK Version : local 6.1.0
Mac OS Version : 10.12.2
Xcode Version : Xcode 8.2.1 Build version 8C1002
Appc CLI AND Appc NPM : {"NPM":"4.2.9-1","CLI":"6.1.0"}
Ti CLI : 5.0.11
Alloy : 1.9.5
Node : v4.6.0
Nexus 6 running 6.0.1, Nexus 6P running 7.1.1 & android emulator 5.1.1

lokeshchdhry · 2017-01-27T23:31:46Z

@hansemannn , What about the minimumFontSize property for textField for android. Are we gonna expose it as well ?

Will merge the PR once this is confirmed.

EDIT: Talked this with frakie & he pointed me to a ticket TIMOB-24298 already created for this.

hansemannn · 2017-01-27T23:43:42Z

I think @m1ga talked about that a while ago, but for this specific 6.1.0 ticket, we will go with the labels first.

lgolu and others added 9 commits December 30, 2016 12:27

Update LabelProxy.java

7595163

# Conflicts: # android/modules/ui/src/java/ti/modules/titanium/ui/LabelProxy.java

Update TiUILabel.java

57fd4f8

Added minimumFontSize property with the same behaviour as on iPhone

Update TiC.java

6e89521

Added property minimumFontSize

Update Label.yml

28cf523

Implemented parity behaviour for the property minimumFontSize on Android.

Update TiUILabel.java

f78afd6

Correction: replaced missing font padding implementation

Update TiUILabel.java

0e3a766

Added import declarations.

Update TiC.java

0b40c3e

Update TiUILabel.java

4844e14

Renamed PROPERTY_MINIMUM_FONT_SIZE

[TIMOB-1618] Update formatting, update docs

d690b69

hansemannn mentioned this pull request Dec 30, 2016

[TIMOB-1618] Android: Label property minimumFontSize not implemented #5909

Closed

hansemannn requested a review from frankieandone December 30, 2016 12:13

[TIMOB-1618] Update constant

bf84ffc

garymathews self-requested a review December 31, 2016 01:58

garymathews reviewed Dec 31, 2016

View reviewed changes

[TIMOB-1618] Address reviw comments

2e4a042

garymathews reviewed Dec 31, 2016

View reviewed changes

[TIMOB-1618] Remove propertySetFontSize references

d054b18

garymathews added android improvement labels Jan 3, 2017

garymathews added this to the 6.1.0 milestone Jan 3, 2017

frankieandone suggested changes Jan 5, 2017

View reviewed changes

hansemannn added 2 commits January 5, 2017 20:51

Update TiC.java

82cd957

Update TiUILabel.java

73d1b69

Update Label.yml

ddc1b50

frankieandone added the in-qe-testing 🕵 label Jan 11, 2017

frankieandone requested a review from lokeshchdhry January 11, 2017 20:50

frankieandone approved these changes Jan 12, 2017

View reviewed changes

lokeshchdhry merged commit 21af827 into tidev:master Jan 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-1618] Android: Expose "minimumFontSize" on Android (Parity) #8724

[TIMOB-1618] Android: Expose "minimumFontSize" on Android (Parity) #8724

hansemannn commented Dec 30, 2016

garymathews Dec 31, 2016 •

edited

garymathews Dec 31, 2016

garymathews Dec 31, 2016

garymathews Dec 31, 2016

hansemannn commented Dec 31, 2016 •

edited

garymathews Dec 31, 2016

garymathews Dec 31, 2016

garymathews Dec 31, 2016

hansemannn commented Jan 1, 2017

frankieandone left a comment •

edited

frankieandone Jan 5, 2017

frankieandone Jan 5, 2017

hansemannn Jan 5, 2017

frankieandone Jan 5, 2017 •

edited

hansemannn Jan 5, 2017

frankieandone Jan 5, 2017

hansemannn Jan 5, 2017

frankieandone Jan 5, 2017

frankieandone Jan 5, 2017

hansemannn Jan 5, 2017

frankieandone Jan 5, 2017

frankieandone Jan 5, 2017

hansemannn commented Jan 5, 2017

frankieandone commented Jan 5, 2017

frankieandone commented Jan 11, 2017

lokeshchdhry commented Jan 27, 2017

lokeshchdhry commented Jan 27, 2017 •

edited

hansemannn commented Jan 27, 2017

		} else if (key.equals(TiC.PROPERTY_FONT)) {
		if (((HashMap) newValue).containsKey("fontSize")) {

[TIMOB-1618] Android: Expose "minimumFontSize" on Android (Parity) #8724

[TIMOB-1618] Android: Expose "minimumFontSize" on Android (Parity) #8724

Conversation

hansemannn commented Dec 30, 2016

garymathews Dec 31, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented Dec 31, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented Jan 1, 2017

frankieandone left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankieandone Jan 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented Jan 5, 2017

frankieandone commented Jan 5, 2017

frankieandone commented Jan 11, 2017

lokeshchdhry commented Jan 27, 2017

lokeshchdhry commented Jan 27, 2017 • edited

hansemannn commented Jan 27, 2017

garymathews Dec 31, 2016 •

edited

hansemannn commented Dec 31, 2016 •

edited

frankieandone left a comment •

edited

frankieandone Jan 5, 2017 •

edited

lokeshchdhry commented Jan 27, 2017 •

edited