-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: restore X button during save/reject prompts (#6865) #6866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Added a stop/close button (X icon) that remains visible when approval buttons are shown - Button allows users to terminate tasks during save/reject phase as requested - Fixes issue where users could not stop tasks when prompted to save/reject changes Fixes #6865
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code because apparently I trust no one, not even myself.
| </StandardTooltip> | ||
| )} | ||
| {(secondaryButtonText || isStreaming) && ( | ||
| {(secondaryButtonText || isStreaming) && !isStreaming && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this condition intentional? The logic checks for (secondaryButtonText || isStreaming) but then immediately requires !isStreaming, which effectively simplifies to just secondaryButtonText && !isStreaming. Could we simplify this for clarity?
| <StandardTooltip content={t("chat:cancel.tooltip")}> | ||
| <VSCodeButton | ||
| appearance="secondary" | ||
| disabled={!(isStreaming && !didClickCancel)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The outer condition already ensures isStreaming is true, so this inner check isStreaming && !didClickCancel could be simplified to just !didClickCancel. Is there a reason for the redundant check?
| <VSCodeButton | ||
| appearance="icon" | ||
| disabled={false} | ||
| className="ml-2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor styling inconsistency: This button uses className="ml-2" while other buttons in this section use ml-[6px]. Consider using consistent spacing for visual harmony.
| disabled={false} | ||
| className="ml-2" | ||
| onClick={() => startNewTask()}> | ||
| <span className="codicon codicon-x"></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding an aria-label attribute for better accessibility. Screen readers would benefit from a descriptive label like aria-label="Close task" since the button only contains an icon.
Description
This PR fixes issue #6865 where the X button (stop/close task button) was missing when the UI was prompting users to Save/Reject changes.
Problem
Users reported that they could not stop or terminate tasks when prompted with Save/Reject buttons. The X button was only visible during streaming operations but disappeared during approval prompts, preventing users from canceling tasks when:
Solution
Added a dedicated stop/close button (X icon) that remains visible when approval buttons are shown. The button:
startNewTask()function to properly clean up the taskChanges Made
ChatView.tsxto add a stop/close button whenenableButtonsis true andprimaryButtonTextexistsTesting
Screenshots
The X button now appears during save/reject prompts, allowing users to stop the task:
Fixes #6865
Important
Adds a stop/close button in
ChatView.tsxduring save/reject prompts to allow task termination, addressing issue #6865.ChatView.tsxduring save/reject prompts whenenableButtonsis true andprimaryButtonTextexists.startNewTask()function.codicon-xfor consistency with VS Code UI.This description was created by
for eed1f15. You can customize this summary. It will automatically update as commits are pushed.