Display Pull Request Reviews for a User. #1545
Conversation
Need to make sure we match the start and end of the string.
We need to be able to read a user model by their login from the API.
This is needed to ensure comment summaries appear without newlines.
Displays a path and intelligently trims with ellipsis when the path doesn't fit in the allocated size.
The default expander style doesn't fit with the VS UI theme - set a new default style which uses a triangle octicon.
When the user clicks on a "Reviewer" item in the PR details view, navigate to a new view to show the PR reviews by that user.
And refactored the `PullRequestUserReviewsViewModel` a little bit to remove stuff that is no longer necessary.
grokys
changed the base branch from
feature/pr-reviews-master
to
feature/pr-details-review-list
Use the same triangle as used elsewhere (e.g. in `SectionControl`).
Fix a few theming issues.
grokys
changed the title
grokys
changed the base branch from
feature/pr-details-review-list
to
feature/pr-reviews-master
grokys
requested review from
StanleyGoldman,
drguthals,
jcansdale,
meaghanlewis and
shana
|
This LGTM 👍 |
jcansdale
changed the base branch from
feature/pr-reviews-master
to
feature/pr-reviews
jcansdale
changed the base branch from
feature/pr-reviews
to
feature/pr-reviews-master
| string RelativePath { get; } | ||
|
|
||
| /// <summary> | ||
| /// Gets a comment which opens the comment in a diff view. |
There was a problem hiding this comment.
Should this be simply, "Opens the comment in a diff view"?
There was a problem hiding this comment.
Ah, should be "Gets a command...". If you look around at C# documentation, properties always start with "Gets a" (for read-only properties) or "Gets or sets a" (for read/write properties). See https://softwareengineering.stackexchange.com/questions/193244/is-the-gets-or-sets-necessary-in-xml-documentation-of-properties and http://stylecop.soyuz5.com/SA1623.html.
There was a problem hiding this comment.
Thanks, I hadn't noticed that convention.
| { | ||
| var text = value as string; | ||
| if (String.IsNullOrEmpty(text)) return null; | ||
| return Regex.Replace(text, @"\t|\n|\r", " "); |
There was a problem hiding this comment.
We we want to replace tabs with a single space as well?
There was a problem hiding this comment.
If we're displaying a comment on a single line, it would make sense to me to convert tabs to spaces, because otherwise we'd be wasting space. Tabs are generally used for alignment between lines, so if we've got no lines it makes sense to strip them. Do you have an example where this wouldn't be a good idea?
There was a problem hiding this comment.
Oh, I was just comparing it to the comment: "trims newlines from a string and replaces them with spaces".
|
@jcansdale addressed your feedback. |
3 participants
When the user clicks on a "Reviewer" item in the PR details view, navigate to a new view to show the PR reviews by that user.
This PR also introduces:
Expander: the default WPF expander style doesn't fit in well with Visual StudioTrimmedPathTextBlockcontrol to intelligently trim paths (see screenshot above)Depends on #1523
Part of #1491