From 70140a270ccb3918a3d2e030ccd9ebbe5664ff99 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Mon, 6 Jan 2025 11:15:40 +0100 Subject: [PATCH] Add a setting to warn when collapsing unsubmitted comments (#236589) Fixes https://github.com/microsoft/vscode-pull-request-github/issues/1455 --- .../comments/browser/commentThreadBody.ts | 5 ++- .../comments/browser/commentThreadHeader.ts | 8 ++-- .../comments/browser/commentThreadWidget.ts | 44 ++++++++++++++----- .../comments/browser/comments.contribution.ts | 6 +++ 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadBody.ts b/src/vs/workbench/contrib/comments/browser/commentThreadBody.ts index fa68fe09303..035b88131fe 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadBody.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadBody.ts @@ -41,7 +41,6 @@ export class CommentThreadBody extends D return this._commentElements.filter(node => node.isEditing)[0]; } - constructor( private readonly _parentEditor: LayoutableEditor, readonly owner: string, @@ -77,6 +76,10 @@ export class CommentThreadBody extends D this._commentsElement.focus(); } + hasCommentsInEditMode() { + return this._commentElements.some(commentNode => commentNode.isEditing); + } + ensureFocusIntoNewEditingComment() { if (this._commentElements.length === 1 && this._commentElements[0].isEditing) { this._commentElements[0].setFocus(true); diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts b/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts index 3f42e0cdf32..cb721233760 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts @@ -44,9 +44,9 @@ export class CommentThreadHeader extends Disposable { private _delegate: { collapse: () => void }, private _commentMenus: CommentMenus, private _commentThread: languages.CommentThread, - private _contextKeyService: IContextKeyService, - private instantiationService: IInstantiationService, - private _contextMenuService: IContextMenuService + @IContextKeyService private readonly _contextKeyService: IContextKeyService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @IContextMenuService private readonly _contextMenuService: IContextMenuService ) { super(); this._headElement = dom.$('.head'); @@ -63,7 +63,7 @@ export class CommentThreadHeader extends Disposable { const actionsContainer = dom.append(this._headElement, dom.$('.review-actions')); this._actionbarWidget = new ActionBar(actionsContainer, { - actionViewItemProvider: createActionViewItem.bind(undefined, this.instantiationService) + actionViewItemProvider: createActionViewItem.bind(undefined, this._instantiationService) }); this._register(this._actionbarWidget); diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts b/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts index 94578614d7b..41fbd153cfd 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts @@ -5,6 +5,7 @@ import './media/review.css'; import * as dom from '../../../../base/browser/dom.js'; +import * as nls from '../../../../nls.js'; import * as domStylesheets from '../../../../base/browser/domStylesheets.js'; import { Emitter } from '../../../../base/common/event.js'; import { Disposable, dispose, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; @@ -28,7 +29,6 @@ import { IRange, Range } from '../../../../editor/common/core/range.js'; import { commentThreadStateBackgroundColorVar, commentThreadStateColorVar } from './commentColors.js'; import { ICellRange } from '../../notebook/common/notebookRange.js'; import { FontInfo } from '../../../../editor/common/config/fontInfo.js'; -import { IContextMenuService } from '../../../../platform/contextview/browser/contextView.js'; import { registerNavigableContainer } from '../../../browser/actions/widgetNavigationCommands.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { COMMENTS_SECTION, ICommentsConfiguration } from '../common/commentsConfiguration.js'; @@ -38,6 +38,8 @@ import { IKeybindingService } from '../../../../platform/keybinding/common/keybi import { AccessibilityCommandId } from '../../accessibility/common/accessibilityCommands.js'; import { LayoutableEditor } from './simpleCommentEditor.js'; import { isCodeEditor } from '../../../../editor/browser/editorBrowser.js'; +import { IDialogService } from '../../../../platform/dialogs/common/dialogs.js'; +import Severity from '../../../../base/common/severity.js'; export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration'; @@ -76,10 +78,11 @@ export class CommentThreadWidget extends actionRunner: (() => void) | null; collapse: () => void; }, - @ICommentService private commentService: ICommentService, - @IContextMenuService contextMenuService: IContextMenuService, - @IConfigurationService private configurationService: IConfigurationService, - @IKeybindingService private _keybindingService: IKeybindingService + @ICommentService private readonly commentService: ICommentService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IKeybindingService private readonly _keybindingService: IKeybindingService, + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IDialogService private readonly _dialogService: IDialogService ) { super(); @@ -89,16 +92,14 @@ export class CommentThreadWidget extends this._commentMenus = this.commentService.getCommentMenus(this._owner); - this._register(this._header = new CommentThreadHeader( + this._register(this._header = this._scopedInstantiationService.createInstance( + CommentThreadHeader, container, { - collapse: this.collapse.bind(this) + collapse: this.collapseAction.bind(this) }, this._commentMenus, - this._commentThread, - this._contextKeyService, - this._scopedInstantiationService, - contextMenuService + this._commentThread )); this._header.updateCommentThread(this._commentThread); @@ -159,6 +160,21 @@ export class CommentThreadWidget extends this.currentThreadListeners(); } + private async confirmCollapse(): Promise { + const confirmSetting = this._configurationService.getValue<'whenHasUnsubmittedComments' | 'never'>('comments.thread.confirmOnCollapse'); + + const hasUnsubmitted = !!this._commentReply?.commentEditor.getValue() || this._body.hasCommentsInEditMode(); + if (confirmSetting === 'whenHasUnsubmittedComments' && hasUnsubmitted) { + const result = await this._dialogService.confirm({ + message: nls.localize('confirmCollapse', "This comment thread has unsubmitted comments. Do you want to collapse it?"), + primaryButton: nls.localize('collapse', "Collapse"), + type: Severity.Warning + }); + return result.confirmed; + } + return true; + } + private _setAriaLabel(): void { let ariaLabel = localize('commentLabel', "Comment"); let keybinding: string | undefined; @@ -375,6 +391,12 @@ export class CommentThreadWidget extends } } + private async collapseAction() { + if (await this.confirmCollapse()) { + this.collapse(); + } + } + collapse() { if (Range.isIRange(this.commentThread.range) && isCodeEditor(this._parentEditor)) { this._parentEditor.setSelection(this.commentThread.range); diff --git a/src/vs/workbench/contrib/comments/browser/comments.contribution.ts b/src/vs/workbench/contrib/comments/browser/comments.contribution.ts index d2cfe1a5680..3cf98a0cde9 100644 --- a/src/vs/workbench/contrib/comments/browser/comments.contribution.ts +++ b/src/vs/workbench/contrib/comments/browser/comments.contribution.ts @@ -138,6 +138,12 @@ Registry.as(ConfigurationExtensions.Configuration).regis type: 'boolean', default: true, description: nls.localize('collapseOnResolve', "Controls whether the comment thread should collapse when the thread is resolved.") + }, + 'comments.thread.confirmOnCollapse': { + type: 'string', + enum: ['whenHasUnsubmittedComments', 'never'], + default: 'never', + description: nls.localize('confirmOnCollapse', "Controls whether a confirmation dialog is shown when collapsing a comment thread with unsubmitted comments.") } } });