-
-
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-25655] Fix Android: Android: Set textfield value by coding will make cursor to the beginning of textfield #9710
Conversation
Generated by 🚫 dangerJS |
* @param type TextView.BufferType - characteristics of the text such as static, styleable, or editable. | ||
*/ | ||
@Override | ||
public void setText(CharSequence text, BufferType type) { |
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.
Jenkins now checks our code changes to see if it conforms to our code formatting rules. This causes a failure because a method's starting curly brace '{' needs to be below the method.
@Override | ||
public void setText(CharSequence text, BufferType type) { | ||
super.setText(text, type); | ||
setSelection(text.length()); |
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 tested iOS and noticed that the cursor is only moved to the end of the TextField if the text actually changes. It won't change the cursor position (or text selection) if setting the value
property to the same text it already has. So, we need to do an equality check like this...
@Override
public void setText(CharSequence text, BufferType type)
{
// Do not continue if the text is unchanged.
CharSequence oldText = getText();
if (text == null) {
if ((oldText == null) || oldText.equals("")) {
return;
}
} else if (text.equals(oldText)) {
return;
}
// Update the field's text.
super.setText(text, type);
// Move the cursor to the end of the field. (Matches iOS' behavior.)
setSelection(length());
}
Also note that your setSelection()
call wasn't checking if the text
argument was null
. My setSelection()
method call above fetches length from the EditText
field instead, avoiding the null pointer exception case.
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.
Thanks for the thoughtful reviews! I tied it out and had this error message:
Caused by: java.lang.ClassCastException: java.lang.String cannot be cast to android.text.Editable
at android.widget.EditText.getText(EditText.java:84)
at ti.modules.titanium.ui.widget.TiUIEditText.setText(TiUIEditText.java:112)
EditText.java line 84 is
public Editable getText() {
return (Editable) super.getText();
}
TiUIEditText.java line 112 is
CharSequence oldText = getText();
I've tried sth like getText().toString() and SpannableStringBuilder etc. It seems come from EditText/TextView source code. At the time generating this error, it looks like doing initialisation, the super.getText() returns a String "" and also the text to be set is "". Any idea how could I avoid the error? @jquick-axway
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.
Oh bummer. This is a bug on Google's end. The returned Editable
type does implement the CharSequence
interface. So, what we're doing on our end is correct. The issue is that Google's getText()
method blindly casts the returned value from super.getText()
which defaults to an empty string in Google's TextView
constructor. The Java String
type does not implement the Editable
interface, hence the class cast exception. :(
We can work-around the issue mentioned above, but I don't want to add extra complexity to the code. So, let's change the code back to what you had before (sorry), but use the EditText.length() method to set the cursor position instead to avoid the null exception case.
@Override
public void setText(CharSequence text, BufferType type)
{
super.setText(text, type);
setSelection(length());
}
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.
Noted with thanks!
Formatting and update
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
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.
FR passed waiting for Jenkins build to pass.
Tested using the test case the ticket and above, the cursor is now at the end of the textfield in all cases.
ENV
SDK Version : 7.0.1.GA, Local 7.1.0
macOS Sierra 10.13.2
Google Pixel 2 XL (8.1.0)
android emulator (7.1.1)
Appc CLI : 7.0.2-master.5
Appc NPM : 4.2.11
Node : v8.9.1
JIRA: https://jira.appcelerator.org/browse/TIMOB-25655
Test case: