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

[WIP] Adding more detail to PullRequestCheckView - #2013

Open
StanleyGoldman wants to merge 38 commits into
masterfrom
check-second-line
Open

[WIP] Adding more detail to PullRequestCheckView#2013
StanleyGoldman wants to merge 38 commits into
masterfrom
check-second-line

Conversation

@StanleyGoldman

@StanleyGoldmanStanleyGoldman commented Oct 25, 2018

Copy link
Copy Markdown
Contributor

Fixes#1967

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

image

  • Use a DataGrid instead of a list control

@donokudadonokuda self-assigned this Oct 25, 2018
@donokuda

Copy link
Copy Markdown
Contributor

@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

@donokudadonokuda removed their assignment Oct 25, 2018
@codecov

codecovBot commented Oct 25, 2018

Copy link
Copy Markdown

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 FilesCoverage Δ
...ViewModels/GitHubPane/PullRequestCheckViewModel.cs1.01% <0%> (ø)
...nlineReviews/Services/PullRequestSessionService.cs17.03% <0%> (ø)
src/GitHub.Exports/Models/CheckRunModel.cs0% <0%> (ø)
.../GitHub.UI/Converters/DurationToStringConverter.cs25% <100%> (ø)
...rc/GitHub.Exports/Extensions/TimeSpanExtensions.cs89.18% <89.18%> (ø)

@StanleyGoldman

Copy link
Copy Markdown
ContributorAuthor

It's looking good. I tweaked it so the "Finished - 4s" part appears as a tooltip instead.

@StanleyGoldman

Copy link
Copy Markdown
ContributorAuthor

image

@StanleyGoldmanStanleyGoldman changed the title [WIP] Adding a second line to Checks in the PullRequestCheckViewAdding a second line to Checks in the PullRequestCheckViewOct 30, 2018
@StanleyGoldmanStanleyGoldman changed the title Adding a second line to Checks in the PullRequestCheckView[WIP] Adding more detail to PullRequestCheckViewDec 13, 2018
@donokuda

Copy link
Copy Markdown
Contributor

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

@donokudadonokuda removed their assignment Dec 14, 2018
# 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
@StanleyGoldmanStanleyGoldman changed the title [WIP] Adding more detail to PullRequestCheckViewAdding more detail to PullRequestCheckViewFeb 26, 2019
@StanleyGoldman

Copy link
Copy Markdown
ContributorAuthor

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

image

@donokuda

Copy link
Copy Markdown
Contributor

I'm taking a look at this right now and will push a fix 👀

@donokuda

Copy link
Copy Markdown
Contributor

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

@donokudadonokuda removed their assignment Feb 28, 2019
@StanleyGoldmanStanleyGoldman changed the title Adding more detail to PullRequestCheckView[WIP] Adding more detail to PullRequestCheckViewMar 14, 2019
Sign up for freeto 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.

Checks need more information

3 participants

@StanleyGoldman@donokuda@meaghanlewis