From 1e0a1f58214354089193a0ed03e5b5b722ba4132 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Tue, 18 Jul 2023 20:28:08 +0200 Subject: [PATCH 1/2] Introduces onlyShowAccessibleDiffViewer. Fixes #182789 --- .../editor/browser/widget/diffEditorWidget.ts | 2 + .../diffEditorWidget2/accessibleDiffViewer.ts | 41 +++++++++++-------- .../diffEditorWidget2/diffEditorOptions.ts | 3 ++ .../diffEditorWidget2/diffEditorWidget2.ts | 9 +++- src/vs/editor/common/config/editorOptions.ts | 5 +++ src/vs/monaco.d.ts | 4 ++ .../inlineChat/browser/inlineChatActions.ts | 4 +- .../browser/inlineChatLivePreviewWidget.ts | 5 ++- .../inlineChat/browser/inlineChatWidget.ts | 5 ++- 9 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/vs/editor/browser/widget/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditorWidget.ts index e21c1e9f3beb2..95c5bea3fe891 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget.ts @@ -299,6 +299,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE collapseUnchangedRegions: false, }, isInEmbeddedEditor: false, + onlyShowAccessibleDiffViewer: false, }); this.isEmbeddedDiffEditorKey = EditorContextKeys.isEmbeddedDiffEditor.bindTo(this._contextKeyService); @@ -2743,6 +2744,7 @@ function validateDiffEditorOptions(options: Readonly, defaul collapseUnchangedRegions: false, }, isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor), + onlyShowAccessibleDiffViewer: false, }; } diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts b/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts index 346f847085edf..94c20eb68da9c 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts @@ -42,7 +42,9 @@ const diffReviewCloseIcon = registerIcon('diff-review-close', Codicon.close, loc export class AccessibleDiffViewer extends Disposable { constructor( private readonly _parentNode: HTMLElement, - private readonly _visible: ISettableObservable, + private readonly _visible: IObservable, + private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void, + private readonly _canClose: IObservable, private readonly _width: IObservable, private readonly _height: IObservable, private readonly _diffs: IObservable, @@ -59,7 +61,7 @@ export class AccessibleDiffViewer extends Disposable { if (!visible) { return null; } - const model = store.add(this._instantiationService.createInstance(ViewModel, this._diffs, this._editors, this._visible)); + const model = store.add(this._instantiationService.createInstance(ViewModel, this._diffs, this._editors, this._setVisible, this._canClose)); const view = store.add(this._instantiationService.createInstance(View, this._parentNode, model, this._width, this._height, this._editors)); return { model, @@ -70,7 +72,7 @@ export class AccessibleDiffViewer extends Disposable { next(): void { transaction(tx => { const isVisible = this._visible.get(); - this._visible.set(true, tx); + this._setVisible(true, tx); if (isVisible) { this.model.get()!.model.nextGroup(tx); } @@ -79,14 +81,14 @@ export class AccessibleDiffViewer extends Disposable { prev(): void { transaction(tx => { - this._visible.set(true, tx); + this._setVisible(true, tx); this.model.get()!.model.previousGroup(tx); }); } close(): void { transaction(tx => { - this._visible.set(false, tx); + this._setVisible(false, tx); }); } } @@ -104,12 +106,11 @@ class ViewModel extends Disposable { public readonly currentElement: IObservable = this._currentElementIdx.map((idx, r) => this.currentGroup.read(r)?.lines[idx]); - public readonly canClose: IObservable = constObservable(true); - constructor( private readonly _diffs: IObservable, private readonly _editors: DiffEditorEditors, - private readonly _visible: ISettableObservable, + private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void, + public readonly canClose: IObservable, @IAudioCueService private readonly _audioCueService: IAudioCueService, ) { super(); @@ -192,7 +193,7 @@ class ViewModel extends Disposable { } revealCurrentElementInEditor(): void { - this._visible.set(false, undefined); + this._setVisible(false, undefined); const curElem = this.currentElement.get(); if (curElem) { @@ -211,7 +212,7 @@ class ViewModel extends Disposable { } close(): void { - this._visible.set(false, undefined); + this._setVisible(false, undefined); this._editors.modified.focus(); } } @@ -338,14 +339,18 @@ class View extends Disposable { this._actionBar = this._register(new ActionBar( actionBarContainer )); - this._actionBar.push(new Action( - 'diffreview.close', - localize('label.close', "Close"), - 'close-diff-review ' + ThemeIcon.asClassName(diffReviewCloseIcon), - true, - async () => _model.close() - ), { label: false, icon: true }); - + this._register(autorun('update actions', reader => { + this._actionBar.clear(); + if (this._model.canClose.read(reader)) { + this._actionBar.push(new Action( + 'diffreview.close', + localize('label.close', "Close"), + 'close-diff-review ' + ThemeIcon.asClassName(diffReviewCloseIcon), + true, + async () => _model.close() + ), { label: false, icon: true }); + } + })); this._content = document.createElement('div'); this._content.className = 'diff-review-content'; diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions.ts b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions.ts index fcec4e86f0926..eed16b994722a 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions.ts @@ -46,6 +46,7 @@ export class DiffEditorOptions { public readonly accessibilityVerbose = derived('accessibilityVerbose', reader => this._options.read(reader).accessibilityVerbose); public readonly diffAlgorithm = derived('diffAlgorithm', reader => this._options.read(reader).diffAlgorithm); public readonly showEmptyDecorations = derived('showEmptyDecorations', reader => this._options.read(reader).experimental.showEmptyDecorations!); + public readonly onlyShowAccessibleDiffViewer = derived('onlyShowAccessibleDiffViewer', reader => this._options.read(reader).onlyShowAccessibleDiffViewer); public updateOptions(changedOptions: IDiffEditorOptions): void { const newDiffEditorOptions = validateDiffEditorOptions(changedOptions, this._options.get()); @@ -75,6 +76,7 @@ const diffEditorDefaultOptions: ValidDiffEditorBaseOptions = { showEmptyDecorations: true, }, isInEmbeddedEditor: false, + onlyShowAccessibleDiffViewer: false, }; function validateDiffEditorOptions(options: Readonly, defaults: ValidDiffEditorBaseOptions): ValidDiffEditorBaseOptions { @@ -99,5 +101,6 @@ function validateDiffEditorOptions(options: Readonly, defaul showEmptyDecorations: validateBooleanOption(options.experimental?.showEmptyDecorations, defaults.experimental.showEmptyDecorations!), }, isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor), + onlyShowAccessibleDiffViewer: validateBooleanOption(options.onlyShowAccessibleDiffViewer, defaults.onlyShowAccessibleDiffViewer), }; } diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts index 82c59dd4ddcc0..86d7edb47b866 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts @@ -66,7 +66,12 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor { private unchangedRangesFeature!: UnchangedRangesFeature; - private _accessibleDiffViewerVisible = observableValue('accessibleDiffViewerVisible', false); + private _accessibleDiffViewerShouldBeVisible = observableValue('accessibleDiffViewerShouldBeVisible', false); + private _accessibleDiffViewerVisible = derived('accessibleDiffViewerVisible', reader => + this._options.onlyShowAccessibleDiffViewer.read(reader) + ? true + : this._accessibleDiffViewerShouldBeVisible.read(reader) + ); private _accessibleDiffViewer!: AccessibleDiffViewer; private readonly _options: DiffEditorOptions; private readonly _editors: DiffEditorEditors; @@ -163,6 +168,8 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor { readHotReloadableExport(AccessibleDiffViewer, reader), this.elements.accessibleDiffViewer, this._accessibleDiffViewerVisible, + (visible, tx) => this._accessibleDiffViewerShouldBeVisible.set(visible, tx), + this._options.onlyShowAccessibleDiffViewer.map(v => !v), this._rootSizeObserver.width, this._rootSizeObserver.height, this._diffModel.map((m, r) => m?.diff.read(r)?.mappings.map(m => m.lineRangeMapping)), diff --git a/src/vs/editor/common/config/editorOptions.ts b/src/vs/editor/common/config/editorOptions.ts index 96dbb05c27523..5f30ccd744d22 100644 --- a/src/vs/editor/common/config/editorOptions.ts +++ b/src/vs/editor/common/config/editorOptions.ts @@ -827,6 +827,11 @@ export interface IDiffEditorBaseOptions { * Defaults to false */ isInEmbeddedEditor?: boolean; + + /** + * If the diff editor should only show the difference review mode. + */ + onlyShowAccessibleDiffViewer?: boolean; } /** diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index dbcada860e609..5011ecde3ccfc 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -3975,6 +3975,10 @@ declare namespace monaco.editor { * Defaults to false */ isInEmbeddedEditor?: boolean; + /** + * If the diff editor should only show the difference review mode. + */ + onlyShowAccessibleDiffViewer?: boolean; } /** diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts index 95fef85528594..341a38fb18111 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts @@ -7,7 +7,7 @@ import { Codicon } from 'vs/base/common/codicons'; import { KeyChord, KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { EditorAction2 } from 'vs/editor/browser/editorExtensions'; -import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget'; +import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget2 } from 'vs/editor/browser/widget/embeddedCodeEditorWidget'; import { EditorContextKeys } from 'vs/editor/common/editorContextKeys'; import { InlineChatController, InlineChatRunOptions } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController'; import { CTX_INLINE_CHAT_FOCUSED, CTX_INLINE_CHAT_HAS_ACTIVE_REQUEST, CTX_INLINE_CHAT_HAS_PROVIDER, CTX_INLINE_CHAT_INNER_CURSOR_FIRST, CTX_INLINE_CHAT_INNER_CURSOR_LAST, CTX_INLINE_CHAT_EMPTY, CTX_INLINE_CHAT_OUTER_CURSOR_POSITION, CTX_INLINE_CHAT_VISIBLE, MENU_INLINE_CHAT_WIDGET, MENU_INLINE_CHAT_WIDGET_DISCARD, MENU_INLINE_CHAT_WIDGET_STATUS, CTX_INLINE_CHAT_LAST_FEEDBACK, CTX_INLINE_CHAT_EDIT_MODE, EditMode, CTX_INLINE_CHAT_LAST_RESPONSE_TYPE, MENU_INLINE_CHAT_WIDGET_MARKDOWN_MESSAGE, CTX_INLINE_CHAT_MESSAGE_CROP_STATE, CTX_INLINE_CHAT_DOCUMENT_CHANGED, CTX_INLINE_CHAT_DID_EDIT, CTX_INLINE_CHAT_HAS_STASHED_SESSION, MENU_INLINE_CHAT_WIDGET_FEEDBACK, ACTION_ACCEPT_CHANGES, ACTION_REGENERATE_RESPONSE, InlineChatResponseType, CTX_INLINE_CHAT_RESPONSE_TYPES, InlineChateResponseTypes, ACTION_VIEW_IN_CHAT, CTX_INLINE_CHAT_USER_DID_EDIT, MENU_INLINE_CHAT_WIDGET_TOGGLE } from 'vs/workbench/contrib/inlineChat/common/inlineChat'; @@ -121,7 +121,7 @@ abstract class AbstractInlineChatAction extends EditorAction2 { if (!ctrl) { for (const diffEditor of accessor.get(ICodeEditorService).listDiffEditors()) { if (diffEditor.getOriginalEditor() === editor || diffEditor.getModifiedEditor() === editor) { - if (diffEditor instanceof EmbeddedDiffEditorWidget) { + if (diffEditor instanceof EmbeddedDiffEditorWidget2) { this.runEditorCommand(accessor, diffEditor.getParentEditor(), ..._args); } } diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatLivePreviewWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatLivePreviewWidget.ts index 3f38c66ae4ea3..a743c224dee84 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatLivePreviewWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatLivePreviewWidget.ts @@ -34,6 +34,7 @@ import { Session } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSessi import { ILanguageService } from 'vs/editor/common/languages/language'; import { FoldingController } from 'vs/editor/contrib/folding/browser/folding'; import { WordHighlighterContribution } from 'vs/editor/contrib/wordHighlighter/browser/wordHighlighter'; +import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; export class InlineChatLivePreviewWidget extends ZoneWidget { @@ -53,6 +54,7 @@ export class InlineChatLivePreviewWidget extends ZoneWidget { @IInstantiationService instantiationService: IInstantiationService, @IThemeService themeService: IThemeService, @ILogService private readonly _logService: ILogService, + @IAccessibilityService private readonly accessibilityService: IAccessibilityService, ) { super(editor, { showArrow: false, showFrame: false, isResizeable: false, isAccessible: true, allowUnlimitedHeight: true, showInHiddenAreas: true, ordinal: 10000 + 1 }); super.create(); @@ -78,7 +80,8 @@ export class InlineChatLivePreviewWidget extends ZoneWidget { stickyScroll: { enabled: false }, minimap: { enabled: false }, isInEmbeddedEditor: true, - overflowWidgetsDomNode: editor.getOverflowWidgetsDomNode() + overflowWidgetsDomNode: editor.getOverflowWidgetsDomNode(), + onlyShowAccessibleDiffViewer: this.accessibilityService.isScreenReaderOptimized(), }, { originalEditor: { contributions: diffContributions }, modifiedEditor: { contributions: diffContributions } diff --git a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts index 4cde6edcb51c8..b0cc6198c47b2 100644 --- a/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts +++ b/src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts @@ -329,7 +329,10 @@ export class InlineChatWidget { this._store.add(feedbackToolbar); // preview editors - this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, _previewEditorEditorOptions, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor))); + this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, { + ..._previewEditorEditorOptions, + onlyShowAccessibleDiffViewer: this._accessibilityService.isScreenReaderOptimized(), + }, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor))); this._previewCreateTitle = this._store.add(_instantiationService.createInstance(ResourceLabel, this._elements.previewCreateTitle, { supportIcons: true })); this._previewCreateEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor))); From c01bc972800eed802e72a0b02fc156bdbcf5a620 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Wed, 19 Jul 2023 01:36:43 +0200 Subject: [PATCH 2/2] Fixes CI --- .../browser/widget/diffEditorWidget2/accessibleDiffViewer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts b/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts index 94c20eb68da9c..cc0f3a5a7a04c 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts @@ -10,7 +10,7 @@ import { Action } from 'vs/base/common/actions'; import { Codicon } from 'vs/base/common/codicons'; import { KeyCode, KeyMod } from 'vs/base/common/keyCodes'; import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle'; -import { IObservable, ISettableObservable, ITransaction, autorun, constObservable, derived, keepAlive, observableValue, transaction } from 'vs/base/common/observable'; +import { IObservable, ITransaction, autorun, derived, keepAlive, observableValue, transaction } from 'vs/base/common/observable'; import { autorunWithStore2 } from 'vs/base/common/observableImpl/autorun'; import { subtransaction } from 'vs/base/common/observableImpl/base'; import { derivedWithStore } from 'vs/base/common/observableImpl/derived';