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-14615 add TYPE_TEXT_FLAG_IME_MULTI_LINE #4544

Merged

Conversation

bijupmb
Copy link
Contributor

@bijupmb bijupmb commented Aug 6, 2013

@@ -119,6 +119,8 @@ public TiUIText(TiViewProxy proxy, boolean field)
if (field) {
tv.setSingleLine();
tv.setMaxLines(1);
} else {
tv.setInputType(InputType.TYPE_TEXT_FLAG_IME_MULTI_LINE);
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be done in processProperties and propertyChanged, otherwise it will be overwritten in many cases. Please look at handleKeyboard()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only depends on field variable ( which is used for checking multi line or single line ) and field variable not changed in processProperties and propertyChanged method then why we changed it in processProperties and propertyChanged methods ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is because there are other places where we call setInputType(). If you call it here at the constructor, then later when another setInputType() is called, it will be overwritten. If you look at handleKeyboard() in TiUIText.java, you will see that we append a bunch of options then set them towards the end of that method. For instance, if I create a TextArea with "autocorrect: false", this fix wouldn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified the changes

@hieupham007
Copy link
Contributor

Code reviewed. Please address comments.

@billdawson
Copy link
Contributor

FR/CR passed. I confirmed Hieu's comment was addressed and that the modification works on a 4.2 and 2.3 device. @hieupham007 In my opinion you can merge this and resolve ticket.

@@ -547,6 +550,9 @@ public void handleReturnKeyType(int type)
tv.setImeOptions(EditorInfo.IME_ACTION_DONE);
break;
case RETURNKEY_SEARCH:
if (!field) {
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 this needs to be done for all return keys, not just search.

@hieupham007
Copy link
Contributor

Code reviewed. Looks good, just a couple minor comments :)

@billdawson
Copy link
Contributor

The last commit broke the new functionality. The search button no longer appears (tested on Jelly Bean and Gingerbread devices.)

@@ -530,6 +533,9 @@ public void setSelection(int start, int end)

public void handleReturnKeyType(int type)
{
if (!field) {
tv.setInputType(tv.getInputType() | InputType.TYPE_TEXT_FLAG_IME_MULTI_LINE);
Copy link
Contributor

Choose a reason for hiding this comment

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

apologies. tv.getInputType() is returning a different inputType... I think the right approach is to delete this if statement entirely then in handleKeyboard() if clause of processProperties(), add an else if clause to set the inputType. I.e:
public void processProperties(KrollDict d) {
...
if (...) {
handleKeyboard(d);
} else if (!field) {
tv.setInputType(InputType.TYPE_TEXT_FLAG_IME_MULTI_LINE);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified

@@ -192,6 +192,8 @@ public void processProperties(KrollDict d)

if (d.containsKey(TiC.PROPERTY_KEYBOARD_TYPE) || d.containsKey(TiC.PROPERTY_AUTOCORRECT) || d.containsKey(TiC.PROPERTY_PASSWORD_MASK) || d.containsKey(TiC.PROPERTY_AUTOCAPITALIZATION) || d.containsKey(TiC.PROPERTY_EDITABLE)) {
handleKeyboard(d);
}else if (!field) {
Copy link
Contributor

Choose a reason for hiding this comment

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

need a space between } and else

@hieupham007
Copy link
Contributor

Code reviewed and functionally tested. Looks good. Just a couple minor comments

@hieupham007
Copy link
Contributor

Code reviewed and functionally tested. Request accepted

@hieupham007
Copy link
Contributor

@billdawson are you ok with this? :)

@billdawson
Copy link
Contributor

@hieupham007 Looks good to me. :-)

hieupham007 added a commit that referenced this pull request Sep 5, 2013
…TextArea

TIMOB-14615 add TYPE_TEXT_FLAG_IME_MULTI_LINE
@hieupham007 hieupham007 merged commit 2918667 into tidev:master Sep 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants