From 7d25f182333d20c14c4975794b5cf42487ec9b70 Mon Sep 17 00:00:00 2001 From: Gerald Versluis Date: Thu, 26 Sep 2019 22:18:17 -0700 Subject: [PATCH] [Android] 28+ Make non-visible pickers work again (#7289) * Repro + fix * Final fix * Update Issue5159.cs * Fixes #7311 * Test tweaks * Update TimePickerRenderer.cs * Revert "Update TimePickerRenderer.cs" This reverts commit 06c1172d2501ec533cdf5b1eabafa1d402687980. * Update TimePickerRenderer.cs * Update TimePickerRenderer.cs * - added instructions --- .../Issue5159.cs | 164 +++++++++++++++++++++ .../Issue7311.cs | 63 ++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 2 + .../AppCompat/PickerRenderer.cs | 7 +- Xamarin.Forms.Platform.Android/PickerManager.cs | 6 +- .../Renderers/DatePickerRenderer.cs | 7 +- .../Renderers/PickerRenderer.cs | 9 +- .../Renderers/TimePickerRenderer.cs | 28 +++- 8 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5159.cs create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7311.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5159.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5159.cs new file mode 100644 index 0000000..d7415c3 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5159.cs @@ -0,0 +1,164 @@ +using System; +using Xamarin.Forms.Internals; +using Xamarin.Forms.CustomAttributes; +#if UITEST +using Xamarin.Forms.Core.UITests; +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ +#if UITEST + [Category(UITestCategories.Picker)] + [Category(UITestCategories.DatePicker)] + [Category(UITestCategories.TimePicker)] +#endif + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Github, 5159, "[Android] Calling Focus on all Pickers running an API 28 devices no longer opens Picker", PlatformAffected.Android)] + public class Issue5159 : TestContentPage + { + const string DatePickerButton = "DatePickerButton"; + const string TimePickerButton = "TimePickerButton"; + const string PickerButton = "PickerButton"; + readonly string[] _pickerValues = { "Foo", "Bar", "42", "1337" }; + + protected override void Init() + { + var stackLayout = new StackLayout + { + VerticalOptions = LayoutOptions.Center, + HorizontalOptions = LayoutOptions.Center + }; + + // DatePicker + var datePickerButton = new Button + { + Text = "Show DatePicker", + AutomationId = DatePickerButton + }; + + var datePicker = new DatePicker + { + IsVisible = false + }; + + datePickerButton.Clicked += (s, a) => + { + Device.BeginInvokeOnMainThread(() => + { + if (datePicker.IsFocused) + datePicker.Unfocus(); + + datePicker.Focus(); + }); + }; + + // TimePicker + var timePickerButton = new Button + { + Text = "Show TimePicker", + AutomationId = TimePickerButton + }; + + var timePicker = new TimePicker + { + IsVisible = false + }; + + timePickerButton.Clicked += (s, a) => + { + Device.BeginInvokeOnMainThread(() => + { + if (timePicker.IsFocused) + timePicker.Unfocus(); + + timePicker.Focus(); + }); + }; + + // Picker + var pickerButton = new Button + { + Text = "Show Picker", + AutomationId = PickerButton + }; + + var picker = new Picker + { + IsVisible = false, + ItemsSource = _pickerValues + }; + + pickerButton.Clicked += (s, a) => + { + Device.BeginInvokeOnMainThread(() => + { + if (picker.IsFocused) + picker.Unfocus(); + + picker.Focus(); + }); + }; + + stackLayout.Children.Add(datePickerButton); + stackLayout.Children.Add(datePicker); + + stackLayout.Children.Add(timePickerButton); + stackLayout.Children.Add(timePicker); + + stackLayout.Children.Add(pickerButton); + stackLayout.Children.Add(picker); + + Content = stackLayout; + } + +#if UITEST && __ANDROID__ + [Test] + [UiTest(typeof(DatePicker))] + public void InvisibleDatepickerShowsDialogOnFocus() + { + RunningApp.WaitForElement(DatePickerButton); + RunningApp.Screenshot("Issue 5159 page is showing in all it's glory"); + RunningApp.Tap(DatePickerButton); + + RunningApp.WaitForElement(x => x.Class("DatePicker")); + + RunningApp.Screenshot("DatePicker is shown"); + RunningApp.TapCoordinates(5, 100); + } + + [Test] + [UiTest(typeof(TimePicker))] + public void InvisibleTimepickerShowsDialogOnFocus() + { + RunningApp.WaitForElement(TimePickerButton); + RunningApp.Screenshot("Issue 5159 page is showing in all it's glory"); + RunningApp.Tap(TimePickerButton); + + RunningApp.WaitForElement(x => x.Class("timePicker")); + + RunningApp.Screenshot("TimePicker is shown"); + RunningApp.TapCoordinates(5, 100); + } + + [Test] + [UiTest(typeof(Picker))] + public void InvisiblePickerShowsDialogOnFocus() + { + RunningApp.WaitForElement(PickerButton); + RunningApp.Screenshot("Issue 5159 page is showing in all it's glory"); + RunningApp.Tap(PickerButton); + + RunningApp.WaitForElement("Foo"); + + RunningApp.Screenshot("Picker is shown"); + + RunningApp.Tap("Foo"); + + RunningApp.WaitForNoElement("Foo"); + + } +#endif + } +} diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7311.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7311.cs new file mode 100644 index 0000000..6f3ce3e --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7311.cs @@ -0,0 +1,63 @@ +using System; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.UITest.iOS; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Github, 7311, "[Bug] [Android] Error back hardware button with Picker", PlatformAffected.Android)] + public class Issue7311 : TestContentPage + { + const string FirstPickerItem = "Uno"; + const string PickerId = "CaptainPickard"; + readonly string[] _items = { FirstPickerItem, "Dos", "Tres" }; + + protected override void Init() + { + var picker = new Picker + { + ItemsSource = _items, + AutomationId = PickerId + }; + + Content = new StackLayout() + { + Children = + { + new Label() + { + Text = "Open Picker. Click hardware back button to close picker. Click hardware button a second time and it should navigate back to gallery" + }, + picker + } + }; + } + +#if UITEST && __ANDROID__ + [Test] + public void OpeningPickerPressingBackButtonTwiceShouldNotOpenPickerAgain() + { + RunningApp.WaitForElement(PickerId); + RunningApp.Tap(PickerId); + + RunningApp.WaitForElement(FirstPickerItem); + + RunningApp.Back(); + + RunningApp.WaitForNoElement(FirstPickerItem); + + RunningApp.Back(); + + RunningApp.WaitForNoElement(FirstPickerItem, "Picker is again visible after second back button press", TimeSpan.FromSeconds(10)); + + RunningApp.Screenshot("Back at the previous page, not showing the picker again"); + } +#endif + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 61ffec7..369f053 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -1019,6 +1019,8 @@ + + diff --git a/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs index 0ae6200..e46ab89 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs @@ -90,7 +90,12 @@ namespace Xamarin.Forms.Platform.Android.AppCompat base.OnFocusChangeRequested(sender, e); if (e.Focus) - CallOnClick(); + { + if (Clickable) + CallOnClick(); + else + ((IPickerRenderer)this)?.OnClick(); + } else if (_dialog != null) { _dialog.Hide(); diff --git a/Xamarin.Forms.Platform.Android/PickerManager.cs b/Xamarin.Forms.Platform.Android/PickerManager.cs index 471494f..aefbd79 100644 --- a/Xamarin.Forms.Platform.Android/PickerManager.cs +++ b/Xamarin.Forms.Platform.Android/PickerManager.cs @@ -10,8 +10,8 @@ namespace Xamarin.Forms.Platform.Android { internal static class PickerManager { - readonly static HashSet availableKeys = new HashSet(new[] { - Keycode.Tab, Keycode.Forward, Keycode.Back, Keycode.DpadDown, Keycode.DpadLeft, Keycode.DpadRight, Keycode.DpadUp + readonly static HashSet AvailableKeys = new HashSet(new[] { + Keycode.Tab, Keycode.Forward, Keycode.DpadDown, Keycode.DpadLeft, Keycode.DpadRight, Keycode.DpadUp }); public static void Init(EditText editText) @@ -42,7 +42,7 @@ namespace Xamarin.Forms.Platform.Android static void OnKeyPress(object sender, AView.KeyEventArgs e) { - if (!availableKeys.Contains(e.KeyCode)) + if (!AvailableKeys.Contains(e.KeyCode)) { e.Handled = false; return; diff --git a/Xamarin.Forms.Platform.Android/Renderers/DatePickerRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/DatePickerRenderer.cs index cce0a5b..be04ecb 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/DatePickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/DatePickerRenderer.cs @@ -94,7 +94,12 @@ namespace Xamarin.Forms.Platform.Android base.OnFocusChangeRequested(sender, e); if (e.Focus) - CallOnClick(); + { + if (Clickable) + CallOnClick(); + else + ((IPickerRenderer)this)?.OnClick(); + } else if (_dialog != null) { _dialog.Hide(); diff --git a/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs index 26d25d6..4c0e2aa 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs @@ -104,7 +104,12 @@ namespace Xamarin.Forms.Platform.Android base.OnFocusChangeRequested(sender, e); if (e.Focus) - CallOnClick(); + { + if (Clickable) + CallOnClick(); + else + ((IPickerRenderer)this)?.OnClick(); + } else if (_dialog != null) { _dialog.Hide(); @@ -119,7 +124,7 @@ namespace Xamarin.Forms.Platform.Android if (_dialog != null) return; - + var picker = new NumberPicker(Context); if (model.Items != null && model.Items.Any()) { diff --git a/Xamarin.Forms.Platform.Android/Renderers/TimePickerRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/TimePickerRenderer.cs index d0ed9fc..1ce033e 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/TimePickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/TimePickerRenderer.cs @@ -16,6 +16,7 @@ namespace Xamarin.Forms.Platform.Android { int _originalHintTextColor; AlertDialog _dialog; + bool _isDisposed; bool Is24HourView { @@ -86,7 +87,12 @@ namespace Xamarin.Forms.Platform.Android base.OnFocusChangeRequested(sender, e); if (e.Focus) - CallOnClick(); + { + if (Clickable) + CallOnClick(); + else + ((IPickerRenderer)this)?.OnClick(); + } else if (_dialog != null) { _dialog.Hide(); @@ -95,6 +101,7 @@ namespace Xamarin.Forms.Platform.Android if (Forms.IsLollipopOrNewer) _dialog.CancelEvent -= OnCancelButtonClicked; + _dialog?.Dispose(); _dialog = null; } } @@ -109,6 +116,25 @@ namespace Xamarin.Forms.Platform.Android return dialog; } + protected override void Dispose(bool disposing) + { + if (_isDisposed) + return; + + _isDisposed = true; + + if (disposing) + { + if (Forms.IsLollipopOrNewer && _dialog.IsAlive()) + _dialog.CancelEvent -= OnCancelButtonClicked; + + _dialog?.Dispose(); + _dialog = null; + } + + base.Dispose(disposing); + } + void IPickerRenderer.OnClick() { if (_dialog != null && _dialog.IsShowing) -- 2.7.4