FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(android): returnPress invoked twice for some returnKeyTypes by EddyVerbruggen · Pull Request #5708 · NativeScript/NativeScript · GitHub

fix(android): returnPress invoked twice for some returnKeyTypes - #5708

Closed
EddyVerbruggen wants to merge 6 commits into
NativeScript:masterfrom
EddyVerbruggen:issue4461-returnpress
Closed

EddyVerbruggen wants to merge 6 commits into
NativeScript:masterfrom
EddyVerbruggen:issue4461-returnpress

Conversation

Copy link
Copy Markdown
Contributor

PR Checklist

What is the current behavior?

A returnKeyType like IME_ACTION_GO also fires an IME_ACTION_UNSPECIFIED actionId with event.getKeyCode() === KEYCODE_ENTER while navigating to the next field. This will trigger the returnPress event twice.

What is the new behavior?

Check for the event not being IME_ACTION_UNSPECIFIED when evaluating the keycode.

Fixes #4461.

ghost assigned EddyVerbruggen Apr 19, 2018
ghost added the in progress label Apr 19, 2018
ghost assigned SvetoslavTsenov Apr 20, 2018
manoldonev changed the title returnPress is invoked twice on Android for some returnKeyTypes fix(android): returnPress invoked twice for some returnKeyTypes Apr 20, 2018
actionId === android.view.inputmethod.EditorInfo.IME_ACTION_SEARCH ||
actionId === android.view.inputmethod.EditorInfo.IME_ACTION_SEND ||
(event && event.getKeyCode() === android.view.KeyEvent.KEYCODE_ENTER)) {
(actionId !== android.view.inputmethod.EditorInfo.IME_ACTION_UNSPECIFIED && event && event.getKeyCode() === android.view.KeyEvent.KEYCODE_ENTER)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

hey @EddyVerbruggen -- I might be missing something but I don't think the proposed addition changes the logic of the conditional statement. If actionId was IME_ACTION_UNSPECIFIED the conditional would have been truthy because of the 2nd row in the statement so if we progressed till the end we already know it is not IME_ACTION_UNSPECIFIED and we do not need to check for it explicitly:

            if (actionId === android.view.inputmethod.EditorInfo.IME_NULL ||
                actionId === android.view.inputmethod.EditorInfo.IME_ACTION_UNSPECIFIED ||
                actionId === android.view.inputmethod.EditorInfo.IME_ACTION_DONE ||
                actionId === android.view.inputmethod.EditorInfo.IME_ACTION_GO ||
                actionId === android.view.inputmethod.EditorInfo.IME_ACTION_SEARCH ||
                actionId === android.view.inputmethod.EditorInfo.IME_ACTION_SEND ||
                (actionId !== android.view.inputmethod.EditorInfo.IME_ACTION_UNSPECIFIED && event && event.getKeyCode() === android.view.KeyEvent.KEYCODE_ENTER)) {
                // If it is TextField, close the keyboard. If it is TextView, do not close it since the TextView is multiline
                // https://github.com/NativeScript/NativeScript/issues/3111
                if (textView.getMaxLines() === 1) {
                    owner.dismissSoftInput();
                }

                owner._onReturnPress();
            }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@manoldonev That's a very valid point. I tested this change with the 3.4 version of code modules where the actionId === android.view.inputmethod.EditorInfo.IME_NULL || actionId === android.view.inputmethod.EditorInfo.IME_ACTION_UNSPECIFIED || lines weren't present (both those static properties resolve to 0 btw) so that worked as expected. In the 4.0 branch this change doesn't make sense anymore as you pointed out.

EddyVerbruggen Apr 20, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@manoldonev I wonder why we need those first 2 lines anyway, because there doesn't seem to be a returnPress event being fired on a TextView anyway.

I mean, this is the implementation on TextField, and there's no implemenation on TextView, and the superclass implementation is empty.

So I think we could remove those 2 lines from the if statement and this will work. Thoughts?

manoldonev Apr 23, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@EddyVerbruggen @hshristov added those lines to fix #5121 (new lines with Japanese IME).

I wonder now whether we can remove the IME_ACTION_GO, IME_ACTION_SEARCH, and IME_ACTION_SEND explicit checks as it seems each of them gets a complementary IME_ACTION_UNSPECIFIED that we will handle -- have to check this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

New PR proposal for this issue here: #5727

ghost assigned manoldonev Apr 23, 2018
manoldonev added this to the 4.1 milestone Apr 23, 2018

Copy link
Copy Markdown
Contributor

Closing this in favor of #5727

manoldonev closed this Apr 25, 2018
ghost removed the in progress label Apr 25, 2018

lock Bot commented Aug 26, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Aug 26, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReturnPress is invoked twice on returnKey tap

4 participants


Back | FazBrowse Home | New Git URL