Skip to content

Fix review button disable logic with special handling for approve button#7507

Merged
alexr00 merged 8 commits intomainfrom
copilot/fix-7476
Aug 7, 2025
Merged

Fix review button disable logic with special handling for approve button#7507
alexr00 merged 8 commits intomainfrom
copilot/fix-7476

Conversation

Copy link
Contributor

Copilot AI commented Aug 4, 2025 •
edited
Loading

This PR fixes an issue where clicking review buttons (Approve/Comment/Request Changes) without entering any text in the comment box and with no pending review comments would cause a GraphQL error and display duplicate entry boxes.

Problem

When users clicked review buttons with empty content, the extension would:

  1. Create a new review
  2. Attempt to submit it
  3. Receive a GraphQL error because there were no review comments and no summary comment
  4. Display duplicate comment entry boxes in a broken state

Duplicate entry boxes issue

Solution

The fix implements differentiated disable logic for review buttons:

  • Approve button: Always enabled (except when system is busy) - allows approval without additional comments
  • Request Changes/Comment buttons: Disabled when both conditions are met:
    • The summary comment box is empty (or contains only whitespace) AND
    • There are no pending review comments (hasReviewDraft is false)

Changes Made

Updated all review submission components to apply the new disable logic:

AddReviewSummaryComment (timeline.tsx)

  • Added state tracking for the textarea content (commentText)
  • Approve button: Only disabled when isBusy || pr.busy
  • Request Changes/Submit Review buttons: Disabled when !commentText.trim() && !pr.hasReviewDraft
  • Made the textarea a controlled component with proper change handling

AddComment & AddCommentSimple (comment.tsx)

  • Added hasReviewDraft to component props
  • Implemented conditional disable logic based on current selection in ContextDropdown
  • When Approve is selected: Dropdown only disabled when system is busy
  • When other actions are selected: Dropdown disabled when !pendingCommentText?.trim() && !hasReviewDraft

Behavior

  • ✅ Approve button is always enabled (except when system busy) - allows approval without comments
  • ✅ Request Changes/Comment buttons enabled when user types text in comment field
  • ✅ Request Changes/Comment buttons enabled when there are pending review comments (even with empty comment)
  • ✅ Request Changes/Comment buttons disabled only when comment is empty AND no review comments exist
  • ✅ Whitespace-only comments are treated as empty
  • ✅ Consistent behavior across all review interfaces (timeline, overview, and comment components)

This prevents the GraphQL error and eliminates the duplicate entry box issue while maintaining GitHub's standard behavior where approvals don't require additional commentary.

Fixes #7476.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@alexr00
Fix review button disable logic - prevent empty review submission
24fffa0 
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copilot AI changed the title [WIP] clicked comment with no contents gave weird state Fix review button disable logic to prevent empty review submission Aug 4, 2025
Copilot AI requested a review from alexr00 August 4, 2025 16:23
alexr00
alexr00 requested changes Aug 5, 2025
View reviewed changes
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot The changes also need to be made in AddComment in comment.tsx

@alexr00
Apply same disable logic to AddComment components in comment.tsx
1a4045c 
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 5, 2025 •
edited
Loading

@copilot The changes also need to be made in AddComment in comment.tsx

Applied the same disable logic to both AddComment and AddCommentSimple components in comment.tsx. The buttons will now be disabled when the comment text is empty AND there are no pending review comments, matching the behavior in timeline.tsx. (1a4045c)

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • update.code.visualstudio.com
    • Triggering command: node ./out/src/test/runTests.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix review button disable logic to prevent empty review submission Fix review button disable logic across all comment components to prevent empty review submission Aug 5, 2025
Copilot AI requested a review from alexr00 August 5, 2025 09:26
alexr00
alexr00 requested changes Aug 5, 2025
View reviewed changes
Copy link
Member

@alexr00 alexr00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Actually, the approve button is allowed even when there are no comments and no pending review.

@alexr00
Allow approve button even with empty content and no pending review
53fdb43 
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 5, 2025

@copilot Actually, the approve button is allowed even when there are no comments and no pending review.

Fixed! The approve button is now allowed even when there are no comments and no pending review. It's only disabled when the system is busy, while other review buttons (Request Changes, Comment/Submit Review) still require either content or existing review draft. (53fdb43)

Copilot AI changed the title Fix review button disable logic across all comment components to prevent empty review submission Fix review button disable logic with special handling for approve button Aug 5, 2025
Copilot AI requested a review from alexr00 August 5, 2025 10:28
@alexr00 alexr00 marked this pull request as ready for review August 6, 2025 10:40
@alexr00 alexr00 enabled auto-merge (squash) August 6, 2025 10:41
@vs-code-engineering vs-code-engineering bot added this to the August 2025 milestone Aug 6, 2025
dbaeumer
dbaeumer previously approved these changes Aug 6, 2025
View reviewed changes
@alexr00 alexr00 merged commit d2e04d2 into main Aug 7, 2025
3 checks passed
@alexr00 alexr00 deleted the copilot/fix-7476 branch August 7, 2025 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benibenj benibenj benibenj approved these changes

@dbaeumer dbaeumer dbaeumer left review comments

+1 more reviewer

@alexr00 alexr00 alexr00 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alexr00 alexr00

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

August 2025

Development

Successfully merging this pull request may close these issues.

clicked comment with no contents gave weird state

4 participants

@dbaeumer @alexr00 @benibenj