Add a setting to warn when collapsing unsubmitted comments (#236589)

Fixes https://github.com/microsoft/vscode-pull-request-github/issues/1455
pull/229089/merge
Alex Ross 2025-01-06 11:15:40 +01:00 committed by GitHub
parent 47258d8289
commit 70140a270c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 47 additions and 16 deletions

View File

@ -41,7 +41,6 @@ export class CommentThreadBody<T extends IRange | ICellRange = IRange> extends D
return this._commentElements.filter(node => node.isEditing)[0]; return this._commentElements.filter(node => node.isEditing)[0];
} }
constructor( constructor(
private readonly _parentEditor: LayoutableEditor, private readonly _parentEditor: LayoutableEditor,
readonly owner: string, readonly owner: string,
@ -77,6 +76,10 @@ export class CommentThreadBody<T extends IRange | ICellRange = IRange> extends D
this._commentsElement.focus(); this._commentsElement.focus();
} }
hasCommentsInEditMode() {
return this._commentElements.some(commentNode => commentNode.isEditing);
}
ensureFocusIntoNewEditingComment() { ensureFocusIntoNewEditingComment() {
if (this._commentElements.length === 1 && this._commentElements[0].isEditing) { if (this._commentElements.length === 1 && this._commentElements[0].isEditing) {
this._commentElements[0].setFocus(true); this._commentElements[0].setFocus(true);

View File

@ -44,9 +44,9 @@ export class CommentThreadHeader<T = IRange> extends Disposable {
private _delegate: { collapse: () => void }, private _delegate: { collapse: () => void },
private _commentMenus: CommentMenus, private _commentMenus: CommentMenus,
private _commentThread: languages.CommentThread<T>, private _commentThread: languages.CommentThread<T>,
private _contextKeyService: IContextKeyService, @IContextKeyService private readonly _contextKeyService: IContextKeyService,
private instantiationService: IInstantiationService, @IInstantiationService private readonly _instantiationService: IInstantiationService,
private _contextMenuService: IContextMenuService @IContextMenuService private readonly _contextMenuService: IContextMenuService
) { ) {
super(); super();
this._headElement = <HTMLDivElement>dom.$('.head'); this._headElement = <HTMLDivElement>dom.$('.head');
@ -63,7 +63,7 @@ export class CommentThreadHeader<T = IRange> extends Disposable {
const actionsContainer = dom.append(this._headElement, dom.$('.review-actions')); const actionsContainer = dom.append(this._headElement, dom.$('.review-actions'));
this._actionbarWidget = new ActionBar(actionsContainer, { this._actionbarWidget = new ActionBar(actionsContainer, {
actionViewItemProvider: createActionViewItem.bind(undefined, this.instantiationService) actionViewItemProvider: createActionViewItem.bind(undefined, this._instantiationService)
}); });
this._register(this._actionbarWidget); this._register(this._actionbarWidget);

View File

@ -5,6 +5,7 @@
import './media/review.css'; import './media/review.css';
import * as dom from '../../../../base/browser/dom.js'; import * as dom from '../../../../base/browser/dom.js';
import * as nls from '../../../../nls.js';
import * as domStylesheets from '../../../../base/browser/domStylesheets.js'; import * as domStylesheets from '../../../../base/browser/domStylesheets.js';
import { Emitter } from '../../../../base/common/event.js'; import { Emitter } from '../../../../base/common/event.js';
import { Disposable, dispose, IDisposable, toDisposable } from '../../../../base/common/lifecycle.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 { commentThreadStateBackgroundColorVar, commentThreadStateColorVar } from './commentColors.js';
import { ICellRange } from '../../notebook/common/notebookRange.js'; import { ICellRange } from '../../notebook/common/notebookRange.js';
import { FontInfo } from '../../../../editor/common/config/fontInfo.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 { registerNavigableContainer } from '../../../browser/actions/widgetNavigationCommands.js';
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
import { COMMENTS_SECTION, ICommentsConfiguration } from '../common/commentsConfiguration.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 { AccessibilityCommandId } from '../../accessibility/common/accessibilityCommands.js';
import { LayoutableEditor } from './simpleCommentEditor.js'; import { LayoutableEditor } from './simpleCommentEditor.js';
import { isCodeEditor } from '../../../../editor/browser/editorBrowser.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'; export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration';
@ -76,10 +78,11 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
actionRunner: (() => void) | null; actionRunner: (() => void) | null;
collapse: () => void; collapse: () => void;
}, },
@ICommentService private commentService: ICommentService, @ICommentService private readonly commentService: ICommentService,
@IContextMenuService contextMenuService: IContextMenuService, @IConfigurationService private readonly configurationService: IConfigurationService,
@IConfigurationService private configurationService: IConfigurationService, @IKeybindingService private readonly _keybindingService: IKeybindingService,
@IKeybindingService private _keybindingService: IKeybindingService @IConfigurationService private readonly _configurationService: IConfigurationService,
@IDialogService private readonly _dialogService: IDialogService
) { ) {
super(); super();
@ -89,16 +92,14 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
this._commentMenus = this.commentService.getCommentMenus(this._owner); this._commentMenus = this.commentService.getCommentMenus(this._owner);
this._register(this._header = new CommentThreadHeader<T>( this._register(this._header = this._scopedInstantiationService.createInstance(
CommentThreadHeader,
container, container,
{ {
collapse: this.collapse.bind(this) collapse: this.collapseAction.bind(this)
}, },
this._commentMenus, this._commentMenus,
this._commentThread, this._commentThread
this._contextKeyService,
this._scopedInstantiationService,
contextMenuService
)); ));
this._header.updateCommentThread(this._commentThread); this._header.updateCommentThread(this._commentThread);
@ -159,6 +160,21 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
this.currentThreadListeners(); this.currentThreadListeners();
} }
private async confirmCollapse(): Promise<boolean> {
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 { private _setAriaLabel(): void {
let ariaLabel = localize('commentLabel', "Comment"); let ariaLabel = localize('commentLabel', "Comment");
let keybinding: string | undefined; let keybinding: string | undefined;
@ -375,6 +391,12 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
} }
} }
private async collapseAction() {
if (await this.confirmCollapse()) {
this.collapse();
}
}
collapse() { collapse() {
if (Range.isIRange(this.commentThread.range) && isCodeEditor(this._parentEditor)) { if (Range.isIRange(this.commentThread.range) && isCodeEditor(this._parentEditor)) {
this._parentEditor.setSelection(this.commentThread.range); this._parentEditor.setSelection(this.commentThread.range);

View File

@ -138,6 +138,12 @@ Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration).regis
type: 'boolean', type: 'boolean',
default: true, default: true,
description: nls.localize('collapseOnResolve', "Controls whether the comment thread should collapse when the thread is resolved.") 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.")
} }
} }
}); });