Repository navigation
Warn before clearing review mode when unsaved comment drafts exist - #9043
Conversation
When a PR is merged or closed, ReviewManager clears review mode, which disposes ReviewCommentController and drops any text only present in the local comment input. Prompt the user before clearing if drafts exist, and remember a "cancel" choice so polling doesn't re-prompt. Fixes microsoft#7556
|
@microsoft-github-policy-service agree |
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
There was a problem hiding this comment.
Thank you for the PR!
Some additional changes requested based on AI code review.
Copilot PRs get a warning even when nothing was typed.
tryAddCopilotMention automatically inserts Copilot into empty comment boxes on Copilot PRs. hasUnsavedCommentDrafts treats that text as a draft, so merging a Copilot PR with any comment box open triggers the warning. Ignore text that is just the auto-inserted mention.
Choosing Cancel skips the rest of the refresh but still records it as done.
The cancel branches at reviewManager.ts:770 and reviewManager.ts:783 return early, so the active PR, the changes view and the layout are never updated. But _isShowingLastReviewChanges (line 752) and _lastCommitSha have already been updated. Whatever triggered this pass (newly pulled commits, or toggling "show changes since review") is lost, because the next pass hits the "no meaningful state change" early return. Once the user chooses to keep review mode, the code should continue the normal flow, the same as with useReviewMode.merged: true.
Keep review mode when a merged or closed PR has real drafts, warn without blocking validation, and let the user exit later. Ignore the auto-inserted Copilot mention and match comment URIs exactly.
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
There was a problem hiding this comment.
I'm trying to repro the original issue so that I can verify the fix:
- Check out a PR
- Write a comment in a file, but don't submit
- Go to GitHub.com and close the PR
- In the active PR view, refresh.
- Review mode is not actually exited, even though I have:
"githubPullRequests.useReviewMode": {
"merged": true,
"closed": false
},
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
There was a problem hiding this comment.
Repro steps:
- Check out the PR, then reload the window (or check out the branch with plain git) so _switchedToPullRequest isn't set.
- Start a file comment and don't submit it.
- Close the PR on GitHub.
- Move the local branch while keeping it in sync with upstream: git commit --allow-empty -m repro, then git push. Pulling a commit someone else pushed also works.
Alex Ross (alexr00)
left a comment
Alex Ross (alexr00)
left a comment
There was a problem hiding this comment.
Thank you for the PR!
|
/AzurePipelines run |
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. |
Fixes #7556
Per Alex Ross (@alexr00)'s guidance (Option 1), warn before clearing when there are
unsaved file-comment drafts.
Testing: