Skip to content

Update confirmations to use new proposed terminal style #260770

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Aug 12, 2025
Merged

Update confirmations to use new proposed terminal style #260770

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e8cd3e6
Clean up terminal tool part names
Tyriar Aug 9, 2025
df1ce23
Terminal progress UI
Tyriar Aug 9, 2025
041dd57
Move message into hover
Tyriar Aug 9, 2025
75d3635
Move tests to new UI
Tyriar Aug 9, 2025
0c6da45
Bring elicitation over
Tyriar Aug 9, 2025
b4b07a0
Further polishing cases
Tyriar Aug 9, 2025
ba2ec76
More confirmation polish
Tyriar Aug 9, 2025
0b413e0
Merge remote-tracking branch 'origin/main' into tyriar/257468
Tyriar Aug 9, 2025
ab95f94
Register model
Tyriar Aug 10, 2025
2443ee5
Make bottom corners rounded
Tyriar Aug 10, 2025
f7d919f
Fix test expectations
Tyriar Aug 10, 2025
3bd5689
Merge remote-tracking branch 'origin/main' into tyriar/257468
Tyriar Aug 12, 2025
67b78f4
More merge conflict fixes
Tyriar Aug 12, 2025
a5d813d
Introduce thenRegisterOrDispose helper
Tyriar Aug 12, 2025
152f679
Fix test expectations
Tyriar Aug 12, 2025
afabd39
Fix margin issue
Tyriar Aug 12, 2025
3ffb2fa
Fix elicitation message duplicating and buttons not hiding
Tyriar Aug 12, 2025
19e3d7f
fix elicitation not rendering result immediately
connor4312 Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 182 additions & 0 deletions src/vs/workbench/contrib/chat/browser/chatContentParts/chatConfirmationWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import { Action } from '../../../../../base/common/actions.js';
import { Emitter, Event } from '../../../../../base/common/event.js';
import { IMarkdownString, MarkdownString } from '../../../../../base/common/htmlContent.js';
import { Disposable, DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js';
import type { ThemeIcon } from '../../../../../base/common/themables.js';
import { IMarkdownRenderResult, MarkdownRenderer, openLinkFromMarkdown } from '../../../../../editor/browser/widget/markdownRenderer/browser/markdownRenderer.js';
import { localize } from '../../../../../nls.js';
import { MenuWorkbenchToolBar } from '../../../../../platform/actions/browser/toolbar.js';
import { MenuId } from '../../../../../platform/actions/common/actions.js';
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js';
import { IContextMenuService } from '../../../../../platform/contextview/browser/contextView.js';
import { IHoverService } from '../../../../../platform/hover/browser/hover.js';
import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js';
import { FocusMode } from '../../../../../platform/native/common/native.js';
Expand Down Expand Up @@ -304,3 +306,183 @@ export class ChatCustomConfirmationWidget extends BaseChatConfirmationWidget {
this.renderMessage(options.message, container);
}
}

export interface IChatConfirmationWidget2Options {
title: string | IMarkdownString;
icon: ThemeIcon;
subtitle?: string | IMarkdownString;
buttons: IChatConfirmationButton[];
toolbarData?: { arg: any; partType: string };
}

abstract class BaseChatConfirmationWidget2 extends Disposable {
private _onDidClick = this._register(new Emitter<IChatConfirmationButton>());
get onDidClick(): Event<IChatConfirmationButton> { return this._onDidClick.event; }

protected _onDidChangeHeight = this._register(new Emitter<void>());
get onDidChangeHeight(): Event<void> { return this._onDidChangeHeight.event; }

private _domNode: HTMLElement;
get domNode(): HTMLElement {
return this._domNode;
}

private get showingButtons() {
return !this.domNode.classList.contains('hideButtons');
}

setShowButtons(showButton: boolean): void {
this.domNode.classList.toggle('hideButtons', !showButton);
}

private readonly messageElement: HTMLElement;
protected readonly markdownRenderer: MarkdownRenderer;
private readonly title: string | IMarkdownString;

private readonly notification = this._register(new MutableDisposable<DisposableStore>());

constructor(
options: IChatConfirmationWidget2Options,
@IInstantiationService protected readonly instantiationService: IInstantiationService,
@IContextMenuService contextMenuService: IContextMenuService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
@IHostService private readonly _hostService: IHostService,
@IViewsService private readonly _viewsService: IViewsService,
@IContextKeyService contextKeyService: IContextKeyService,
) {
super();

const { title, subtitle, buttons, icon } = options;
this.title = title;

const elements = dom.h('.chat-confirmation-widget2@root', [
dom.h('.chat-confirmation-widget-title', [
dom.h('.chat-title@title'),
dom.h('.chat-buttons@buttons'),
]),
dom.h('.chat-confirmation-widget-message@message'),
dom.h('.chat-buttons-container@buttonsContainer', [
dom.h('.chat-toolbar@toolbar'),
]),
]);
this._domNode = elements.root;
this.markdownRenderer = this.instantiationService.createInstance(MarkdownRenderer, {});

const titlePart = this._register(instantiationService.createInstance(
ChatQueryTitlePart,
elements.title,
new MarkdownString(`$(${icon.id}) ${title}`),
subtitle,
this.markdownRenderer,
));

this._register(titlePart.onDidChangeHeight(() => this._onDidChangeHeight.fire()));

this.messageElement = elements.message;

for (const buttonData of buttons) {
const buttonOptions: IButtonOptions = { ...defaultButtonStyles, secondary: buttonData.isSecondary, title: buttonData.tooltip, disabled: buttonData.disabled };

let button: IButton;
if (buttonData.moreActions) {
button = new ButtonWithDropdown(elements.buttons, {
...buttonOptions,
contextMenuProvider: contextMenuService,
addPrimaryActionToDropdown: false,
actions: buttonData.moreActions.map(action => this._register(new Action(
action.label,
action.label,
undefined,
!action.disabled,
() => {
this._onDidClick.fire(action);
return Promise.resolve();
},
))),
});
} else {
button = new Button(elements.buttons, buttonOptions);
}

this._register(button);
button.label = buttonData.label;
this._register(button.onDidClick(() => this._onDidClick.fire(buttonData)));
if (buttonData.onDidChangeDisablement) {
this._register(buttonData.onDidChangeDisablement(disabled => button.enabled = !disabled));
}
}

// Create toolbar if actions are provided
if (options?.toolbarData) {
const overlay = contextKeyService.createOverlay([['chatConfirmationPartType', options.toolbarData.partType]]);
const nestedInsta = this._register(instantiationService.createChild(new ServiceCollection([IContextKeyService, overlay])));
this._register(nestedInsta.createInstance(
MenuWorkbenchToolBar,
elements.toolbar,
MenuId.ChatConfirmationMenu,
{
// buttonConfigProvider: () => ({ showLabel: false, showIcon: true }),
menuOptions: {
arg: options.toolbarData.arg,
shouldForwardArgs: true,
}
}
));
}
}

protected renderMessage(element: HTMLElement, listContainer: HTMLElement): void {
this.messageElement.append(element);

if (this.showingButtons && this._configurationService.getValue<boolean>('chat.notifyWindowOnConfirmation')) {
const targetWindow = dom.getWindow(listContainer);
if (!targetWindow.document.hasFocus()) {
this.notifyConfirmationNeeded(targetWindow);
}
}
}

private async notifyConfirmationNeeded(targetWindow: Window): Promise<void> {

// Focus Window
this._hostService.focus(targetWindow, { mode: FocusMode.Notify });

// Notify
const title = renderAsPlaintext(this.title);
const notification = await dom.triggerNotification(title ? localize('notificationTitle', "Chat: {0}", title) : localize('defaultTitle', "Chat: Confirmation Required"),
{
detail: localize('notificationDetail', "The current chat session requires your confirmation to proceed.")
}
);
if (notification) {
const disposables = this.notification.value = new DisposableStore();
disposables.add(notification);

disposables.add(Event.once(notification.onClick)(() => {
this._hostService.focus(targetWindow, { mode: FocusMode.Force });
showChatView(this._viewsService);
}));

disposables.add(this._hostService.onDidChangeFocus(focus => {
if (focus) {
disposables.dispose();
}
}));
}
}
}
export class ChatCustomConfirmationWidget2 extends BaseChatConfirmationWidget2 {
constructor(
container: HTMLElement,
options: IChatConfirmationWidget2Options & { message: HTMLElement },
@IInstantiationService instantiationService: IInstantiationService,
@IContextMenuService contextMenuService: IContextMenuService,
@IConfigurationService configurationService: IConfigurationService,
@IHostService hostService: IHostService,
@IViewsService viewsService: IViewsService,
@IContextKeyService contextKeyService: IContextKeyService,
) {
super(options, instantiationService, contextMenuService, configurationService, hostService, viewsService, contextKeyService);
this.renderMessage(options.message, container);
}
}
32 changes: 32 additions & 0 deletions src/vs/workbench/contrib/chat/browser/chatContentParts/media/chatConfirmationWidget.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,35 @@
}
}
}

.chat-confirmation-widget2 .chat-confirmation-widget-title {
border: 1px solid var(--vscode-chat-requestBorder);
border-bottom: none;
border-top-left-radius: 4px;
border-top-right-radius: 4px;
padding: 5px 9px;
display: flex;
justify-content: space-between;

.chat-buttons {
display: flex;
column-gap: 10px;
}

.monaco-button {
overflow-wrap: break-word;
padding: 2px 4px
}

.rendered-markdown {
line-height: 24px !important;
p {
margin: 0 !important;
}
}
}

.interactive-session .interactive-response .chat-confirmation-widget2 .chat-confirmation-widget-message .interactive-result-code-block {
border-top-left-radius: 0px;
border-top-right-radius: 0px;
}
54 changes: 38 additions & 16 deletions ...nvocationParts/chatTerminalToolSubPart.ts → ...ts/chatTerminalToolConfirmationSubPart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
*--------------------------------------------------------------------------------------------*/

import * as dom from '../../../../../../base/browser/dom.js';
import { HoverPosition } from '../../../../../../base/browser/ui/hover/hoverWidget.js';
import { asArray } from '../../../../../../base/common/arrays.js';
import { Codicon } from '../../../../../../base/common/codicons.js';
import { ErrorNoTelemetry } from '../../../../../../base/common/errors.js';
import { MarkdownString, type IMarkdownString } from '../../../../../../base/common/htmlContent.js';
import { thenIfNotDisposed } from '../../../../../../base/common/lifecycle.js';
Expand All @@ -15,9 +17,11 @@ import { generateUuid } from '../../../../../../base/common/uuid.js';
import { MarkdownRenderer } from '../../../../../../editor/browser/widget/markdownRenderer/browser/markdownRenderer.js';
import { ILanguageService } from '../../../../../../editor/common/languages/language.js';
import { IModelService } from '../../../../../../editor/common/services/model.js';
import { ITextModelService } from '../../../../../../editor/common/services/resolverService.js';
import { localize } from '../../../../../../nls.js';
import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js';
import { IHoverService } from '../../../../../../platform/hover/browser/hover.js';
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
import { IKeybindingService } from '../../../../../../platform/keybinding/common/keybinding.js';
import { IPreferencesService } from '../../../../../services/preferences/common/preferences.js';
Expand All @@ -30,11 +34,13 @@ import { CancelChatActionId } from '../../actions/chatExecuteActions.js';
import { AcceptToolConfirmationActionId } from '../../actions/chatToolActions.js';
import { IChatCodeBlockInfo, IChatWidgetService } from '../../chat.js';
import { ICodeBlockRenderOptions } from '../../codeBlockPart.js';
import { ChatCustomConfirmationWidget, IChatConfirmationButton } from '../chatConfirmationWidget.js';
import { ChatCustomConfirmationWidget2, IChatConfirmationButton } from '../chatConfirmationWidget.js';
import { IChatContentPartRenderContext } from '../chatContentParts.js';
import { ChatMarkdownContentPart, EditorPool } from '../chatMarkdownContentPart.js';
import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js';

const $ = dom.$;

export interface ITerminalNewAutoApproveRule {
key: string;
value: boolean | {
Expand All @@ -48,7 +54,7 @@ export type TerminalNewAutoApproveButtonData = (
{ type: 'newRule'; rule: ITerminalNewAutoApproveRule | ITerminalNewAutoApproveRule[] }
);

export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSubPart {
export class ChatTerminalToolConfirmationSubPart extends BaseChatToolInvocationSubPart {
public readonly domNode: HTMLElement;
public readonly codeblocks: IChatCodeBlockInfo[] = [];

Expand All @@ -69,6 +75,8 @@ export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSub
@IContextKeyService private readonly contextKeyService: IContextKeyService,
@IChatWidgetService private readonly chatWidgetService: IChatWidgetService,
@IPreferencesService private readonly preferencesService: IPreferencesService,
@ITextModelService textModelService: ITextModelService,
@IHoverService hoverService: IHoverService,
) {
super(toolInvocation);

Expand Down Expand Up @@ -98,11 +106,8 @@ export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSub
data: false,
isSecondary: true,
tooltip: cancelTooltip,
}];
const renderedMessage = this._register(this.renderer.render(
typeof message === 'string' ? new MarkdownString(message) : message,
{ asyncRenderCallback: () => this._onDidChangeHeight.fire() }
));
}
];
const codeBlockRenderOptions: ICodeBlockRenderOptions = {
hideToolbar: true,
reserveWidth: 19,
Expand All @@ -114,19 +119,26 @@ export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSub
ariaLabel: typeof title === 'string' ? title : title.value
}
};
const langId = this.languageService.getLanguageIdByLanguageName(terminalData.language ?? 'sh') ?? 'shellscript';
const languageId = this.languageService.getLanguageIdByLanguageName(terminalData.language ?? 'sh') ?? 'shellscript';
const model = this.modelService.createModel(
terminalData.commandLine.toolEdited ?? terminalData.commandLine.original,
this.languageService.createById(langId),
this.languageService.createById(languageId),
this._getUniqueCodeBlockUri(),
true
);
textModelService.createModelReference(model.uri).then(ref => {
Copy link
Member

Choose a reason for hiding this comment

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

See my helper thenifNotDisposed

Copy link
Member

Choose a reason for hiding this comment

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

The fact that we have to do this is confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think I can use that since I need to call dispose on it to free the reference when the chat part is disposed?

Copy link
Member

Choose a reason for hiding this comment

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

Good point. There could probably be a helper that covers what you want to do

Copy link
Member Author

Choose a reason for hiding this comment

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

Added helper, created #261211 to adopt after this PR is merged.

if (this._store.isDisposed) {
ref.dispose();
} else {
this._register(ref);
}
});
const editor = this._register(this.editorPool.get());
const renderPromise = editor.object.render({
codeBlockIndex: this.codeBlockStartIndex,
codeBlockPartIndex: 0,
element: this.context.element,
languageId: langId,
languageId,
renderOptions: codeBlockRenderOptions,
textModel: Promise.resolve(model),
chatSessionId: this.context.element.sessionId
Expand All @@ -150,17 +162,26 @@ export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSub
this._register(model.onDidChangeContent(e => {
terminalData.commandLine.userEdited = model.getValue();
}));
const element = dom.$('');
dom.append(element, editor.object.element);
dom.append(element, renderedMessage.element);
const messageElement = $('');
dom.append(messageElement, editor.object.element);
this._register(hoverService.setupDelayedHover(messageElement, {
content: message,
position: { hoverPosition: HoverPosition.BELOW },
appearance: { showPointer: true }
}));
const confirmWidget = this._register(this.instantiationService.createInstance(
ChatCustomConfirmationWidget,
ChatCustomConfirmationWidget2,
this.context.container,
{ title, message: element, buttons },
{
title,
icon: Codicon.terminal,
message: messageElement,
buttons
},
));

if (disclaimer) {
this._appendMarkdownPart(element, disclaimer, codeBlockRenderOptions);
this._appendMarkdownPart(messageElement, disclaimer, codeBlockRenderOptions);
}

ChatContextKeys.Editing.hasToolConfirmation.bindTo(this.contextKeyService).set(true);
Expand Down Expand Up @@ -217,6 +238,7 @@ export class TerminalConfirmationWidgetSubPart extends BaseChatToolInvocationSub
ChatContextKeys.Editing.hasToolConfirmation.bindTo(this.contextKeyService).set(false);
this._onNeedsRerender.fire();
});

this.domNode = confirmWidget.domNode;
}

Expand Down
2 changes: 1 addition & 1 deletion ...Parts/chatTerminalMarkdownProgressPart.ts → ...tionParts/chatTerminalToolProgressPart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ChatMarkdownContentPart, EditorPool } from '../chatMarkdownContentPart.
import { ChatCustomProgressPart } from '../chatProgressContentPart.js';
import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js';

export class ChatTerminalMarkdownProgressPart extends BaseChatToolInvocationSubPart {
export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart {
public readonly domNode: HTMLElement;

private markdownPart: ChatMarkdownContentPart | undefined;
Expand Down
Loading
Loading