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

Lightweight fix for PR detail view not refreshing #1359

Merged
merged 3 commits into from
Dec 4, 2017

Conversation

jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Dec 1, 2017

This PR listens to the IVSGitExt service for ActiveRepositoriesChanged events and refreshes the model.

I considered extending ITeamExplorerServiceHolder, but this code is already quite confusing and I didn't want to complicate it further.

At the moment there is no obvious way to dispose of view models. As a workaround, this code assumes there is only one active PullRequestDetailViewModel object and automatically disposes of any previous instances.

I spoke with @grokys about how this might be handled in future. A future implementation of the GitHub pane might dispose of view models, making the AutoDispose workaround redundant.

Fixes #1345.

This commit will automatically dispose of any preview PullRequestDetailViewModel object.
Keep CA happy by making AutoDispose static.
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

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

This code assumes there is only one active PullRequestDetailViewModel object and automatically disposes of any previous instances.

I'm afraid that's not a valid assumption, try this:

  1. Open a PR (let's call this PR1)
  2. Check out PR1's branch
  3. Click the PR List icon in the toolbar to go back to the list
  4. Open another PR (call it PR2)
  5. Check out PR2's branch
  6. Click back twice to go back to PR1
  7. Check out PR1's branch again
  8. See that it wasn't refreshed

I say for the moment, forget about removing the event subscription and I'll create a PR on top of #1346 that fixes this.

@jcansdale jcansdale changed the title Light weight fix for PR detail view not refreshing Lightweight fix for PR detail view not refreshing Dec 4, 2017
PullRequestDetailViewModel will be disposed in future commit.
@jcansdale
Copy link
Collaborator Author

@grokys removed the auto-dispose.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants