From ffc51af6a86ab31b5128e4b3135aa70baac30d36 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 17:29:22 +0200 Subject: [PATCH 1/7] Make sure the toolbar is reset when the controller is stopped --- .../UI/Views/GitHubPaneViewModel.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 3b2c494f59..36ca29f0ff 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -118,13 +118,7 @@ async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) if (!IsGitHubRepo.Value) { - if (uiController != null) - { - Stop(); - //var factory = ServiceProvider.GetExportedValue(); - //var c = factory.CreateViewAndViewModel(UIViewType.LoggedOut); - //Control = c.View; - } + Stop(); return; } @@ -249,12 +243,17 @@ void DisableButtons() void Stop() { + if (uiController == null) + return; + + DisableButtons(); windowController?.Close(); uiController.Stop(); disposables.Clear(); uiController = null; currentNavItem = -1; navStack.Clear(); + UpdateToolbar(); } string title; From aeb4469d68d74ea4d34a9ee6d0cf0f8368a4cd9e Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 18:00:45 +0200 Subject: [PATCH 2/7] Rewrite logic of loading individual views in the GitHubPane Rewrite the logic of loading individual views that are not controlled by an UIController in the GitHubPane, so it's clearer what needs to happen and in which conditions. --- .../UI/Views/GitHubPaneViewModel.cs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 36ca29f0ff..bfc63453fc 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -118,34 +118,37 @@ async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) if (!IsGitHubRepo.Value) { - Stop(); + //LoadView(UIViewType.NotAGitHubRepo); return; } var connection = await connectionManager.LookupConnection(ActiveRepo); IsLoggedIn = await connection.IsLoggedIn(hosts); - if (IsLoggedIn) + if (!IsLoggedIn) { - if (uiController == null || (data != null && data.ActiveFlow != uiController.CurrentFlow)) - StartFlow(data?.ActiveFlow ?? UIControllerFlow.PullRequests, connection, data); - else if (data != null || currentNavItem >= 0) - uiController.Jump(data ?? navStack[currentNavItem]); - } - else - { - var factory = ServiceProvider.GetExportedValue(); - var c = factory.CreateViewAndViewModel(UIViewType.LoggedOut); - c.View.DataContext = c.ViewModel; - Control = c.View; + LoadView(UIViewType.LoggedOut); + return; } - return; + + if (uiController == null || (data != null && data.ActiveFlow != uiController.SelectedFlow)) + StartFlow(data?.ActiveFlow ?? UIControllerFlow.PullRequests, connection, data); + else if (data != null || currentNavItem >= 0) + uiController.Jump(data ?? navStack[currentNavItem]); + } + + void LoadView(UIViewType type) + { + Stop(); + var factory = ServiceProvider.GetExportedValue(); + var c = factory.CreateViewAndViewModel(type); + c.View.DataContext = c.ViewModel; + Control = c.View; } void StartFlow(UIControllerFlow controllerFlow, [AllowNull]IConnection conn, ViewWithData data = null) { - if (uiController != null) - Stop(); + Stop(); if (conn == null) return; From a94e297fe8fa23ab1b7752c6f65c696255fa3aa6 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 18:07:16 +0200 Subject: [PATCH 3/7] Fix build --- src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index bfc63453fc..2a16167244 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -131,7 +131,7 @@ async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) return; } - if (uiController == null || (data != null && data.ActiveFlow != uiController.SelectedFlow)) + if (uiController == null || (data != null && data.ActiveFlow != uiController.CurrentFlow)) StartFlow(data?.ActiveFlow ?? UIControllerFlow.PullRequests, connection, data); else if (data != null || currentNavItem >= 0) uiController.Jump(data ?? navStack[currentNavItem]); From d67e4f4ce267ff233521dac91c45fa9cb05587a0 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 20:22:23 +0200 Subject: [PATCH 4/7] UIController needs to be stopped when switching repos --- src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 2a16167244..2f292b4bab 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -105,6 +105,7 @@ protected override void RepoChanged(bool changed) if (!changed) return; + Stop(); IsGitHubRepo = null; Reload().Forget(); } From 295bcbd7478cb69264c80010750333d9752d8a96 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 20:22:51 +0200 Subject: [PATCH 5/7] Make Reload reentrant --- .../UI/Views/GitHubPaneViewModel.cs | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 2f292b4bab..ff13be91c8 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -19,6 +19,7 @@ using ReactiveUI; using System.Collections.Generic; using System.Threading.Tasks; +using System.Diagnostics; namespace GitHub.VisualStudio.UI.Views { @@ -40,6 +41,7 @@ public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel bool navigatingViaArrows; bool disabled; Microsoft.VisualStudio.Shell.OleMenuCommand back, forward, refresh; + int latestReloadCallId; [ImportingConstructor] public GitHubPaneViewModel(ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, @@ -110,12 +112,45 @@ protected override void RepoChanged(bool changed) Reload().Forget(); } + /// + /// This method is reentrant, so all await calls need to be done before + /// any actions are performed on the data. More recent calls to this method + /// will cause previous calls pending on await calls to exit early. + /// + /// async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) { + if (!initialized) + return; + + latestReloadCallId++; + var reloadCallId = latestReloadCallId; + navigatingViaArrows = navigating; if (!IsGitHubRepo.HasValue) - IsGitHubRepo = await IsAGitHubRepo(); + { + var isGitHubRepo = await IsAGitHubRepo(); + if (reloadCallId != latestReloadCallId) + return; + + IsGitHubRepo = isGitHubRepo; + } + + var connection = await connectionManager.LookupConnection(ActiveRepo); + if (reloadCallId != latestReloadCallId) + return; + + if (connection == null) + IsLoggedIn = false; + else + { + var isLoggedIn = await connection.IsLoggedIn(hosts); + if (reloadCallId != latestReloadCallId) + return; + + IsLoggedIn = isLoggedIn; + } if (!IsGitHubRepo.Value) { @@ -123,9 +158,6 @@ async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) return; } - var connection = await connectionManager.LookupConnection(ActiveRepo); - IsLoggedIn = await connection.IsLoggedIn(hosts); - if (!IsLoggedIn) { LoadView(UIViewType.LoggedOut); From e0c7781b4aa10fb17d132b473f79529969e1f2c7 Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 21:22:09 +0200 Subject: [PATCH 6/7] Cleanup Reload code --- .../UI/Views/GitHubPaneViewModel.cs | 50 ++++++++++++++----- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 857a81e0dc..ba4b6fef35 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -27,6 +27,8 @@ namespace GitHub.VisualStudio.UI.Views [PartCreationPolicy(CreationPolicy.NonShared)] public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel { + const UIControllerFlow DefaultControllerFlow = UIControllerFlow.PullRequests; + bool initialized; readonly CompositeDisposable disposables = new CompositeDisposable(); IUIController uiController; @@ -53,8 +55,6 @@ public GitHubPaneViewModel(ISimpleApiClientFactory apiFactory, ITeamExplorerServ syncContext = SynchronizationContext.Current; CancelCommand = ReactiveCommand.Create(); Title = "GitHub"; - - hosts.WhenAnyValue(x => x.IsLoggedInToAnyHost).Subscribe(_ => Reload().Forget()); } public override void Initialize(IServiceProvider serviceProvider) @@ -89,6 +89,8 @@ public override void Initialize(IServiceProvider serviceProvider) initialized = true; base.Initialize(serviceProvider); + + hosts.WhenAnyValue(x => x.IsLoggedInToAnyHost).Subscribe(_ => Reload().Forget()); } public void Initialize([AllowNull] ViewWithData data) @@ -155,28 +157,50 @@ async Task Reload([AllowNull] ViewWithData data = null, bool navigating = false) if (!IsGitHubRepo.Value) { //LoadView(UIViewType.NotAGitHubRepo); - return; } - if (!IsLoggedIn) + else if (!IsLoggedIn) { LoadView(UIViewType.LoggedOut); - return; } - if (uiController == null || (data != null && data.ActiveFlow != uiController.SelectedFlow)) - StartFlow(data?.ActiveFlow ?? UIControllerFlow.PullRequests, connection, data); - else if (data != null || currentNavItem >= 0) + else + { + LoadView(data?.ActiveFlow ?? DefaultControllerFlow, connection, data); + } + } + + void LoadView(UIControllerFlow flow, IConnection connection = null, ViewWithData data = null, UIViewType type = UIViewType.None) + { + // if we're loading a single view or a different flow, we need to stop the current controller + var restart = flow == UIControllerFlow.None || uiController?.SelectedFlow != flow; + + if (restart) + Stop(); + + // if there's no selected flow, then just load a view directly + if (flow == UIControllerFlow.None) + { + var factory = ServiceProvider.GetExportedValue(); + var c = factory.CreateViewAndViewModel(type); + c.View.DataContext = c.ViewModel; + Control = c.View; + } + // it's a new flow! + else if (restart) + { + StartFlow(flow, connection, data); + } + // navigate to a requested view within the currently running uiController + else + { uiController.Jump(data ?? navStack[currentNavItem]); + } } void LoadView(UIViewType type) { - Stop(); - var factory = ServiceProvider.GetExportedValue(); - var c = factory.CreateViewAndViewModel(type); - c.View.DataContext = c.ViewModel; - Control = c.View; + LoadView(UIControllerFlow.None, type: type); } void StartFlow(UIControllerFlow controllerFlow, [AllowNull]IConnection conn, ViewWithData data = null) From 2464be720d6fa1a70bcb8070512105e6b9983a9c Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Thu, 16 Jun 2016 21:33:22 +0200 Subject: [PATCH 7/7] Remove unused using --- src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index ba4b6fef35..8707d8cfb7 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -19,7 +19,6 @@ using ReactiveUI; using System.Collections.Generic; using System.Threading.Tasks; -using System.Diagnostics; namespace GitHub.VisualStudio.UI.Views { @@ -366,4 +365,4 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } } -} \ No newline at end of file +}