This repository was archived by the owner on Jun 21, 2023. It is now read-only.
Validate PR review state before allowing submission. #1582
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
Split the `PullRequestReviewAuthoringViewModel.Submit` command into 2 separate commands and add a `CanExecute` observable for them: - `Approve` can always be submitted - `Comment` needs either a review body or >0 review comments - `RequestChanges` always needs a review body
This was referenced Apr 4, 2018
Contributor
|
This LGTM - except I thought that |
Contributor
Author
|
@meaghanlewis oh, yes you're right! Fixed that. |
grokys
requested review from
StanleyGoldman,
drguthals,
jcansdale and
meaghanlewis
Sigh. Seems that hyperlinks don't respect dependency property precedence, so the `IsEnabled=False` trigger for `Hyperlink.Foreground` doesn't override the foreground `TemplateBinding`. Not sure what we can do here other than set the control's opacity when disabled, so that's what we'll do for now.
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
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.
Split the
PullRequestReviewAuthoringViewModel.Submitcommand into 2 separate commands and add aCanExecuteobservable for them:Approvecan always be submittedCommentneeds either a review body or >0 review commentsRequestChangesneeds either a review body or >0 review commentsHACK
Disabling the
GitHubActionLinks that are used for the approve/comment/request changes buttons had no visual effect. Seems that hyperlinks don't respect dependency property precedence, so theIsEnabled=Falsetrigger forHyperlink.Foregrounddoesn't override the foregroundTemplateBinding. Not sure what we can do here other than set the control's opacity when disabled, so that's what I did for now.Depends on #1581
Part of #1491