Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Dec 5, 2018

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.

What this PR does

  • Don't change button visibility if repo changed after async call was made

What this PR doesn't do

This is probably only one of a number of issues like this. This is a point fix for the pull requests button disappearing (this is likely to have the most impact). Unfortunately the Team Explorer integration code has become fix upon fix and is difficult to work with and test.

Ideally we would create a view-model that controls visibility of all repository related buttons and UI inside Team Explorer. This would be a large (but worthwhile) refactoring, but is out of scope for this fix.

How to repro

  1. Open a Git repository that isn't on GitHub (e.g. one from Azure DevOps)
  2. Open a Git repository that is on GitHub
  3. Expect the pull request button to appear and then disappear

This is what it ends up looking like:

image

How to test

  1. Open a Git repository that isn't on GitHub (e.g. one from Azure DevOps)
  2. Open a Git repository that is on GitHub
  3. Expect the pull request button to appear and remain!

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.
This would have crashed Visual Studio if Invalidate had thrown. Moved
async into a separate InvalidateAsync method.
Copy link
Contributor

@StanleyGoldman StanleyGoldman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what is happening to fall into this trap?

@meaghanlewis meaghanlewis added this to the 2.6.0 milestone Dec 6, 2018
…explorer-button-visibility

# Conflicts:
#	src/GitHub.TeamFoundation.14/Home/ForkNavigationItem.cs
@StanleyGoldman StanleyGoldman merged commit d5780f2 into master Dec 6, 2018
@StanleyGoldman StanleyGoldman deleted the fixes/team-explorer-button-visibility branch December 6, 2018 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants