From 91fcd67d33863566aeb5629eb6a012d6f7482ea9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 5 Dec 2018 13:29:51 +0000 Subject: [PATCH 1/2] Don't change button visibility if repo has changed There can be delay before IsAGitHubRepo returns its result. If the repository changes before the call returns, the button can end up with the visibility for the wrong repository. This fix only sets the button visibility if the repository hasn't changed. Button is invalidated multiple times, so one of them returns the visibility for the correct repository. --- .../Base/EnsureLoggedInSection.cs | 2 +- .../Base/TeamExplorerNavigationItemBase.cs | 15 ++++++++++++++- .../Home/ForkNavigationItem.cs | 2 +- .../Home/GitHubHomeSection.cs | 4 ++-- .../Home/IssuesNavigationItem.cs | 2 +- .../Home/WikiNavigationItem.cs | 2 +- .../Base/TeamExplorerItemBase.cs | 14 +++++--------- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs b/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs index d2063710b1..e240fa0cad 100644 --- a/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs +++ b/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs @@ -45,7 +45,7 @@ async Task CheckLogin() if (ActiveRepo == null || ActiveRepoUri == null) return; - var isgithub = await IsAGitHubRepo(); + var isgithub = await IsAGitHubRepo(ActiveRepoUri); if (!isgithub) return; diff --git a/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs b/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs index 384f830af3..1341a0860c 100644 --- a/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs +++ b/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs @@ -3,16 +3,20 @@ using System.Drawing; using GitHub.Api; using GitHub.Extensions; +using GitHub.Logging; using GitHub.Services; using GitHub.UI; using GitHub.VisualStudio.Helpers; using Microsoft.TeamFoundation.Controls; using Microsoft.VisualStudio.PlatformUI; +using Serilog; namespace GitHub.VisualStudio.Base { public class TeamExplorerNavigationItemBase : TeamExplorerItemBase, ITeamExplorerNavigationItem2 { + static readonly ILogger log = LogManager.ForContext(); + readonly Octicon octicon; public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider, @@ -38,7 +42,16 @@ public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider, public override async void Invalidate() { IsVisible = false; - IsVisible = await IsAGitHubRepo(); + + var uri = ActiveRepoUri; + var isVisible = await IsAGitHubRepo(uri); + if (ActiveRepoUri != uri) + { + log.Information("Not setting button visibility because repository changed from {BeforeUrl} to {AfterUrl}", uri, ActiveRepoUri); + return; + } + + IsVisible = isVisible; } void OnThemeChanged() diff --git a/src/GitHub.TeamFoundation.14/Home/ForkNavigationItem.cs b/src/GitHub.TeamFoundation.14/Home/ForkNavigationItem.cs index 12e8424b40..3762c43940 100644 --- a/src/GitHub.TeamFoundation.14/Home/ForkNavigationItem.cs +++ b/src/GitHub.TeamFoundation.14/Home/ForkNavigationItem.cs @@ -97,7 +97,7 @@ public override async void Invalidate() { IsVisible = false; - if ((packageSettings?.ForkButton ?? false) && await IsAGitHubDotComRepo()) + if ((packageSettings?.ForkButton ?? false) && await IsAGitHubDotComRepo(ActiveRepoUri)) { var connection = await ConnectionManager.GetConnection(ActiveRepo); IsVisible = connection?.IsLoggedIn ?? false; diff --git a/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs b/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs index ecb393b055..5d276cc3b4 100644 --- a/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs +++ b/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs @@ -64,7 +64,7 @@ protected async override void RepoChanged() base.RepoChanged(); - IsVisible = await IsAGitHubRepo(); + IsVisible = await IsAGitHubRepo(ActiveRepoUri); if (IsVisible) { @@ -93,7 +93,7 @@ protected async override void RepoChanged() public override async void Refresh() { - IsVisible = await IsAGitHubRepo(); + IsVisible = await IsAGitHubRepo(ActiveRepoUri); if (IsVisible) { IsLoggedIn = await IsUserAuthenticated(); diff --git a/src/GitHub.TeamFoundation.14/Home/IssuesNavigationItem.cs b/src/GitHub.TeamFoundation.14/Home/IssuesNavigationItem.cs index 0ca6e99b98..54e2a4cdc5 100644 --- a/src/GitHub.TeamFoundation.14/Home/IssuesNavigationItem.cs +++ b/src/GitHub.TeamFoundation.14/Home/IssuesNavigationItem.cs @@ -40,7 +40,7 @@ public override async void Invalidate() { IsVisible = false; - var visible = await IsAGitHubRepo(); + var visible = await IsAGitHubRepo(ActiveRepoUri); if (visible) { var repo = await SimpleApiClient.GetRepository(); diff --git a/src/GitHub.TeamFoundation.14/Home/WikiNavigationItem.cs b/src/GitHub.TeamFoundation.14/Home/WikiNavigationItem.cs index ddac4fa924..26fb5ab3dc 100644 --- a/src/GitHub.TeamFoundation.14/Home/WikiNavigationItem.cs +++ b/src/GitHub.TeamFoundation.14/Home/WikiNavigationItem.cs @@ -38,7 +38,7 @@ public override void Execute() public override async void Invalidate() { - var visible = await IsAGitHubRepo(); + var visible = await IsAGitHubRepo(ActiveRepoUri); if (visible) { var repo = await SimpleApiClient.GetRepository(); diff --git a/src/GitHub.VisualStudio.UI/Base/TeamExplorerItemBase.cs b/src/GitHub.VisualStudio.UI/Base/TeamExplorerItemBase.cs index acab84eab8..ece0c9cc85 100644 --- a/src/GitHub.VisualStudio.UI/Base/TeamExplorerItemBase.cs +++ b/src/GitHub.VisualStudio.UI/Base/TeamExplorerItemBase.cs @@ -123,12 +123,8 @@ protected virtual void RepoChanged() } } - protected async Task GetRepositoryOrigin() + protected async Task GetRepositoryOrigin(UriString uri) { - if (ActiveRepo == null) - return RepositoryOrigin.NonGitRepository; - - var uri = ActiveRepoUri; if (uri == null) return RepositoryOrigin.Other; @@ -154,15 +150,15 @@ protected async Task GetRepositoryOrigin() return RepositoryOrigin.Other; } - protected async Task IsAGitHubRepo() + protected async Task IsAGitHubRepo(UriString uri) { - var origin = await GetRepositoryOrigin(); + var origin = await GetRepositoryOrigin(uri); return origin == RepositoryOrigin.DotCom || origin == RepositoryOrigin.Enterprise; } - protected async Task IsAGitHubDotComRepo() + protected async Task IsAGitHubDotComRepo(UriString uri) { - var origin = await GetRepositoryOrigin(); + var origin = await GetRepositoryOrigin(uri); return origin == RepositoryOrigin.DotCom; } From f969094913a135580a08c8eeebe6ece9b650fb8c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 5 Dec 2018 14:38:40 +0000 Subject: [PATCH 2/2] Avoid using async void for Invalidate This would have crashed Visual Studio if Invalidate had thrown. Moved async into a separate InvalidateAsync method. --- .../Base/TeamExplorerNavigationItemBase.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs b/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs index 1341a0860c..e4acc09206 100644 --- a/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs +++ b/src/GitHub.TeamFoundation.14/Base/TeamExplorerNavigationItemBase.cs @@ -1,6 +1,7 @@ using System; using System.Diagnostics; using System.Drawing; +using System.Threading.Tasks; using GitHub.Api; using GitHub.Extensions; using GitHub.Logging; @@ -39,10 +40,14 @@ public TeamExplorerNavigationItemBase(IGitHubServiceProvider serviceProvider, SubscribeToRepoChanges(); } - public override async void Invalidate() + public override void Invalidate() { IsVisible = false; + InvalidateAsync().Forget(log); + } + async Task InvalidateAsync() + { var uri = ActiveRepoUri; var isVisible = await IsAGitHubRepo(uri); if (ActiveRepoUri != uri)