Skip to content

Commit

Permalink
Merge pull request #188181 from microsoft/hediet/b/experimental-sailfish
Browse files Browse the repository at this point in the history
Introduces onlyShowAccessibleDiffViewer. Fixes #182789
  • Loading branch information
hediet authored Jul 19, 2023
2 parents 4789314 + c01bc97 commit 13c4597
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 24 deletions.
2 changes: 2 additions & 0 deletions src/vs/editor/browser/widget/diffEditorWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -2743,6 +2744,7 @@ function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaul
collapseUnchangedRegions: false,
},
isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor),
onlyShowAccessibleDiffViewer: false,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<boolean>,
private readonly _visible: IObservable<boolean>,
private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void,
private readonly _canClose: IObservable<boolean>,
private readonly _width: IObservable<number>,
private readonly _height: IObservable<number>,
private readonly _diffs: IObservable<LineRangeMapping[] | undefined>,
Expand All @@ -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,
Expand All @@ -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);
}
Expand All @@ -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);
});
}
}
Expand All @@ -104,12 +106,11 @@ class ViewModel extends Disposable {
public readonly currentElement: IObservable<ViewElement | undefined>
= this._currentElementIdx.map((idx, r) => this.currentGroup.read(r)?.lines[idx]);

public readonly canClose: IObservable<boolean> = constObservable(true);

constructor(
private readonly _diffs: IObservable<LineRangeMapping[] | undefined>,
private readonly _editors: DiffEditorEditors,
private readonly _visible: ISettableObservable<boolean>,
private readonly _setVisible: (visible: boolean, tx: ITransaction | undefined) => void,
public readonly canClose: IObservable<boolean>,
@IAudioCueService private readonly _audioCueService: IAudioCueService,
) {
super();
Expand Down Expand Up @@ -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) {
Expand All @@ -211,7 +212,7 @@ class ViewModel extends Disposable {
}

close(): void {
this._visible.set(false, undefined);
this._setVisible(false, undefined);
this._editors.modified.focus();
}
}
Expand Down Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -75,6 +76,7 @@ const diffEditorDefaultOptions: ValidDiffEditorBaseOptions = {
showEmptyDecorations: true,
},
isInEmbeddedEditor: false,
onlyShowAccessibleDiffViewer: false,
};

function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaults: ValidDiffEditorBaseOptions): ValidDiffEditorBaseOptions {
Expand All @@ -99,5 +101,6 @@ function validateDiffEditorOptions(options: Readonly<IDiffEditorOptions>, defaul
showEmptyDecorations: validateBooleanOption(options.experimental?.showEmptyDecorations, defaults.experimental.showEmptyDecorations!),
},
isInEmbeddedEditor: validateBooleanOption(options.isInEmbeddedEditor, defaults.isInEmbeddedEditor),
onlyShowAccessibleDiffViewer: validateBooleanOption(options.onlyShowAccessibleDiffViewer, defaults.onlyShowAccessibleDiffViewer),
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)),
Expand Down
5 changes: 5 additions & 0 deletions src/vs/editor/common/config/editorOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/vs/monaco.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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();
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down

0 comments on commit 13c4597

Please sign in to comment.