-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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-24639] Android: Refactor TextInputLayout implementation #9361
Conversation
f0ea621
to
f4d7a3f
Compare
apidoc/Titanium/UI/TextArea.yml
Outdated
@@ -88,6 +88,8 @@ properties: | |||
The underlying attributed string drawn by the textArea. If set, avoid setting common attributes | |||
in textArea, such as `value`, `color` and `font`, as unexpected behaviors may result. | |||
|
|||
This has no effect when used with `hintType` Ti.UI.HINT_TYPE_ANIMATED. | |||
|
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.
Remove this sentence from the "attributedString" property documentation. Hint type does not apply to it.
The TextArea documentation is missing the "attributedHintText" property.
tv.setHint(""); | ||
textInputLayout.setHint(hintText); | ||
} | ||
setHintText(type, hintText); |
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.
Dynamically changing the "hintType" property ignores the "attributedHintText" property. Perhaps it should be written like this?
Object text = d.get(TiC.PROPERTY_ATTRIBUTED_HINT_TEXT);
if (text instanceof AttributedStringProxy) {
setAttributedStringHint((AttributedStringProxy)text);
} else {
text = TiConvert.toString(d.get(TiC.PROPERTY_HINT_TEXT), "");
if (text != null) {
int type = TiConvert.toInt(newValue);
setHintText(type, (String)text);
}
}
nestedScrollView.setFillViewport(true); | ||
nestedScrollView.addView(textInputLayout); | ||
|
||
setNativeView(nestedScrollView); |
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.
We'll need to double check if Titanium's setTouchEnabled(false)
call will gray out the TextField/TextArea like it did before TextInputLayout was implemented. (I forgot to check this before.)
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.
I just verified that setting touchEnabled
property to false
no longer disable TextFields/TextAreas. In fact, they can still be edited.
The best solution to this may be to duplicate what Google has done in their TextInputLayout.java
code where they override the setEnabled() method and pass that state to its children via our own NestedScrollView derived class.
https://github.com/android/platform_frameworks_support/blob/master/design/src/android/support/design/widget/TextInputLayout.java#L843
However, there is another issue. Animated hint text (the text above the TextField/TextArea) can be scrolled off the view because we're wrapping the TextInputLayout within a NestedScrollView. The solution may be to dump the NestedScrollView (which fixes the above enabled
issue) and have the TiUIEditText
class implement the NestedScrollingChild
interface.
@@ -135,10 +137,16 @@ public void onLayoutChange(View v, int left, int top, int right, int bottom, int | |||
} else { | |||
tv.setGravity(Gravity.TOP | Gravity.LEFT); | |||
} | |||
tv.setBackgroundColor(Color.TRANSPARENT); |
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.
Question:
Will this remove the underline even if no custom background was applied?
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.
I just confirmed that the underline is no longer shown. We should only hide it if a custom background color/image is applied.
42d72fc
to
46c6215
Compare
@@ -295,22 +296,24 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP | |||
tv.setText(truncateText); | |||
tv.setSelection(cursor); | |||
} | |||
} else if (key.equals(TiC.PROPERTY_BACKGROUND_COLOR)) { | |||
tv.setBackgroundColor(Color.TRANSPARENT); |
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.
The below needs to be called after changing TextView background color to transparent since we're in an else-if block (the super class' method only gets called in the else block).
super.propertyChanged(key, oldValue, newValue, proxy);
@@ -17,6 +17,7 @@ | |||
import ti.modules.titanium.ui.widget.TiUIText; | |||
import android.graphics.drawable.Drawable; | |||
import android.support.design.widget.TextInputLayout; | |||
import android.support.v4.widget.NestedScrollView; |
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.
This import statement isn't needed anymore, right?
apidoc/Titanium/UI/TextArea.yml
Outdated
- name: hintType | ||
summary: Hint type to display on the text field. | ||
platforms: [android] | ||
since: {android: "6.2.0"} |
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.
Change to version 7.0.0
Side Note:
I resolved it in my nested TextArea scrolling PR ( #9402 ) by modifying |
d7e3eaa
to
afafe4d
Compare
@jquick-axway Updated PR |
Tests:
Generated by 🚫 dangerJS |
d737fe2
to
44fe658
Compare
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.
CR: Pass
@garymathews, oh wait. The new Ti.UI.HINT_TYPE_ANIMATED constant is currently flagged as deprecated here... |
FR Passed.
Studio Ver: 4.10.0.201709271713 |
TIMOB-25086: UseNestedScrollView
to allow nested scrolling (e.g: TableView, ScrollView)attributedHintText
, although attributes will be ignored (Android limitation)height
propertyEditText
background color to transparenthintType: Ti.UI.HINT_TYPE_ANIMATED
(e.g: cannot sethintTextColor
, again Android limitation)TEST CASES