Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MVVM-ize things (attempt 2) #1367

Merged
merged 29 commits into from Dec 5, 2017
Merged

MVVM-ize things (attempt 2) #1367

merged 29 commits into from Dec 5, 2017

Conversation

@grokys
Copy link
Contributor

@grokys grokys commented Dec 5, 2017

#1346 was targeting the wrong branch (it was targeting #1353) when I pushed. This PR is exactly the same as #1346 except targeting master.

grokys added 28 commits Nov 28, 2017
The MVVM refactor (#1346) was originally based on the SSO feature branch and had a _lot_ of WIP etc commits. Rebased it off of #1353 and squashed into a single commit.
...instead of `IGitHubServiceProvider` to create view models in `GitHubDialogWindowViewModel`. Also added some doc comments.
Fixes #1355.
And use it instead of defining `LoginView` and `NavigationView` separately.
MEF requires an array here.
When the page to be navigated to in the GitHub pane is the current page, do nothing.
 Conflicts:
	src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs
`IVSGitExt.ActiveRepositoriesChanged` isn't notified on the UI thread which breaks things. Switch to the main thread when refreshing due to a notification from this.
@grokys grokys requested review from sguthals and jcansdale Dec 5, 2017
Copy link
Collaborator

@jcansdale jcansdale left a comment

LGTM!

@jcansdale jcansdale merged commit 2353ab1 into master Dec 5, 2017
3 checks passed
3 checks passed
GitHub CLA @grokys has accepted the GitHub Contributor License Agreement.
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@jcansdale jcansdale deleted the refactor/mvvm branch Dec 5, 2017
grokys added a commit that referenced this pull request Dec 12, 2017
#1367 accidentially removed the code to refresh the PR list when switching between fork and parent PRs. Also update the `WebUrl` when this happens to fix #1218.

Fixes #1380
Fixes #1218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.