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

[Android] Keyboard should not change layout while dismissing on Entry and Editor #663

Merged
merged 6 commits into from Jan 26, 2017

Conversation

Projects
None yet
5 participants
@adrianknight89
Contributor

adrianknight89 commented Dec 30, 2016

Description of Change

When the soft keyboard is dismissing on Entry and Editor, it changes characters. This seems to happen randomly instead of following a consistent pattern unless you use a Google keyboard (tested on S6) in which case it seems to happen every time.

Bugs Fixed

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Jan 25, 2017

Contributor

@hartez Since you're looking at the other keyboard issue, any way this can be given priority as well?

Contributor

adrianknight89 commented Jan 25, 2017

@hartez Since you're looking at the other keyboard issue, any way this can be given priority as well?

@hartez

Just needs a couple of changes to the test; other than that, looks good.

Show outdated Hide outdated ...ms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla43867.cs
{
Text = "Focus and unfocus each element 10 times. Observe that the soft keyboard does not show different characters while hiding."
},
new Entry

This comment has been minimized.

@hartez

hartez Jan 26, 2017

Member

Your test doesn't set the keyboards to numeric, so the effect you're testing against never happens.

@hartez

hartez Jan 26, 2017

Member

Your test doesn't set the keyboards to numeric, so the effect you're testing against never happens.

This comment has been minimized.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Sorry, the issue exists for other kinds of keyboard types as well although in the bug description I had mentioned numeric keyboard only. Made the change to make it aligned with the bug.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Sorry, the issue exists for other kinds of keyboard types as well although in the bug description I had mentioned numeric keyboard only. Made the change to make it aligned with the bug.

Show outdated Hide outdated ...ms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla43867.cs
{
new Label
{
Text = "Focus and unfocus each element 10 times. Observe that the soft keyboard does not show different characters while hiding."

This comment has been minimized.

@hartez

hartez Jan 26, 2017

Member

You should specify that the tester also needs to verify that the problem doesn't occur when hitting the 'back' button; I've never seen this problem reproduced just from tapping off of the Entry/Editor to unfocus them. This might be different on other hardware/emulators, but the test should cover both.

@hartez

hartez Jan 26, 2017

Member

You should specify that the tester also needs to verify that the problem doesn't occur when hitting the 'back' button; I've never seen this problem reproduced just from tapping off of the Entry/Editor to unfocus them. This might be different on other hardware/emulators, but the test should cover both.

This comment has been minimized.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Done.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Done.

@hartez

hartez approved these changes Jan 26, 2017

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Jan 26, 2017

Member

It's failing a test 45926 on Android, can you check @adrianknight89 ?

Member

rmarinho commented Jan 26, 2017

It's failing a test 45926 on Android, can you check @adrianknight89 ?

if (keyCode != Keycode.Back || e.Action != KeyEventActions.Down)
return base.OnKeyPreIme(keyCode, e);
this.HideKeyboard();

This comment has been minimized.

@rmarinho

rmarinho Jan 26, 2017

Member

can't we drop the this ?

@rmarinho

rmarinho Jan 26, 2017

Member

can't we drop the this ?

This comment has been minimized.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Since HideKeyboard is an extension method, this is needed.

@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

Since HideKeyboard is an extension method, this is needed.

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Jan 26, 2017

Contributor

@rmarinho I just ran 45926, and it seems to be passing for me. Can you run it again to see if it wasn't an intermittent issue?

Contributor

adrianknight89 commented Jan 26, 2017

@rmarinho I just ran 45926, and it seems to be passing for me. Can you run it again to see if it wasn't an intermittent issue?

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Jan 26, 2017

Member

@adrianknight89 yap .. we have alot of runs queued but it's up there to be run again.

Member

rmarinho commented Jan 26, 2017

@adrianknight89 yap .. we have alot of runs queued but it's up there to be run again.

@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Jan 26, 2017

Member

@rmarinho @adrianknight89 Just checked it, the failure on 45926 is that the version in this PR is slightly out-of-date. There was a change to that test checked in to master a few days back. So it's not the changes in this PR which are causing that failure.

Member

hartez commented Jan 26, 2017

@rmarinho @adrianknight89 Just checked it, the failure on 45926 is that the version in this PR is slightly out-of-date. There was a change to that test checked in to master a few days back. So it's not the changes in this PR which are causing that failure.

@hartez hartez merged commit c013452 into xamarin:master Jan 26, 2017

2 of 3 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests failed: 1, pas…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3695, ignored: 10
Details

@samhouts samhouts added D-15.4 and removed cla-already-signed labels Oct 10, 2017

@samhouts samhouts modified the milestone: 3.1.0 Jun 1, 2018

@samhouts samhouts added this to Done in vNext+1 (master) Jun 26, 2018

@samhouts samhouts removed this from Done in vNext+1 (master) Jun 26, 2018

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment