[Android] Show keyboard on app resume if control has focus #480

Merged
merged 3 commits into from Nov 15, 2016

Conversation

Projects
None yet
7 participants
@adrianknight89
Contributor

adrianknight89 commented Oct 21, 2016

Description of Change

Converting the work done in #441 to platform-specific functionality.

API Changes

Added:

  • public static readonly BindableProperty CanShowKeyboardOnResumeProperty
  • public static bool GetCanShowKeyboardOnResume(BindableObject element)
  • public static void SetCanShowKeyboardOnResume(BindableObject element, bool value)
  • public static bool GetCanShowKeyboardOnResume(this IPlatformElementConfiguration<Android, FormsElement> config)
  • public static IPlatformElementConfiguration<Android, FormsElement> CanShowKeyboardOnResume(this IPlatformElementConfiguration<Android, FormsElement> config, bool value)

EDIT: APIs were renamed to reflect PreserveKeyboardOnResume.

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
@@ -53,5 +49,28 @@ public static bool GetSendAppearingEventOnResume(this IPlatformElementConfigurat
SetSendAppearingEventOnResume(config.Element, value);
return config;
}
+
+ public static readonly BindableProperty CanShowKeyboardOnResumeProperty = BindableProperty.Create(nameof(CanShowKeyboardOnResume), typeof(bool), typeof(Application), false);

This comment has been minimized.

@pauldipietro

pauldipietro Oct 21, 2016

Member

Semantically speaking here, with this feature available, the keyboard can have the ability to be shown, but ultimately is or isn't, so I'd name this ShouldShowKeyboardOnResume / ShouldShowKeyboardOnResumeProperty. Just my two cents.

@pauldipietro

pauldipietro Oct 21, 2016

Member

Semantically speaking here, with this feature available, the keyboard can have the ability to be shown, but ultimately is or isn't, so I'd name this ShouldShowKeyboardOnResume / ShouldShowKeyboardOnResumeProperty. Just my two cents.

This comment has been minimized.

@adrianknight89

adrianknight89 Oct 21, 2016

Contributor

I thought about this. ShouldShowKeyboardOnResume implies that the keyboard should be shown on resume regardless of whether a control has focus or not. I went with Can because there needs to be a focused control available. That said, I'm not great with semantics, so I can change the naming if needed.

@adrianknight89

adrianknight89 Oct 21, 2016

Contributor

I thought about this. ShouldShowKeyboardOnResume implies that the keyboard should be shown on resume regardless of whether a control has focus or not. I went with Can because there needs to be a focused control available. That said, I'm not great with semantics, so I can change the naming if needed.

This comment has been minimized.

@ahmedalejo

ahmedalejo Oct 21, 2016

ShouldShowKeyboardOnResumeProperty to PreserveKeyboardVisibilityOnResumeProperty

@ahmedalejo

ahmedalejo Oct 21, 2016

ShouldShowKeyboardOnResumeProperty to PreserveKeyboardVisibilityOnResumeProperty

This comment has been minimized.

@adrianknight89

adrianknight89 Oct 24, 2016

Contributor

Changed to ShouldPreserveKeyboardOnResume.

@adrianknight89

adrianknight89 Oct 24, 2016

Contributor

Changed to ShouldPreserveKeyboardOnResume.

@jassmith jassmith merged commit 04cc360 into xamarin:master Nov 15, 2016

1 of 3 checks passed

iOS10-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests failed: 1 (…
Details
iOS8-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests failed: 1, p…
Details
iOS9-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 346,…
Details
@rmarinho

This comment has been minimized.

Show comment
Hide comment
Member

rmarinho commented Nov 29, 2016

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Nov 29, 2016

Contributor

@rmarinho See my response. It appears that something added to 2.3.4-pre1 is rendering this PR obsolete, so for now we could revert it and add it back later if we can reproduce the issue.

Contributor

adrianknight89 commented Nov 29, 2016

@rmarinho See my response. It appears that something added to 2.3.4-pre1 is rendering this PR obsolete, so for now we could revert it and add it back later if we can reproduce the issue.

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

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