From 22f3bd1073a80f421d7bb5d46f19b48a14786bf4 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Fri, 17 Feb 2017 00:16:09 -0500 Subject: [PATCH] tearing down event handlers on page disappears --- src/App/App.csproj | 1 + src/App/Controls/DismissModalToolBarItem.cs | 19 ++---- src/App/Controls/ExtendedToolbarItem.cs | 30 ++++++++ src/App/Controls/VaultListViewCell.cs | 11 +-- src/App/Pages/RegisterPage.cs | 27 +++++--- src/App/Pages/Tools/ToolsPage.cs | 68 +++++++++++++------ .../Vault/VaultAutofillListLoginsPage.cs | 39 ++++++----- src/App/Pages/Vault/VaultListLoginsPage.cs | 31 +++++---- src/App/Pages/Vault/VaultViewLoginPage.cs | 18 +++-- 9 files changed, 154 insertions(+), 90 deletions(-) create mode 100644 src/App/Controls/ExtendedToolbarItem.cs diff --git a/src/App/App.csproj b/src/App/App.csproj index fdcb8b5cb..2fd6a73c9 100644 --- a/src/App/App.csproj +++ b/src/App/App.csproj @@ -58,6 +58,7 @@ + diff --git a/src/App/Controls/DismissModalToolBarItem.cs b/src/App/Controls/DismissModalToolBarItem.cs index 739f452ba..8e987b2b1 100644 --- a/src/App/Controls/DismissModalToolBarItem.cs +++ b/src/App/Controls/DismissModalToolBarItem.cs @@ -4,14 +4,13 @@ using Xamarin.Forms; namespace Bit.App.Controls { - public class DismissModalToolBarItem : ToolbarItem, IDisposable + public class DismissModalToolBarItem : ExtendedToolbarItem, IDisposable { private readonly ContentPage _page; - private readonly Action _cancelClickedAction; public DismissModalToolBarItem(ContentPage page, string text = null, Action cancelClickedAction = null) + : base(cancelClickedAction) { - _cancelClickedAction = cancelClickedAction; _page = page; // TODO: init and dispose events from pages InitEvents(); @@ -19,20 +18,10 @@ namespace Bit.App.Controls Priority = -1; } - private async void ClickedItem(object sender, EventArgs e) + protected async override void ClickedItem(object sender, EventArgs e) { - _cancelClickedAction?.Invoke(); + base.ClickedItem(sender, e); await _page.Navigation.PopModalAsync(); } - - public void InitEvents() - { - Clicked += ClickedItem; - } - - public void Dispose() - { - Clicked -= ClickedItem; - } } } diff --git a/src/App/Controls/ExtendedToolbarItem.cs b/src/App/Controls/ExtendedToolbarItem.cs new file mode 100644 index 000000000..d6649078a --- /dev/null +++ b/src/App/Controls/ExtendedToolbarItem.cs @@ -0,0 +1,30 @@ +using System; +using Xamarin.Forms; + +namespace Bit.App.Controls +{ + public class ExtendedToolbarItem : ToolbarItem, IDisposable + { + public ExtendedToolbarItem(Action clickAction = null) + { + ClickAction = clickAction; + } + + public Action ClickAction { get; set; } + + protected virtual void ClickedItem(object sender, EventArgs e) + { + ClickAction?.Invoke(); + } + + public void InitEvents() + { + Clicked += ClickedItem; + } + + public void Dispose() + { + Clicked -= ClickedItem; + } + } +} diff --git a/src/App/Controls/VaultListViewCell.cs b/src/App/Controls/VaultListViewCell.cs index 5733fc4b0..1099d9e7d 100644 --- a/src/App/Controls/VaultListViewCell.cs +++ b/src/App/Controls/VaultListViewCell.cs @@ -6,21 +6,17 @@ namespace Bit.App.Controls { public class VaultListViewCell : LabeledDetailCell { - private Action _moreClickedAction; - public static readonly BindableProperty LoginParameterProperty = BindableProperty.Create(nameof(LoginParameter), typeof(VaultListPageModel.Login), typeof(VaultListViewCell), null); public VaultListViewCell(Action moreClickedAction) { - _moreClickedAction = moreClickedAction; - SetBinding(LoginParameterProperty, new Binding(".")); Label.SetBinding(Label.TextProperty, s => s.Name); Detail.SetBinding(Label.TextProperty, s => s.Username); Button.Image = "more"; - Button.Command = new Command(() => ShowMore()); + Button.Command = new Command(() => moreClickedAction?.Invoke(LoginParameter)); Button.BackgroundColor = Color.Transparent; BackgroundColor = Color.White; @@ -31,10 +27,5 @@ namespace Bit.App.Controls get { return GetValue(LoginParameterProperty) as VaultListPageModel.Login; } set { SetValue(LoginParameterProperty, value); } } - - private void ShowMore() - { - _moreClickedAction?.Invoke(LoginParameter); - } } } diff --git a/src/App/Pages/RegisterPage.cs b/src/App/Pages/RegisterPage.cs index d4e184c31..c7f8231e0 100644 --- a/src/App/Pages/RegisterPage.cs +++ b/src/App/Pages/RegisterPage.cs @@ -35,6 +35,9 @@ namespace Bit.App.Pages public FormEntryCell PasswordCell { get; set; } public FormEntryCell ConfirmPasswordCell { get; set; } public FormEntryCell PasswordHintCell { get; set; } + public StackLayout StackLayout { get; set; } + public Label PasswordLabel { get; set; } + public Label HintLabel { get; set; } private void Init() { @@ -71,7 +74,7 @@ namespace Bit.App.Pages } }; - var passwordLabel = new Label + PasswordLabel = new Label { Text = AppResources.MasterPasswordDescription, LineBreakMode = LineBreakMode.WordWrap, @@ -93,7 +96,7 @@ namespace Bit.App.Pages } }; - var hintLabel = new Label + HintLabel = new Label { Text = AppResources.MasterPasswordHintDescription, LineBreakMode = LineBreakMode.WordWrap, @@ -102,21 +105,15 @@ namespace Bit.App.Pages Margin = new Thickness(15, (this.IsLandscape() ? 5 : 0), 15, 25) }; - var layout = new StackLayout + StackLayout = new StackLayout { - Children = { table, passwordLabel, table2, hintLabel }, + Children = { table, PasswordLabel, table2, HintLabel }, Spacing = 0 }; - layout.LayoutChanged += (sender, args) => - { - passwordLabel.WidthRequest = layout.Bounds.Width - passwordLabel.Bounds.Left * 2; - hintLabel.WidthRequest = layout.Bounds.Width - hintLabel.Bounds.Left * 2; - }; - var scrollView = new ScrollView { - Content = layout + Content = StackLayout }; var loginToolbarItem = new ToolbarItem(AppResources.Submit, null, async () => @@ -148,6 +145,7 @@ namespace Bit.App.Pages PasswordHintCell.InitEvents(); ConfirmPasswordCell.InitEvents(); PasswordHintCell.Entry.Completed += Entry_Completed; + StackLayout.LayoutChanged += Layout_LayoutChanged; EmailCell.Entry.FocusWithDelay(); } protected override void OnDisappearing() @@ -158,6 +156,13 @@ namespace Bit.App.Pages PasswordHintCell.Dispose(); ConfirmPasswordCell.Dispose(); PasswordHintCell.Entry.Completed -= Entry_Completed; + StackLayout.LayoutChanged -= Layout_LayoutChanged; + } + + private void Layout_LayoutChanged(object sender, EventArgs e) + { + PasswordLabel.WidthRequest = StackLayout.Bounds.Width - PasswordLabel.Bounds.Left * 2; + HintLabel.WidthRequest = StackLayout.Bounds.Width - HintLabel.Bounds.Left * 2; } private async void Entry_Completed(object sender, EventArgs e) diff --git a/src/App/Pages/Tools/ToolsPage.cs b/src/App/Pages/Tools/ToolsPage.cs index e447c3127..022f0f9f6 100644 --- a/src/App/Pages/Tools/ToolsPage.cs +++ b/src/App/Pages/Tools/ToolsPage.cs @@ -23,42 +23,37 @@ namespace Bit.App.Pages Init(); } + public ToolsViewCell GeneratorCell { get; set; } + public ToolsViewCell WebCell { get; set; } + public ToolsViewCell ImportCell { get; set; } + public ToolsViewCell ExtensionCell { get; set; } + public ToolsViewCell AutofillCell { get; set; } + public void Init() { - var generatorCell = new ToolsViewCell(AppResources.PasswordGenerator, AppResources.PasswordGeneratorDescription, + GeneratorCell = new ToolsViewCell(AppResources.PasswordGenerator, AppResources.PasswordGeneratorDescription, "refresh"); - generatorCell.Tapped += GeneratorCell_Tapped; - var webCell = new ToolsViewCell(AppResources.WebVault, AppResources.WebVaultDescription, "globe"); - webCell.Tapped += WebCell_Tapped; - var importCell = new ToolsViewCell(AppResources.ImportLogins, AppResources.ImportLoginsDescription, "cloudup"); - importCell.Tapped += ImportCell_Tapped; + WebCell = new ToolsViewCell(AppResources.WebVault, AppResources.WebVaultDescription, "globe"); + ImportCell = new ToolsViewCell(AppResources.ImportLogins, AppResources.ImportLoginsDescription, "cloudup"); - var section = new TableSection { generatorCell }; + var section = new TableSection { GeneratorCell }; if(Device.OS == TargetPlatform.iOS) { - var extensionCell = new ToolsViewCell(AppResources.BitwardenAppExtension, + ExtensionCell = new ToolsViewCell(AppResources.BitwardenAppExtension, AppResources.BitwardenAppExtensionDescription, "upload"); - extensionCell.Tapped += (object sender, EventArgs e) => - { - Navigation.PushModalAsync(new ExtendedNavigationPage(new ToolsExtensionPage())); - }; - section.Add(extensionCell); + section.Add(ExtensionCell); } else { - var autofillServiceCell = new ToolsViewCell( + AutofillCell = new ToolsViewCell( string.Format("{0} ({1})", AppResources.BitwardenAutofillService, AppResources.Beta), AppResources.BitwardenAutofillServiceDescription, "upload"); - autofillServiceCell.Tapped += (object sender, EventArgs e) => - { - Navigation.PushModalAsync(new ExtendedNavigationPage(new ToolsAutofillServicePage())); - }; - section.Add(autofillServiceCell); + section.Add(AutofillCell); } - section.Add(webCell); - section.Add(importCell); + section.Add(WebCell); + section.Add(ImportCell); var table = new ExtendedTableView { @@ -81,6 +76,37 @@ namespace Bit.App.Pages Content = table; } + protected override void OnAppearing() + { + base.OnAppearing(); + GeneratorCell.Tapped += GeneratorCell_Tapped; + WebCell.Tapped += WebCell_Tapped; + ImportCell.Tapped += ImportCell_Tapped; + ExtensionCell.Tapped += ExtensionCell_Tapped; + AutofillCell.Tapped += AutofillCell_Tapped; + } + + protected override void OnDisappearing() + { + base.OnDisappearing(); + GeneratorCell.Tapped -= GeneratorCell_Tapped; + WebCell.Tapped -= WebCell_Tapped; + ImportCell.Tapped -= ImportCell_Tapped; + ExtensionCell.Tapped -= ExtensionCell_Tapped; + AutofillCell.Tapped -= AutofillCell_Tapped; + + } + + private void AutofillCell_Tapped(object sender, EventArgs e) + { + Navigation.PushModalAsync(new ExtendedNavigationPage(new ToolsAutofillServicePage())); + } + + private void ExtensionCell_Tapped(object sender, EventArgs e) + { + Navigation.PushModalAsync(new ExtendedNavigationPage(new ToolsExtensionPage())); + } + private async void GeneratorCell_Tapped(object sender, EventArgs e) { await Navigation.PushForDeviceAsync(new ToolsPasswordGeneratorPage()); diff --git a/src/App/Pages/Vault/VaultAutofillListLoginsPage.cs b/src/App/Pages/Vault/VaultAutofillListLoginsPage.cs index 7349746b7..6e5a5788e 100644 --- a/src/App/Pages/Vault/VaultAutofillListLoginsPage.cs +++ b/src/App/Pages/Vault/VaultAutofillListLoginsPage.cs @@ -59,6 +59,8 @@ namespace Bit.App.Pages public StackLayout NoDataStackLayout { get; set; } public ListView ListView { get; set; } public ActivityIndicator LoadingIndicator { get; set; } + private SearchToolBarItem SearchItem { get; set; } + private AddLoginToolBarItem AddLoginItem { get; set; } private IGoogleAnalyticsService GoogleAnalyticsService { get; set; } private IUserDialogs UserDialogs { get; set; } private string Uri { get; set; } @@ -88,8 +90,10 @@ namespace Bit.App.Pages Spacing = 20 }; - ToolbarItems.Add(new AddLoginToolBarItem(this)); - ToolbarItems.Add(new SearchToolBarItem(this)); + AddLoginItem = new AddLoginToolBarItem(this); + ToolbarItems.Add(AddLoginItem); + SearchItem = new SearchToolBarItem(this); + ToolbarItems.Add(SearchItem); ListView = new ListView(ListViewCachingStrategy.RecycleElement) { @@ -106,8 +110,6 @@ namespace Bit.App.Pages ListView.RowHeight = -1; } - ListView.ItemSelected += LoginSelected; - Title = string.Format(AppResources.LoginsForUri, _name ?? "--"); LoadingIndicator = new ActivityIndicator @@ -123,9 +125,20 @@ namespace Bit.App.Pages protected override void OnAppearing() { base.OnAppearing(); + ListView.ItemSelected += LoginSelected; + AddLoginItem.InitEvents(); + SearchItem.InitEvents(); _filterResultsCancellationTokenSource = FetchAndLoadVault(); } + protected override void OnDisappearing() + { + base.OnDisappearing(); + ListView.ItemSelected -= LoginSelected; + AddLoginItem.Dispose(); + SearchItem.Dispose(); + } + protected override bool OnBackButtonPressed() { GoogleAnalyticsService.TrackExtensionEvent("BackClosed", Uri.StartsWith("http") ? "Website" : "App"); @@ -266,26 +279,18 @@ namespace Bit.App.Pages UserDialogs.Toast(string.Format(AppResources.ValueHasBeenCopied, alertLabel)); } - private class AddLoginToolBarItem : ToolbarItem + private class AddLoginToolBarItem : ExtendedToolbarItem { - private readonly VaultAutofillListLoginsPage _page; - public AddLoginToolBarItem(VaultAutofillListLoginsPage page) + : base(() => page.AddLoginAsync()) { - _page = page; Text = AppResources.Add; Icon = "plus"; - Clicked += ClickedItem; Priority = 2; } - - private void ClickedItem(object sender, EventArgs e) - { - _page.AddLoginAsync(); - } } - private class SearchToolBarItem : ToolbarItem + private class SearchToolBarItem : ExtendedToolbarItem { private readonly VaultAutofillListLoginsPage _page; @@ -294,11 +299,11 @@ namespace Bit.App.Pages _page = page; Text = AppResources.Search; Icon = "search"; - Clicked += ClickedItem; Priority = 1; + ClickAction = () => DoClick(); } - private void ClickedItem(object sender, EventArgs e) + private void DoClick() { _page.GoogleAnalyticsService.TrackExtensionEvent("CloseToSearch", _page.Uri.StartsWith("http") ? "Website" : "App"); diff --git a/src/App/Pages/Vault/VaultListLoginsPage.cs b/src/App/Pages/Vault/VaultListLoginsPage.cs index 4246297cc..d66d42594 100644 --- a/src/App/Pages/Vault/VaultListLoginsPage.cs +++ b/src/App/Pages/Vault/VaultListLoginsPage.cs @@ -66,6 +66,7 @@ namespace Bit.App.Pages public StackLayout NoDataStackLayout { get; set; } public StackLayout ResultsStackLayout { get; set; } public ActivityIndicator LoadingIndicator { get; set; } + private AddLoginToolBarItem AddLoginItem { get; set; } public string Uri { get; set; } private void Init() @@ -80,7 +81,8 @@ namespace Bit.App.Pages if(!_favorites) { - ToolbarItems.Add(new AddLoginToolBarItem(this)); + AddLoginItem = new AddLoginToolBarItem(this); + ToolbarItems.Add(AddLoginItem); } ListView = new ListView(ListViewCachingStrategy.RecycleElement) @@ -98,16 +100,12 @@ namespace Bit.App.Pages ListView.RowHeight = -1; } - ListView.ItemSelected += LoginSelected; - Search = new SearchBar { Placeholder = AppResources.SearchVault, FontSize = Device.GetNamedSize(NamedSize.Small, typeof(Button)), CancelButtonColor = Color.FromHex("3c8dbc") }; - Search.TextChanged += SearchBar_TextChanged; - Search.SearchButtonPressed += SearchBar_SearchButtonPressed; // Bug with searchbar on android 7, ref https://bugzilla.xamarin.com/show_bug.cgi?id=43975 if(Device.OS == TargetPlatform.Android && _deviceInfoService.Version >= 24) { @@ -231,6 +229,11 @@ namespace Bit.App.Pages protected override void OnAppearing() { base.OnAppearing(); + ListView.ItemSelected += LoginSelected; + Search.TextChanged += SearchBar_TextChanged; + Search.SearchButtonPressed += SearchBar_SearchButtonPressed; + AddLoginItem?.InitEvents(); + if(_loadExistingData) { _filterResultsCancellationTokenSource = FetchAndLoadVault(); @@ -268,6 +271,15 @@ namespace Bit.App.Pages } } + protected override void OnDisappearing() + { + base.OnDisappearing(); + ListView.ItemSelected -= LoginSelected; + Search.TextChanged -= SearchBar_TextChanged; + Search.SearchButtonPressed -= SearchBar_SearchButtonPressed; + AddLoginItem.Dispose(); + } + protected override bool OnBackButtonPressed() { if(string.IsNullOrWhiteSpace(Uri)) @@ -469,21 +481,16 @@ namespace Bit.App.Pages await Navigation.PushForDeviceAsync(page); } - private class AddLoginToolBarItem : ToolbarItem + private class AddLoginToolBarItem : ExtendedToolbarItem { private readonly VaultListLoginsPage _page; public AddLoginToolBarItem(VaultListLoginsPage page) + : base(() => page.AddLogin()) { _page = page; Text = AppResources.Add; Icon = "plus"; - Clicked += ClickedItem; - } - - private void ClickedItem(object sender, EventArgs e) - { - _page.AddLogin(); } } diff --git a/src/App/Pages/Vault/VaultViewLoginPage.cs b/src/App/Pages/Vault/VaultViewLoginPage.cs index d8a00a43f..cb928b5bb 100644 --- a/src/App/Pages/Vault/VaultViewLoginPage.cs +++ b/src/App/Pages/Vault/VaultViewLoginPage.cs @@ -6,6 +6,7 @@ using Bit.App.Models.Page; using Bit.App.Resources; using Xamarin.Forms; using XLabs.Ioc; +using System.Threading.Tasks; namespace Bit.App.Pages { @@ -33,10 +34,12 @@ namespace Bit.App.Pages public LabeledValueCell UsernameCell { get; set; } public LabeledValueCell PasswordCell { get; set; } public LabeledValueCell UriCell { get; set; } + private EditLoginToolBarItem EditItem { get; set; } private void Init() { - ToolbarItems.Add(new EditLoginToolBarItem(this, _loginId)); + EditItem = new EditLoginToolBarItem(this, _loginId); + ToolbarItems.Add(EditItem); if(Device.OS == TargetPlatform.iOS) { ToolbarItems.Add(new DismissModalToolBarItem(this)); @@ -121,6 +124,8 @@ namespace Bit.App.Pages protected async override void OnAppearing() { + EditItem.InitEvents(); + var login = await _loginService.GetByIdAsync(_loginId); if(login == null) { @@ -169,13 +174,18 @@ namespace Bit.App.Pages base.OnAppearing(); } + protected override void OnDisappearing() + { + EditItem.Dispose(); + } + private void Copy(string copyText, string alertLabel) { _clipboardService.CopyToClipboard(copyText); _userDialogs.Toast(string.Format(AppResources.ValueHasBeenCopied, alertLabel)); } - private class EditLoginToolBarItem : ToolbarItem + private class EditLoginToolBarItem : ExtendedToolbarItem { private readonly VaultViewLoginPage _page; private readonly string _loginId; @@ -185,10 +195,10 @@ namespace Bit.App.Pages _page = page; _loginId = loginId; Text = AppResources.Edit; - Clicked += ClickedItem; + ClickAction = async () => await ClickedItem(); } - private async void ClickedItem(object sender, EventArgs e) + private async Task ClickedItem() { var page = new VaultEditLoginPage(_loginId); await _page.Navigation.PushForDeviceAsync(page);