Vscode-pull-request-github: Disable option to Start Review when no comment text

Created on 26 Mar 2019  路  10Comments  路  Source: microsoft/vscode-pull-request-github

  • Extension version: 1.79.0
  • VSCode Version: 1.33.0
  • OS: macOS

Steps to Reproduce:

  1. Go to leave a comment on a line
  2. Observe that the comment box has the Start Review option enabled by default.
    Screen Shot 2019-03-26 at 2 17 51 PM

Solution:
This option should be disabled until some text is actually entered in the comment box.

Discovered when testing for: https://github.com/Microsoft/vscode/issues/71103

bug

All 10 comments

Side note: This also seems to confuse things on GitHub.com where it shows a review has been started but there are 0 pending comments.

Screen Shot 2019-03-26 at 2 20 55 PM

I think fixing this might involve tweaking the API. Right now, buttons are added to comments by specifying an acceptInputCommand and an optional list of additionalAcceptInputCommands. Based on the names of those properties, it would make sense that they always be disabled when there is no input. The acceptInputCommand for us is always "Add Comment", the "Start Review", "Finish Review", and "Delete Review" buttons are all from additionalAcceptInputCommands. The "Finish Review" and "Delete Review" buttons we add can be run without input - running "Delete Review" with input would be very weird, but this button is also a bit odd in its current placement.

We could

  • disable all additionalAcceptInputCommands buttons when there is no text, removing the delete review button from the list
  • change the additionalAcceptInputCommands property to have a flag indicating if it can be run when there is no input

@RMacfarlane thanks for the additional context here, I appreciate it.

Just to clarify the proposed solution - it will disable the buttons "Start Review, "Finish Review" and "Delete Review" but then add a flag to indicate if the command can be run? So only "Start Review" will have a flag that indicates input is required before running the command?

Yeah, that's right.

So instead of having the API be additionalAcceptInputCommands: Command[];, it would be
```ts
additionalAcceptInputCommands: [{
command: Command;
disableWhenEmpty: boolean;
}]
````

Right now we call them additionalCommands and their states are not determined by the textarea, which is designed this way. However if we call them *AcceptInputCommands, then they should behave the same as acceptInputCommand, which is disabled if the textarea is empty.

Side note: This also seems to confuse things on GitHub.com where it shows a review has been started but there are 0 pending comments

I'd love to start a review without creating comments first actually. Right now .com doesn't allow that but the api does.

Yeah that's a great point @rebornix. The API does allow it, but I had never noticed that until testing out the new comment provider and it seemed strange to me.

Are you suggesting we leave this implementation as-is?

It sounds like a good feature to me but if you think we'd better avoid that, we can throw an error or show a notification when users try to create a review without comments. This way we don't need to wait for any vscode side code change, which is already late at this point.

I'm okay with just showing some notification that says review comments must be added before starting a review.

This was fixed as part of adopting the new comments API (https://github.com/microsoft/vscode-pull-request-github/pull/1168)

Was this page helpful?
0 / 5 - 0 ratings