This repository was archived by the owner on Jun 21, 2023. It is now read-only.
Add lifecycle management for GitHub Pane pages #1366
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove `NavigationViewModel.RegisterDispose` and call `Dispose` on pages when they're removed from the history - Add `Activated` and `Deactivated` methods to `IPanePageViewModel` which are called when the page is moved to/away from in the navigator
Rather than making derved classes implement it if necessary. This is to prevent them maybe forgetting to dispose subscriptions to it.
When a repository change is detected by a non-active `PullRequestDetailViewModel`, set a flag to refresh the view model when it becomes active again.
Conflicts: src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs
Previously we were only clearing it when it changes to a new valid repository. Instead clear it if the TE context is cleared for example. We need to check that the repository is actually different here because `TeamExplorerRepositoryHolder` sometimes notifies a change when the repository has not changed.
Don't try to update the PR list filter when the collection has been disposed.
We only want to check whether the repository has changed when we're getting the notification from `TeamExplorerRepositoryHolder` (as it sometimes signals a change when there is none) - in the case of a connection changing we _should_ update the content when the repository hasn't changed.
Contributor
|
LGTM. Tested this using VS2017 using the github/github repo. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes
IPanePageViewModels get disposed when they are removed from aNavigationViewModel's history. It also addsActivatedandDeactivatedmethods toIPanePageViewModelwhich are called when the page is moved to/away from in the navigator.NavigationViewModel.RegisterDisposehas been removed as disposing of the view models should suffice.It modifies
PullRequestDetailViewModelto only refresh when the page is active. When not active, it simply sets a flag to refresh when the page is activated. This should prevent all pages in the GitHub pane history refreshing at once, which is an expensive operation.Depends on #1346
Fixes #1360