Skip to content

Commit 13c4597

Browse files
hediethediet
authored
Merge pull request microsoft#188181 from microsoft/hediet/b/experimental-sailfish
Introduces onlyShowAccessibleDiffViewer. Fixes microsoft#182789
2 parents 4789314 + c01bc97 commit 13c4597

File tree

9 files changed

+56
-24
lines changed

9 files changed

+56
-24
lines changed

‎src/vs/editor/browser/widget/diffEditorWidget.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
299299
collapseUnchangedRegions: false,
300300
},
301301
isInEmbeddedEditor: false,
302+
onlyShowAccessibleDiffViewer: false,
302303
});
303304

304305
this.isEmbeddedDiffEditorKey = EditorContextKeys.isEmbeddedDiffEditor.bindTo(this._contextKeyService);
@@ -2743,6 +2744,7 @@ function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaul
27432744
collapseUnchangedRegions: false,
27442745
},
27452746
isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor),
2747+
onlyShowAccessibleDiffViewer: false,
27462748
};
27472749
}
27482750

‎src/vs/editor/browser/widget/diffEditorWidget2/accessibleDiffViewer.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Action } from 'vs/base/common/actions';
1010
import { Codicon } from 'vs/base/common/codicons';
1111
import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
1212
import { Disposable, DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
13-
import { IObservable, ISettableObservable, ITransaction, autorun, constObservable, derived, keepAlive, observableValue, transaction } from 'vs/base/common/observable';
13+
import { IObservable, ITransaction, autorun, derived, keepAlive, observableValue, transaction } from 'vs/base/common/observable';
1414
import { autorunWithStore2 } from 'vs/base/common/observableImpl/autorun';
1515
import { subtransaction } from 'vs/base/common/observableImpl/base';
1616
import { derivedWithStore } from 'vs/base/common/observableImpl/derived';
@@ -42,7 +42,9 @@ const diffReviewCloseIcon = registerIcon('diff-review-close', Codicon.close, loc
4242
export class AccessibleDiffViewer extends Disposable {
4343
constructor(
4444
private readonly _parentNode: HTMLElement,
45-
private readonly _visible: ISettableObservable<boolean>,
45+
private readonly _visible: IObservable<boolean>,
46+
private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void,
47+
private readonly _canClose: IObservable<boolean>,
4648
private readonly _width: IObservable<number>,
4749
private readonly _height: IObservable<number>,
4850
private readonly _diffs: IObservable<LineRangeMapping[] | undefined>,
@@ -59,7 +61,7 @@ export class AccessibleDiffViewer extends Disposable {
5961
if (!visible) {
6062
return null;
6163
}
62-
const model = store.add(this._instantiationService.createInstance(ViewModel, this._diffs, this._editors, this._visible));
64+
const model = store.add(this._instantiationService.createInstance(ViewModel, this._diffs, this._editors, this._setVisible, this._canClose));
6365
const view = store.add(this._instantiationService.createInstance(View, this._parentNode, model, this._width, this._height, this._editors));
6466
return {
6567
model,
@@ -70,7 +72,7 @@ export class AccessibleDiffViewer extends Disposable {
7072
next(): void {
7173
transaction(tx => {
7274
const isVisible = this._visible.get();
73-
this._visible.set(true, tx);
75+
this._setVisible(true, tx);
7476
if (isVisible) {
7577
this.model.get()!.model.nextGroup(tx);
7678
}
@@ -79,14 +81,14 @@ export class AccessibleDiffViewer extends Disposable {
7981

8082
prev(): void {
8183
transaction(tx => {
82-
this._visible.set(true, tx);
84+
this._setVisible(true, tx);
8385
this.model.get()!.model.previousGroup(tx);
8486
});
8587
}
8688

8789
close(): void {
8890
transaction(tx => {
89-
this._visible.set(false, tx);
91+
this._setVisible(false, tx);
9092
});
9193
}
9294
}
@@ -104,12 +106,11 @@ class ViewModel extends Disposable {
104106
public readonly currentElement: IObservable<ViewElement | undefined>
105107
= this._currentElementIdx.map((idx, r) => this.currentGroup.read(r)?.lines[idx]);
106108

107-
public readonly canClose: IObservable<boolean> = constObservable(true);
108-
109109
constructor(
110110
private readonly _diffs: IObservable<LineRangeMapping[] | undefined>,
111111
private readonly _editors: DiffEditorEditors,
112-
private readonly _visible: ISettableObservable<boolean>,
112+
private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void,
113+
public readonly canClose: IObservable<boolean>,
113114
@IAudioCueService private readonly _audioCueService: IAudioCueService,
114115
) {
115116
super();
@@ -192,7 +193,7 @@ class ViewModel extends Disposable {
192193
}
193194

194195
revealCurrentElementInEditor(): void {
195-
this._visible.set(false, undefined);
196+
this._setVisible(false, undefined);
196197

197198
const curElem = this.currentElement.get();
198199
if (curElem) {
@@ -211,7 +212,7 @@ class ViewModel extends Disposable {
211212
}
212213

213214
close(): void {
214-
this._visible.set(false, undefined);
215+
this._setVisible(false, undefined);
215216
this._editors.modified.focus();
216217
}
217218
}
@@ -338,14 +339,18 @@ class View extends Disposable {
338339
this._actionBar = this._register(new ActionBar(
339340
actionBarContainer
340341
));
341-
this._actionBar.push(new Action(
342-
'diffreview.close',
343-
localize('label.close', "Close"),
344-
'close-diff-review ' + ThemeIcon.asClassName(diffReviewCloseIcon),
345-
true,
346-
async () => _model.close()
347-
), { label: false, icon: true });
348-
342+
this._register(autorun('update actions', reader => {
343+
this._actionBar.clear();
344+
if (this._model.canClose.read(reader)) {
345+
this._actionBar.push(new Action(
346+
'diffreview.close',
347+
localize('label.close', "Close"),
348+
'close-diff-review ' + ThemeIcon.asClassName(diffReviewCloseIcon),
349+
true,
350+
async () => _model.close()
351+
), { label: false, icon: true });
352+
}
353+
}));
349354

350355
this._content = document.createElement('div');
351356
this._content.className = 'diff-review-content';

‎src/vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export class DiffEditorOptions {
4646
public readonly accessibilityVerbose = derived('accessibilityVerbose', reader => this._options.read(reader).accessibilityVerbose);
4747
public readonly diffAlgorithm = derived('diffAlgorithm', reader => this._options.read(reader).diffAlgorithm);
4848
public readonly showEmptyDecorations = derived('showEmptyDecorations', reader => this._options.read(reader).experimental.showEmptyDecorations!);
49+
public readonly onlyShowAccessibleDiffViewer = derived('onlyShowAccessibleDiffViewer', reader => this._options.read(reader).onlyShowAccessibleDiffViewer);
4950

5051
public updateOptions(changedOptions: IDiffEditorOptions): void {
5152
const newDiffEditorOptions = validateDiffEditorOptions(changedOptions, this._options.get());
@@ -75,6 +76,7 @@ const diffEditorDefaultOptions: ValidDiffEditorBaseOptions = {
7576
showEmptyDecorations: true,
7677
},
7778
isInEmbeddedEditor: false,
79+
onlyShowAccessibleDiffViewer: false,
7880
};
7981

8082
function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaults: ValidDiffEditorBaseOptions): ValidDiffEditorBaseOptions {
@@ -99,5 +101,6 @@ function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaul
99101
showEmptyDecorations: validateBooleanOption(options.experimental?.showEmptyDecorations, defaults.experimental.showEmptyDecorations!),
100102
},
101103
isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor),
104+
onlyShowAccessibleDiffViewer: validateBooleanOption(options.onlyShowAccessibleDiffViewer, defaults.onlyShowAccessibleDiffViewer),
102105
};
103106
}

‎src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
6666

6767
private unchangedRangesFeature!: UnchangedRangesFeature;
6868

69-
private _accessibleDiffViewerVisible = observableValue('accessibleDiffViewerVisible', false);
69+
private _accessibleDiffViewerShouldBeVisible = observableValue('accessibleDiffViewerShouldBeVisible', false);
70+
private _accessibleDiffViewerVisible = derived('accessibleDiffViewerVisible', reader =>
71+
this._options.onlyShowAccessibleDiffViewer.read(reader)
72+
? true
73+
: this._accessibleDiffViewerShouldBeVisible.read(reader)
74+
);
7075
private _accessibleDiffViewer!: AccessibleDiffViewer;
7176
private readonly _options: DiffEditorOptions;
7277
private readonly _editors: DiffEditorEditors;
@@ -163,6 +168,8 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
163168
readHotReloadableExport(AccessibleDiffViewer, reader),
164169
this.elements.accessibleDiffViewer,
165170
this._accessibleDiffViewerVisible,
171+
(visible, tx) => this._accessibleDiffViewerShouldBeVisible.set(visible, tx),
172+
this._options.onlyShowAccessibleDiffViewer.map(v => !v),
166173
this._rootSizeObserver.width,
167174
this._rootSizeObserver.height,
168175
this._diffModel.map((m, r) => m?.diff.read(r)?.mappings.map(m => m.lineRangeMapping)),

‎src/vs/editor/common/config/editorOptions.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,11 @@ export interface IDiffEditorBaseOptions {
827827
* Defaults to false
828828
*/
829829
isInEmbeddedEditor?: boolean;
830+
831+
/**
832+
* If the diff editor should only show the difference review mode.
833+
*/
834+
onlyShowAccessibleDiffViewer?: boolean;
830835
}
831836

832837
/**

‎src/vs/monaco.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3975,6 +3975,10 @@ declare namespace monaco.editor {
39753975
* Defaults to false
39763976
*/
39773977
isInEmbeddedEditor?: boolean;
3978+
/**
3979+
* If the diff editor should only show the difference review mode.
3980+
*/
3981+
onlyShowAccessibleDiffViewer?: boolean;
39783982
}
39793983

39803984
/**

‎src/vs/workbench/contrib/inlineChat/browser/inlineChatActions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Codicon } from 'vs/base/common/codicons';
77
import { KeyChord, KeyCode, KeyMod } from 'vs/base/common/keyCodes';
88
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
99
import { EditorAction2 } from 'vs/editor/browser/editorExtensions';
10-
import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
10+
import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget2 } from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
1111
import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
1212
import { InlineChatController, InlineChatRunOptions } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController';
1313
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 {
121121
if (!ctrl) {
122122
for (const diffEditor of accessor.get(ICodeEditorService).listDiffEditors()) {
123123
if (diffEditor.getOriginalEditor() === editor || diffEditor.getModifiedEditor() === editor) {
124-
if (diffEditor instanceof EmbeddedDiffEditorWidget) {
124+
if (diffEditor instanceof EmbeddedDiffEditorWidget2) {
125125
this.runEditorCommand(accessor, diffEditor.getParentEditor(), ..._args);
126126
}
127127
}

‎src/vs/workbench/contrib/inlineChat/browser/inlineChatLivePreviewWidget.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { Session } from 'vs/workbench/contrib/inlineChat/browser/inlineChatSessi
3434
import { ILanguageService } from 'vs/editor/common/languages/language';
3535
import { FoldingController } from 'vs/editor/contrib/folding/browser/folding';
3636
import { WordHighlighterContribution } from 'vs/editor/contrib/wordHighlighter/browser/wordHighlighter';
37+
import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility';
3738

3839
export class InlineChatLivePreviewWidget extends ZoneWidget {
3940

@@ -53,6 +54,7 @@ export class InlineChatLivePreviewWidget extends ZoneWidget {
5354
@IInstantiationService instantiationService: IInstantiationService,
5455
@IThemeService themeService: IThemeService,
5556
@ILogService private readonly _logService: ILogService,
57+
@IAccessibilityService private readonly accessibilityService: IAccessibilityService,
5658
) {
5759
super(editor, { showArrow: false, showFrame: false, isResizeable: false, isAccessible: true, allowUnlimitedHeight: true, showInHiddenAreas: true, ordinal: 10000 + 1 });
5860
super.create();
@@ -78,7 +80,8 @@ export class InlineChatLivePreviewWidget extends ZoneWidget {
7880
stickyScroll: { enabled: false },
7981
minimap: { enabled: false },
8082
isInEmbeddedEditor: true,
81-
overflowWidgetsDomNode: editor.getOverflowWidgetsDomNode()
83+
overflowWidgetsDomNode: editor.getOverflowWidgetsDomNode(),
84+
onlyShowAccessibleDiffViewer: this.accessibilityService.isScreenReaderOptimized(),
8285
}, {
8386
originalEditor: { contributions: diffContributions },
8487
modifiedEditor: { contributions: diffContributions }

‎src/vs/workbench/contrib/inlineChat/browser/inlineChatWidget.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,10 @@ export class InlineChatWidget {
329329
this._store.add(feedbackToolbar);
330330

331331
// preview editors
332-
this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, _previewEditorEditorOptions, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor)));
332+
this._previewDiffEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedDiffEditorWidget2, this._elements.previewDiff, {
333+
..._previewEditorEditorOptions,
334+
onlyShowAccessibleDiffViewer: this._accessibilityService.isScreenReaderOptimized(),
335+
}, { modifiedEditor: codeEditorWidgetOptions, originalEditor: codeEditorWidgetOptions }, parentEditor)));
333336

334337
this._previewCreateTitle = this._store.add(_instantiationService.createInstance(ResourceLabel, this._elements.previewCreateTitle, { supportIcons: true }));
335338
this._previewCreateEditor = new IdleValue(() => this._store.add(_instantiationService.createInstance(EmbeddedCodeEditorWidget, this._elements.previewCreate, _previewEditorEditorOptions, codeEditorWidgetOptions, parentEditor)));

0 commit comments

Comments
 (0)