Skip to content

/ VisualStudio

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

Jump to bottom

[WIP] Adding more detail to PullRequestCheckView #2013

Open
wants to merge 38 commits into
base: master
from
Open

[WIP] Adding more detail to PullRequestCheckView #2013

wants to merge 38 commits into from
+524 −118

Conversation

@StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Oct 25, 2018 •
edited

Fixes #1967

  • Adds a second line with more detail
  • Adds a required flag

image

  • Use a DataGrid instead of a list control
@StanleyGoldman
Adding a second line to Checks in the PullRequestCheckView
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
25952ef
@donokuda donokuda self-assigned this Oct 25, 2018
donokuda added 2 commits Oct 25, 2018
@donokuda
Update checks list layout
6c2e1c8
@donokuda
Little bit of spacing and some text color formatting
Loading status checks…
2a8b50f
@donokuda
Copy link
Member

@donokuda donokuda commented Oct 25, 2018

@StanleyGoldman I pushed a couple of commits cleaning this up. This should be ready for review now but I'm happy to incorporate design changes that people might have:

Light theme
screen shot 2018-10-25 at 2 21 24 pm

Dark theme
screen shot 2018-10-25 at 2 21 06 pm

Blue Theme
screen shot 2018-10-25 at 2 20 44 pm
@donokuda donokuda removed their assignment Oct 25, 2018
@codecov
Copy link

@codecov codecov bot commented Oct 25, 2018 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@ce9e5c4). Click here to learn what that means.
The diff coverage is 65.38%.

@@            Coverage Diff            @@
##             master    #2013   +/-   ##
=========================================
  Coverage          ?   39.34%           
=========================================
  Files             ?      411           
  Lines             ?    17619           
  Branches          ?     2440           
=========================================
  Hits              ?     6933           
  Misses            ?    10125           
  Partials          ?      561
Impacted Files Coverage Δ
...ViewModels/GitHubPane/PullRequestCheckViewModel.cs 1.01% <0%> (ø)
...nlineReviews/Services/PullRequestSessionService.cs 17.03% <0%> (ø)
src/GitHub.Exports/Models/CheckRunModel.cs 0% <0%> (ø)
.../GitHub.UI/Converters/DurationToStringConverter.cs 25% <100%> (ø)
...rc/GitHub.Exports/Extensions/TimeSpanExtensions.cs 89.18% <89.18%> (ø)
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7a288d1
@StanleyGoldman
Making duration status a tooltip
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
632c9fb
@StanleyGoldman
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman commented Oct 30, 2018

It's looking good. I tweaked it so the "Finished - 4s" part appears as a tooltip instead.
@StanleyGoldman
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman commented Oct 30, 2018

image
@StanleyGoldman StanleyGoldman changed the title [WIP] Adding a second line to Checks in the PullRequestCheckView Adding a second line to Checks in the PullRequestCheckView Oct 30, 2018
@StanleyGoldman
Allow functionality to set ProtectedBranches
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
406c5d4
@StanleyGoldman
Merge branch 'octokit-graphql-update' into protected-branches
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
8518c42
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7e185c3
@StanleyGoldman
GraphQL fix
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
0b979b3
@StanleyGoldman
Adding a second check for differing non-required statuses
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
e17b44b
@StanleyGoldman
Adding a required tag to PullRequestCheckViewModel
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
bc92842
@StanleyGoldman
Fix compilation error
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
5d0c825
@StanleyGoldman
Merge branch 'master' into protected-branches
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
73134d9
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c816c8d
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a7e3fcb
@StanleyGoldman
Merge remote-tracking branch 'origin/master' into check-second-line
Loading status checks…
1d0b8d2 
# Conflicts:
#	src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs
#	src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestCheckViewModel.cs
#	src/GitHub.Exports/Models/CheckRunModel.cs
#	src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
#	src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml
@StanleyGoldman
Fixes needed for compilation
Loading status checks…
6e03523
@StanleyGoldman
Fixing formatting
Loading status checks…
51a00c7
@StanleyGoldman
Merge remote-tracking branch 'origin/master' into check-second-line
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
cefc43a
@StanleyGoldman
Fxing how the model is populated
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
a489cbe
@StanleyGoldman
Repositioning
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
1cf3c80
@StanleyGoldman
Cleanup
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
018fdab
@StanleyGoldman
Merge pull request #2029 from github/protected-branches
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
bda657a 
Adding indications for protected branches
@StanleyGoldman StanleyGoldman changed the title Adding a second line to Checks in the PullRequestCheckView [WIP] Adding more detail to PullRequestCheckView Dec 13, 2018
StanleyGoldman and others added 7 commits Dec 13, 2018
@StanleyGoldman
Comments
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
95bb822
@StanleyGoldman
Removing unused enum
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
b7d9d2e
@StanleyGoldman
More comments
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
20690ea
@donokuda
Flailing a little
a3cb52f
@donokuda
Use a dock panel layout
1a46be8
@donokuda
Theming
2a9402e
@donokuda
uncomment visibility condition
Loading status checks…
6691ab7
@donokuda
Copy link
Member

@donokuda donokuda commented Dec 14, 2018

I went ahead and polished this up a bit:

screen shot 2018-12-13 at 4 53 08 pm

screen shot 2018-12-13 at 4 52 45 pm

I forced the required badge to show in order to test my styles and reverted that change in 6691ab7
@donokuda donokuda removed their assignment Dec 14, 2018
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
fbf9bef 
# Conflicts:
#	src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
#	src/GitHub.Resources/Resources.zh-CN.resx
#	src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml
#	src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml
#	src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml
@StanleyGoldman
Merge branch 'master' into check-second-line
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
6fb41a2
@StanleyGoldman StanleyGoldman changed the title [WIP] Adding more detail to PullRequestCheckView Adding more detail to PullRequestCheckView Feb 26, 2019
@StanleyGoldman
Fixing view
Verified
This commit was signed with a verified signature.
StanleyGoldman Stanley Goldman
GPG key ID: 7728F3B9BD13C8C3 Learn about signing commits
Loading status checks…
fa4add2
@meaghanlewis meaghanlewis added this to the 2.9.0 milestone Feb 26, 2019
@StanleyGoldman
Copy link
Contributor Author

@StanleyGoldman StanleyGoldman commented Feb 27, 2019

Changes in master seem to have reduced the design from it's previous glory...

image
@donokuda
Copy link
Member

@donokuda donokuda commented Feb 27, 2019

I'm taking a look at this right now and will push a fix 👀
donokuda added 2 commits Feb 27, 2019
@donokuda
Fix line spacing issues between check titles and descriptions
f3b746c
@donokuda
Leave enough space for the Required Badge
Loading status checks…
8bec5f0
@donokuda
Copy link
Member

@donokuda donokuda commented Feb 27, 2019

I pushed a couple of commits that fix the line spacing:

(With the required badge)
checks required

(No required badge)
checks

I'm running into a layout "gotcha" where if the panel is too small, some details gets pushed off the screen:
screen shot 2019-02-27 at 1 42 29 pm

I have ran into this a number of times throughout the extension, and I haven't figured out a good way to solve this especially in a rendered list. Open to any ideas that might help.

Other than that, I think this is ready for review again
@donokuda donokuda removed their assignment Feb 28, 2019
@StanleyGoldman StanleyGoldman changed the title Adding more detail to PullRequestCheckView [WIP] Adding more detail to PullRequestCheckView Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.9.0
Linked issues

Successfully merging this pull request may close these issues.

Checks need more information
3 participants
@StanleyGoldman @donokuda @meaghanlewis
You can’t perform that action at this time.