From d15d213ba99eedc10c73f6bbce1e52757b128c24 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Mon, 28 Oct 2024 12:54:17 +0100 Subject: [PATCH] Unable to disable/remap Escape key closing CommentThreadWidget Fixes #231674 --- .../comments/browser/commentThreadHeader.ts | 3 ++- .../comments/browser/commentThreadWidget.ts | 12 +++------- .../browser/commentThreadZoneWidget.ts | 4 ++++ .../comments/browser/commentsController.ts | 8 ++----- .../browser/commentsEditorContribution.ts | 23 ++++++++++++++++++- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts b/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts index c76c842f122ff..3f42e0cdf323f 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts @@ -23,6 +23,7 @@ import { IContextMenuService } from '../../../../platform/contextview/browser/co import { MarshalledId } from '../../../../base/common/marshallingIds.js'; import { StandardMouseEvent } from '../../../../base/browser/mouseEvent.js'; import { MarshalledCommentThread } from '../../../common/comments.js'; +import { CommentCommandId } from '../common/commentCommandIds.js'; const collapseIcon = registerIcon('review-comment-collapse', Codicon.chevronUp, nls.localize('collapseIcon', 'Icon to collapse a review comment.')); const COLLAPSE_ACTION_CLASS = 'expand-review-action ' + ThemeIcon.asClassName(collapseIcon); @@ -68,7 +69,7 @@ export class CommentThreadHeader extends Disposable { this._register(this._actionbarWidget); const collapseClass = threadHasComments(this._commentThread.comments) ? COLLAPSE_ACTION_CLASS : DELETE_ACTION_CLASS; - this._collapseAction = new Action('review.expand', nls.localize('label.collapse', "Collapse"), collapseClass, true, () => this._delegate.collapse()); + this._collapseAction = new Action(CommentCommandId.Hide, nls.localize('label.collapse', "Collapse"), collapseClass, true, () => this._delegate.collapse()); if (!threadHasComments(this._commentThread.comments)) { const commentsChanged: MutableDisposable = this._register(new MutableDisposable()); commentsChanged.value = this._commentThread.onDidChangeComments(() => { diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts b/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts index ebad6e3f24bde..7bac752e389aa 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts @@ -36,7 +36,6 @@ import { AccessibilityVerbositySettingId } from '../../accessibility/browser/acc import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js'; import { AccessibilityCommandId } from '../../accessibility/common/accessibilityCommands.js'; import { LayoutableEditor } from './simpleCommentEditor.js'; -import { DomEmitter } from '../../../../base/browser/event.js'; import { isCodeEditor } from '../../../../editor/browser/editorBrowser.js'; export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration'; @@ -157,14 +156,6 @@ export class CommentThreadWidget extends } this.currentThreadListeners(); - this._register(new DomEmitter(this.container, 'keydown').event(e => { - if (dom.isKeyboardEvent(e) && e.key === 'Escape') { - if (Range.isIRange(this.commentThread.range) && isCodeEditor(this._parentEditor)) { - this._parentEditor.setSelection(this.commentThread.range); - } - this.collapse(); - } - })); } private _setAriaLabel(): void { @@ -384,6 +375,9 @@ export class CommentThreadWidget extends } collapse() { + if (Range.isIRange(this.commentThread.range) && isCodeEditor(this._parentEditor)) { + this._parentEditor.setSelection(this.commentThread.range); + } this._containerDelegate.collapse(); } diff --git a/src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts b/src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts index 4ef7bbc6025c1..6790337c79aca 100644 --- a/src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts +++ b/src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts @@ -504,6 +504,10 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget this._refresh(this._commentThreadWidget.getDimensions()); } + collapseAndFocusRange() { + this._commentThreadWidget.collapse(); + } + override hide() { if (this._isExpanded) { this._isExpanded = false; diff --git a/src/vs/workbench/contrib/comments/browser/commentsController.ts b/src/vs/workbench/contrib/comments/browser/commentsController.ts index c4cb1896a5386..6fc73d49776fc 100644 --- a/src/vs/workbench/contrib/comments/browser/commentsController.ts +++ b/src/vs/workbench/contrib/comments/browser/commentsController.ts @@ -1388,12 +1388,8 @@ export class CommentController implements IEditorContribution { } } - public closeWidget(): void { - this._commentWidgets?.forEach(widget => widget.hide()); - if (this.editor) { - this.editor.focus(); - this.editor.revealRangeInCenter(this.editor.getSelection()!); - } + public collapseAndFocusRange(threadId: string): void { + this._commentWidgets?.find(widget => widget.commentThread.threadId === threadId)?.collapseAndFocusRange(); } private removeCommentWidgetsAndStoreCache() { diff --git a/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts b/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts index 7574e10e00787..ce0458cd9421e 100644 --- a/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts +++ b/src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts @@ -421,11 +421,32 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({ weight: KeybindingWeight.EditorContrib, primary: KeyCode.Escape, secondary: [KeyMod.Shift | KeyCode.Escape], - when: ctxCommentEditorFocused, + when: ContextKeyExpr.or(ctxCommentEditorFocused, CommentContextKeys.commentFocused), handler: (accessor, args) => { const activeCodeEditor = accessor.get(ICodeEditorService).getFocusedCodeEditor(); if (activeCodeEditor instanceof SimpleCommentEditor) { activeCodeEditor.getParentThread().collapse(); + } else if (activeCodeEditor) { + const controller = CommentController.get(activeCodeEditor); + if (!controller) { + return; + } + const notificationService = accessor.get(INotificationService); + const commentService = accessor.get(ICommentService); + let error = false; + try { + const activeComment = commentService.lastActiveCommentcontroller?.activeComment; + if (!activeComment) { + error = true; + } else { + controller.collapseAndFocusRange(activeComment.thread.threadId); + } + } catch (e) { + error = true; + } + if (error) { + notificationService.error(nls.localize('comments.focusCommand.error', "The cursor must be on a line with a comment to focus the comment")); + } } } });