This repository was archived by the owner on Jun 21, 2023. It is now read-only.
Highlight checked out PR in list. #1106
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
Contributor
|
The GitHub pane is throwing this error when I try to test this branch out: |
grokys
changed the title
Contributor
Author
|
@donokuda I think this is one of those things that needs to be fixed by cleaning the solution and resetting the experimental instance. However, we can do that later - we've punted this PR to the next release due to the caveats listed. |
9 tasks
Conflicts: src/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModel.cs src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml src/GitHub.VisualStudio/Views/GitHubPane/PullRequestListItem.xaml test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModelTests.cs
Contributor
Author
|
@jcansdale @donokuda this should be ready for review again now, bearing in mind the gotcha in the "Not yet implemented" section of the PR description. |
grokys
changed the title
Contributor
Contributor
Author
|
Good point @shana - updated. |
StanleyGoldman
approved these changes
Feb 21, 2018
Contributor
StanleyGoldman
left a comment
StanleyGoldman
left a comment
There was a problem hiding this comment.
I tested this and it works well from what I see.
the current PR may not be highlighted until you open its details
I didn't find that comment to be true. The pull request list view updates the highlighted value automatically.
| (s, r) => new { Session = s, Repository = r }) | ||
| .Subscribe(x => | ||
| { | ||
| CheckedOutPullRequest = x.Session?.RepositoryOwner == x.Repository?.Owner ? |
Contributor
There was a problem hiding this comment.
Why are you only checking the repository owner here?
jcansdale
approved these changes
Mar 5, 2018
Collaborator
jcansdale
left a comment
jcansdale
left a comment
There was a problem hiding this comment.
I've just merged the latest commits and checked that this is working. I haven't noticed the current PR not being highlighted until its details are opened. It was highlighted fine when I first viewed the PR list.
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 highlights the currently checked out PR in the PR list.
Blue:
Dark:
Light:
Not yet implemented
To link a branch to a PR we use a property set in the git
.configfile - currently this is only set when a branch is a) checked out in the PR details view b) the PR details are opened for the current branch's PR.We can actually do better here: when we load the PR list we have enough information to mark all PR branches with their related PR but there are a couple of things preventing us from doing this:
TrackingCollection.OriginalCompletedis fired in the PR list when the list starts loading for the first time, not when the load completes. This works correctly on subsequent refreshes - appears to be a bug inTrackingCollectionTrackingCollection.originalwould need to be exposed so we can get an unfiltered list of all PRsNavigating to the PR list causes a reload of all PRs (Navigating to the PR list causes a reload of all PRs #1105) which means we'd be doing this way more often than we should; as the operation involves opening the repository and reading/writing config values this may not be a good idea. This appears to be caused by the fact that in the navigation controller there is no distinction between loading a page for the first time and moving to an already loaded page.- now fixedI think this should probably be implemented in another PR, so bear in mind that the current PR may not be highlighted until you open its details.