Skip to content

Commit 4ac3a9a

Browse files
TyriarTyriar
authored
Merge pull request microsoft#261324 from microsoft/tyriar/257468_2
Move confirmation actions to bottom and tool bar to top
2 parents 98b8d4a + 634b6dc commit 4ac3a9a

File tree

5 files changed

+51
-35
lines changed

5 files changed

+51
-35
lines changed

‎src/vs/workbench/contrib/chat/browser/chatContentParts/chatConfirmationWidget.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,13 @@ abstract class BaseChatConfirmationWidget extends Disposable {
351351
const elements = dom.h('.chat-confirmation-widget2@root', [
352352
dom.h('.chat-confirmation-widget-title', [
353353
dom.h('.chat-title@title'),
354-
dom.h('.chat-buttons@buttons'),
354+
dom.h('.chat-toolbar-container@buttonsContainer', [
355+
dom.h('.chat-toolbar@toolbar'),
356+
]),
355357
]),
356358
dom.h('.chat-confirmation-widget-message@message'),
357-
dom.h('.chat-buttons-container@buttonsContainer', [
358-
dom.h('.chat-toolbar@toolbar'),
359+
dom.h('.chat-confirmation-widget-buttons', [
360+
dom.h('.chat-buttons@buttons'),
359361
]),
360362
]);
361363
this._domNode = elements.root;

‎src/vs/workbench/contrib/chat/browser/chatContentParts/media/chatConfirmationWidget.css‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -188,17 +188,6 @@
188188
justify-content: space-between;
189189
column-gap: 10px;
190190

191-
.chat-buttons {
192-
display: flex;
193-
column-gap: 10px;
194-
align-items: center;
195-
}
196-
197-
.monaco-button {
198-
overflow-wrap: break-word;
199-
padding: 2px 4px;
200-
}
201-
202191
.rendered-markdown {
203192
line-height: 24px !important;
204193
p {
@@ -226,20 +215,45 @@
226215
background: var(--vscode-chat-requestBackground);
227216
border: 1px solid var(--vscode-chat-requestBorder);
228217
padding: 6px 9px 0 9px;
229-
border-bottom-left-radius: 3px;
230-
border-bottom-right-radius: 3px;
231218

232219
p {
233220
margin-top: 0;
234221
margin-bottom: 9px;
235222
}
236223
}
237224

238-
.interactive-session .interactive-response .chat-confirmation-widget2 .chat-confirmation-message-terminal .interactive-result-code-block {
239-
border-top-left-radius: 0px;
240-
border-top-right-radius: 0px;
225+
226+
.chat-confirmation-widget2 .chat-confirmation-message-terminal .interactive-result-code-block {
227+
border-radius: 0px;
228+
}
229+
.chat-confirmation-widget2.hideButtons .chat-confirmation-widget-message,
230+
.chat-confirmation-widget2.hideButtons .chat-confirmation-message-terminal .interactive-result-code-block,
231+
.chat-confirmation-widget2.hideButtons .chat-confirmation-message-terminal:not(:has(.interactive-result-code-block)) {
232+
border-bottom-left-radius: 4px !important;
233+
border-bottom-right-radius: 4px !important;
241234
}
242235

243-
.chat-confirmation-widget2.hideButtons .chat-buttons {
236+
.chat-confirmation-widget2.hideButtons .chat-confirmation-widget-buttons,
237+
.chat-confirmation-widget2.hideButtons .chat-toolbar-container {
244238
display: none;
245239
}
240+
241+
.chat-confirmation-widget2 .chat-confirmation-widget-buttons {
242+
display: flex;
243+
border: 1px solid var(--vscode-chat-requestBorder);
244+
border-top: none;
245+
border-bottom-left-radius: 4px;
246+
border-bottom-right-radius: 4px;
247+
padding: 5px 9px;
248+
249+
.chat-buttons {
250+
display: flex;
251+
column-gap: 10px;
252+
align-items: center;
253+
254+
.monaco-button {
255+
overflow-wrap: break-word;
256+
padding: 2px 4px;
257+
}
258+
}
259+
}

‎src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,18 @@ export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationS
8787
terminalData = migrateLegacyTerminalToolSpecificData(terminalData);
8888

8989
const { title, message, disclaimer, terminalCustomActions } = toolInvocation.confirmationMessages;
90-
const runLabel = localize('run', "Run");
91-
const runKeybinding = keybindingService.lookupKeybinding(AcceptToolConfirmationActionId)?.getLabel();
92-
const runTooltip = runKeybinding ? `${runLabel} (${runKeybinding})` : runLabel;
90+
const continueLabel = localize('continue', "Continue");
91+
const continueKeybinding = keybindingService.lookupKeybinding(AcceptToolConfirmationActionId)?.getLabel();
92+
const continueTooltip = continueKeybinding ? `${continueLabel} (${continueKeybinding})` : continueLabel;
9393
const cancelLabel = localize('cancel', "Cancel");
9494
const cancelKeybinding = keybindingService.lookupKeybinding(CancelChatActionId)?.getLabel();
9595
const cancelTooltip = cancelKeybinding ? `${cancelLabel} (${cancelKeybinding})` : cancelLabel;
9696

9797
const buttons: IChatConfirmationButton[] = [
9898
{
99-
label: runLabel,
99+
label: continueLabel,
100100
data: true,
101-
tooltip: runTooltip,
101+
tooltip: continueTooltip,
102102
moreActions: terminalCustomActions,
103103
},
104104
{

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,8 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
337337
}
338338
confirmationMessages = isAutoApproved ? undefined : {
339339
title: args.isBackground
340-
? localize('runInTerminal.background', "{0} (background terminal)", shellType)
341-
: shellType,
340+
? localize('runInTerminal.background', "Run `{0}` command? (background terminal)", shellType)
341+
: localize('runInTerminal', "Run `{0}` command?", shellType),
342342
message: new MarkdownString(args.explanation),
343343
disclaimer,
344344
terminalCustomActions: customActions,

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalTool.test.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ suite('RunInTerminalTool', () => {
159159
command: 'rm file.txt',
160160
explanation: 'Remove a file'
161161
});
162-
assertConfirmationRequired(result, 'pwsh');
162+
assertConfirmationRequired(result, 'Run `pwsh` command?');
163163
});
164164

165165
test('should require confirmation for commands in deny list even if in allow list', async () => {
@@ -172,7 +172,7 @@ suite('RunInTerminalTool', () => {
172172
command: 'rm dangerous-file.txt',
173173
explanation: 'Remove a dangerous file'
174174
});
175-
assertConfirmationRequired(result, 'pwsh');
175+
assertConfirmationRequired(result, 'Run `pwsh` command?');
176176
});
177177

178178
test('should handle background commands with confirmation', async () => {
@@ -185,7 +185,7 @@ suite('RunInTerminalTool', () => {
185185
explanation: 'Start watching for file changes',
186186
isBackground: true
187187
});
188-
assertConfirmationRequired(result, 'pwsh (background terminal)');
188+
assertConfirmationRequired(result, 'Run `pwsh` command? (background terminal)');
189189
});
190190

191191
test('should auto-approve background commands in allow list', async () => {
@@ -312,7 +312,7 @@ suite('RunInTerminalTool', () => {
312312
explanation: 'Build the project'
313313
});
314314

315-
assertConfirmationRequired(result, 'pwsh');
315+
assertConfirmationRequired(result, 'Run `pwsh` command?');
316316
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
317317

318318
const customActions = result!.confirmationMessages!.terminalCustomActions!;
@@ -384,7 +384,7 @@ suite('RunInTerminalTool', () => {
384384
explanation: 'Build the project'
385385
});
386386

387-
assertConfirmationRequired(result, 'pwsh');
387+
assertConfirmationRequired(result, 'Run `pwsh` command?');
388388
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
389389

390390
const customActions = result!.confirmationMessages!.terminalCustomActions!;
@@ -401,7 +401,7 @@ suite('RunInTerminalTool', () => {
401401
explanation: 'Install dependencies and build'
402402
});
403403

404-
assertConfirmationRequired(result, 'pwsh');
404+
assertConfirmationRequired(result, 'Run `pwsh` command?');
405405
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
406406

407407
const customActions = result!.confirmationMessages!.terminalCustomActions!;
@@ -432,7 +432,7 @@ suite('RunInTerminalTool', () => {
432432
explanation: 'Run foo command and show first 20 lines'
433433
});
434434

435-
assertConfirmationRequired(result, 'pwsh');
435+
assertConfirmationRequired(result, 'Run `pwsh` command?');
436436
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
437437

438438
const customActions = result!.confirmationMessages!.terminalCustomActions!;
@@ -478,7 +478,7 @@ suite('RunInTerminalTool', () => {
478478
explanation: 'Run multiple piped commands'
479479
});
480480

481-
assertConfirmationRequired(result, 'pwsh');
481+
assertConfirmationRequired(result, 'Run `pwsh` command?');
482482
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
483483

484484
const customActions = result!.confirmationMessages!.terminalCustomActions!;

0 commit comments

Comments
 (0)