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-25855] Android: TextArea lines and maxLines support #9927

Merged
merged 12 commits into from
Jun 27, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
TiC.PROPERTY_HINT_TEXT_COLOR,
TiC.PROPERTY_HINT_TYPE,
TiC.PROPERTY_KEYBOARD_TYPE,
TiC.PROPERTY_LINES,
TiC.PROPERTY_MAX_LINES,
TiC.PROPERTY_MAX_LENGTH,
TiC.PROPERTY_PASSWORD_MASK,
TiC.PROPERTY_TEXT_ALIGN,
Expand All @@ -58,6 +60,8 @@ public TextAreaProxy()
{
super();
defaultValues.put(TiC.PROPERTY_VALUE, "");
defaultValues.put(TiC.PROPERTY_LINES, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think lines should default to -1.

For backward compatibility, we should respect the TextArea's height property. The lines property is used to set the height of the TextArea by line height instead (definitely a good feature) but the developer should opt-in to it.

defaultValues.put(TiC.PROPERTY_MAX_LINES, -1);
defaultValues.put(TiC.PROPERTY_MAX_LENGTH, -1);
defaultValues.put(TiC.PROPERTY_FULLSCREEN, true);
defaultValues.put(TiC.PROPERTY_EDITABLE, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ public void processProperties(KrollDict d)
tv.setImeOptions(EditorInfo.IME_FLAG_NO_FULLSCREEN);
}
}

if (d.containsKey(TiC.PROPERTY_LINES)) {
if (!field) tv.setLines(TiConvert.toInt(d, TiC.PROPERTY_LINES));
}

if (d.containsKey(TiC.PROPERTY_MAX_LINES)) {
if (!field) tv.setMaxLines(TiConvert.toInt(d, TiC.PROPERTY_MAX_LINES));
Copy link
Contributor

Choose a reason for hiding this comment

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

From looking at Google's Textview Java code, they don't expect setMaxLines() to be set to a value <= 0. Google defaults this to Integer.MAX_VALUE by default. We should do the same if the property is set to <= 0. This is to avoid possible layout issues.
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/widget/TextView.java#L658

I believe we might have a similar issue with Android's setLines() method since it set private member variable mMaximum (used as max lines) to the given lines value.
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/widget/TextView.java#L4616

So, if the lines property is <= 0, then we should call setMinLines(0) instead.

I think the best thing to do is to apply lines and maxLines property changes via a method like how I'm doing it in the code below. You don't need the setSingleLine() call, but you do need to call setMaxLines() after calling setLines() since Google resets the maximum afterwards.
https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/widget/TiUILabel.java#L706

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah nice, I'll copy the parts over to the UIText!

}

disableChangeEvent = false;
}

Expand Down Expand Up @@ -367,6 +376,10 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP
if (!TiConvert.toBoolean(newValue, true)) {
tv.setImeOptions(EditorInfo.IME_FLAG_NO_FULLSCREEN);
}
} else if (key.equals(TiC.PROPERTY_LINES)) {
if (!field) tv.setLines(TiConvert.toInt(newValue));
} else if (key.equals(TiC.PROPERTY_MAX_LINES)) {
if (!field) tv.setMaxLines(TiConvert.toInt(newValue));
} else {
super.propertyChanged(key, oldValue, newValue, proxy);
}
Expand Down
16 changes: 16 additions & 0 deletions apidoc/Titanium/UI/TextArea.yml
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,14 @@ properties:
default: <Titanium.UI.KEYBOARD_DEFAULT>
platforms: [android, iphone, ipad]

- name: lines
summary: Line count of text field input.
description: Sets the initial size of lines that the field will have at start.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it clear that lines sets the TextArea height by line height... and get rid of the "text field" references since this only applies to text areas.

How about the following...

summary: Number of lines tall the text area height will be, if set.
description: Sets the height of the text are by line height. Set to -1 to respect the view's height property instead.

default: 1
type: Number
since: {android: "7.2.0"}
platforms: [android]

- name: maxLength
summary: Maximum length of text field input.
description: Any attempt to input text beyond this length (including pasting a string
Expand All @@ -376,6 +384,14 @@ properties:
since: {android: "3.0.0", iphone: "3.0.0", ipad: "3.0.0"}
platforms: [android, iphone, ipad]

- name: maxLines
summary: Maximum line count of text field input.
description: Sets the maximum of lines that the field will be extended to when pressing `Return`.
default: -1
type: Number
since: {android: "7.2.0"}
platforms: [android]

- name: padding
summary: Sets the left and right padding of this TextArea. The text will always be vertically centered.
type: TextAreaPadding
Expand Down