Skip to content

Commit

Permalink
Get rid of cell focus, refactor context keys, fix #99390
Browse files Browse the repository at this point in the history
  • Loading branch information
roblourens committed Jun 9, 2020
1 parent 315d372 commit 4615200
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 60 deletions.
14 changes: 12 additions & 2 deletions src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts
Expand Up @@ -938,7 +938,12 @@ registerAction2(class extends NotebookAction {
title: localize('cursorMoveDown', 'Cursor Move Down'),
category: NOTEBOOK_ACTIONS_CATEGORY,
keybinding: {
when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, ContextKeyExpr.has(InputFocusedContextKey), EditorContextKeys.editorTextFocus, NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('top'), NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('none')),
when: ContextKeyExpr.and(
NOTEBOOK_EDITOR_FOCUSED,
ContextKeyExpr.has(InputFocusedContextKey),
EditorContextKeys.editorTextFocus,
NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('top'),
NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('none')),
primary: KeyCode.DownArrow,
weight: EDITOR_WIDGET_ACTION_WEIGHT
}
Expand Down Expand Up @@ -971,7 +976,12 @@ registerAction2(class extends NotebookAction {
title: localize('cursorMoveUp', 'Cursor Move Up'),
category: NOTEBOOK_ACTIONS_CATEGORY,
keybinding: {
when: ContextKeyExpr.and(NOTEBOOK_EDITOR_FOCUSED, ContextKeyExpr.has(InputFocusedContextKey), EditorContextKeys.editorTextFocus, NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('bottom'), NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('none')),
when: ContextKeyExpr.and(
NOTEBOOK_EDITOR_FOCUSED,
ContextKeyExpr.has(InputFocusedContextKey),
EditorContextKeys.editorTextFocus,
NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('bottom'),
NOTEBOOK_EDITOR_CURSOR_BOUNDARY.notEqualsTo('none')),
primary: KeyCode.UpArrow,
weight: EDITOR_WIDGET_ACTION_WEIGHT
},
Expand Down
10 changes: 10 additions & 0 deletions src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
Expand Up @@ -50,6 +50,7 @@ import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/no
import { URI } from 'vs/base/common/uri';
import { PANEL_BORDER } from 'vs/workbench/common/theme';
import { debugIconStartForeground } from 'vs/workbench/contrib/debug/browser/debugToolBar';
import { CellContextKeyManager } from 'vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys';

const $ = DOM.$;

Expand Down Expand Up @@ -96,6 +97,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
private _isDisposed: boolean = false;
private readonly _onDidFocusWidget = this._register(new Emitter<void>());
public get onDidFocus(): Event<any> { return this._onDidFocusWidget.event; }
private _cellContextKeyManager: CellContextKeyManager | null = null;

get isDisposed() {
return this._isDisposed;
Expand Down Expand Up @@ -401,6 +403,14 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
}
}));

this.localStore.add(this.list!.onDidChangeFocus(() => {
this._cellContextKeyManager?.dispose();
const focused = this.list!.getFocusedElements()[0];
if (focused) {
this._cellContextKeyManager = this.localStore.add(new CellContextKeyManager(this.contextKeyService, textModel, focused as any));
}
}));

// reveal cell if editor options tell to do so
if (options instanceof NotebookEditorOptions && options.cellOptions) {
const cellOptions = options.cellOptions;
Expand Down
14 changes: 0 additions & 14 deletions src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts
Expand Up @@ -56,7 +56,6 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
@IThemeService themeService: IThemeService,
@IConfigurationService configurationService: IConfigurationService,
@IKeybindingService keybindingService: IKeybindingService

) {
super(listUser, container, delegate, renderers, options, contextKeyService, listService, themeService, configurationService, keybindingService);
this._previousFocusedElements = this.getFocusedElements();
Expand All @@ -67,19 +66,6 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
}
});
this._previousFocusedElements = e.elements;

// if focus is in the list, but is not inside the focused element, then reset focus
setTimeout(() => {
if (DOM.isAncestor(document.activeElement, this.rowsContainer)) {
const focusedElement = this.getFocusedElements()[0];
if (focusedElement) {
const focusedDomElement = this.domElementOfElement(focusedElement);
if (focusedDomElement && !DOM.isAncestor(document.activeElement, focusedDomElement)) {
focusedDomElement.focus();
}
}
}
}, 0);
}));

const notebookEditorCursorAtBoundaryContext = NOTEBOOK_EDITOR_CURSOR_BOUNDARY.bindTo(contextKeyService);
Expand Down
@@ -0,0 +1,97 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { INotebookTextModel, NotebookCellRunState } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { BaseCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel';
import { NOTEBOOK_CELL_TYPE, NOTEBOOK_VIEW_TYPE, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_RUNNABLE, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_RUN_STATE, NOTEBOOK_CELL_HAS_OUTPUTS, CellViewModelStateChangeEvent, CellEditState } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel';
import { MarkdownCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markdownCellViewModel';
import { Disposable } from 'vs/base/common/lifecycle';

export class CellContextKeyManager extends Disposable {

private cellType: IContextKey<string>;
private viewType: IContextKey<string>;
private cellEditable: IContextKey<boolean>;
private cellRunnable: IContextKey<boolean>;
private cellRunState: IContextKey<string>;
private cellHasOutputs: IContextKey<boolean>;

private markdownEditMode: IContextKey<boolean>;

constructor(
private readonly contextKeyService: IContextKeyService,
private readonly notebookTextModel: INotebookTextModel,
private readonly element: BaseCellViewModel
) {
super();

this.cellType = NOTEBOOK_CELL_TYPE.bindTo(this.contextKeyService);
this.viewType = NOTEBOOK_VIEW_TYPE.bindTo(this.contextKeyService);
this.cellEditable = NOTEBOOK_CELL_EDITABLE.bindTo(this.contextKeyService);
this.cellRunnable = NOTEBOOK_CELL_RUNNABLE.bindTo(this.contextKeyService);
this.markdownEditMode = NOTEBOOK_CELL_MARKDOWN_EDIT_MODE.bindTo(this.contextKeyService);
this.cellRunState = NOTEBOOK_CELL_RUN_STATE.bindTo(this.contextKeyService);
this.cellHasOutputs = NOTEBOOK_CELL_HAS_OUTPUTS.bindTo(this.contextKeyService);

this._register(element.onDidChangeState(e => this.onDidChangeState(e)));

if (element instanceof CodeCellViewModel) {
this._register(element.onDidChangeOutputs(() => this.updateForOutputs()));
}

this.initialize();
}

private initialize() {
if (this.element instanceof MarkdownCellViewModel) {
this.cellType.set('markdown');
} else if (this.element instanceof CodeCellViewModel) {
this.cellType.set('code');
}

this.updateForMetadata();
this.updateForEditState();
this.updateForOutputs();

this.viewType.set(this.element.viewType);
}

private onDidChangeState(e: CellViewModelStateChangeEvent) {
if (e.metadataChanged) {
this.updateForMetadata();
}

if (e.editStateChanged) {
this.updateForEditState();
}
}

private updateForMetadata() {
const metadata = this.element.getEvaluatedMetadata(this.notebookTextModel.metadata);
this.cellEditable.set(!!metadata.editable);
this.cellRunnable.set(!!metadata.runnable);

const runState = metadata.runState ?? NotebookCellRunState.Idle;
this.cellRunState.set(NotebookCellRunState[runState]);
}

private updateForEditState() {
if (this.element instanceof MarkdownCellViewModel) {
this.markdownEditMode.set(this.element.editState === CellEditState.Editing);
} else {
this.markdownEditMode.set(false);
}
}

private updateForOutputs() {
if (this.element instanceof CodeCellViewModel) {
this.cellHasOutputs.set(this.element.outputs.length > 0);
} else {
this.cellHasOutputs.set(false);
}
}
}
Expand Up @@ -43,7 +43,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { BOTTOM_CELL_TOOLBAR_HEIGHT, EDITOR_BOTTOM_PADDING, EDITOR_TOOLBAR_HEIGHT, EDITOR_TOP_MARGIN, EDITOR_TOP_PADDING } from 'vs/workbench/contrib/notebook/browser/constants';
import { CancelCellAction, ChangeCellLanguageAction, ExecuteCellAction, INotebookCellActionContext, InsertCodeCellAction, InsertMarkdownCellAction } from 'vs/workbench/contrib/notebook/browser/contrib/coreActions';
import { BaseCellRenderTemplate, CellEditState, CodeCellRenderTemplate, ICellViewModel, INotebookCellList, INotebookEditor, MarkdownCellRenderTemplate, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_RUNNABLE, NOTEBOOK_CELL_RUN_STATE, NOTEBOOK_CELL_TYPE, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { BaseCellRenderTemplate, CellEditState, CodeCellRenderTemplate, ICellViewModel, INotebookCellList, INotebookEditor, MarkdownCellRenderTemplate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { CellMenus } from 'vs/workbench/contrib/notebook/browser/view/renderers/cellMenus';
import { CodeCell } from 'vs/workbench/contrib/notebook/browser/view/renderers/codeCell';
import { StatefullMarkdownCell } from 'vs/workbench/contrib/notebook/browser/view/renderers/markdownCell';
Expand All @@ -52,6 +52,7 @@ import { MarkdownCellViewModel } from 'vs/workbench/contrib/notebook/browser/vie
import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel';
import { CellKind, NotebookCellRunState, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { containsDragType } from 'vs/workbench/browser/dnd';
import { CellContextKeyManager } from 'vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys';

const $ = DOM.$;

Expand Down Expand Up @@ -364,7 +365,6 @@ export class MarkdownCellRenderer extends AbstractCellRenderer implements IListR
}

renderTemplate(container: HTMLElement): MarkdownCellRenderTemplate {
container.tabIndex = -1;
container.classList.add('markdown-cell-row');
const disposables = new DisposableStore();
const contextKeyService = disposables.add(this.contextKeyServiceProvider(container));
Expand Down Expand Up @@ -446,6 +446,8 @@ export class MarkdownCellRenderer extends AbstractCellRenderer implements IListR
if (height) {
const elementDisposables = templateData.elementDisposables;

elementDisposables.add(new CellContextKeyManager(templateData.contextKeyService, this.notebookEditor.viewModel?.notebookDocument!, element));

// render toolbar first
this.setupCellToolbarActions(templateData.contextKeyService, templateData, elementDisposables);

Expand All @@ -462,31 +464,6 @@ export class MarkdownCellRenderer extends AbstractCellRenderer implements IListR
elementDisposables.add(this.editorOptions.onDidChange(newValue => markdownCell.updateEditorOptions(newValue)));
elementDisposables.add(markdownCell);

NOTEBOOK_CELL_TYPE.bindTo(templateData.contextKeyService).set('markdown');
NOTEBOOK_VIEW_TYPE.bindTo(templateData.contextKeyService).set(element.viewType);
const metadata = element.getEvaluatedMetadata(this.notebookEditor.viewModel!.notebookDocument.metadata);
const cellEditableKey = NOTEBOOK_CELL_EDITABLE.bindTo(templateData.contextKeyService);
cellEditableKey.set(!!metadata.editable);
const updateForMetadata = () => {
const metadata = element.getEvaluatedMetadata(this.notebookEditor.viewModel!.notebookDocument.metadata);
cellEditableKey.set(!!metadata.editable);
};

updateForMetadata();
elementDisposables.add(element.onDidChangeState((e) => {
if (e.metadataChanged) {
updateForMetadata();
}
}));

const editModeKey = NOTEBOOK_CELL_MARKDOWN_EDIT_MODE.bindTo(templateData.contextKeyService);
editModeKey.set(element.editState === CellEditState.Editing);
elementDisposables.add(element.onDidChangeState((e) => {
if (e.editStateChanged) {
editModeKey.set(element.editState === CellEditState.Editing);
}
}));

element.totalHeight = height;

templateData.languageStatusBarItem.update(element, this.notebookEditor);
Expand Down Expand Up @@ -919,7 +896,6 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende

renderTemplate(container: HTMLElement): CodeCellRenderTemplate {
container.classList.add('code-cell-row');
container.tabIndex = -1;
const disposables = new DisposableStore();
const contextKeyService = disposables.add(this.contextKeyServiceProvider(container));
const toolbar = disposables.add(this.createToolbar(container));
Expand Down Expand Up @@ -1003,8 +979,6 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
runState = NotebookCellRunState.Idle;
}

const runStateKey = NOTEBOOK_CELL_RUN_STATE.bindTo(templateData.contextKeyService);
runStateKey.set(NotebookCellRunState[runState]);
if (runState === NotebookCellRunState.Running) {
templateData.progressBar.infinite().show(500);

Expand All @@ -1021,14 +995,9 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
}

private updateForMetadata(element: CodeCellViewModel, templateData: CodeCellRenderTemplate): void {
const cellEditableKey = NOTEBOOK_CELL_EDITABLE.bindTo(templateData.contextKeyService);
const cellRunnableKey = NOTEBOOK_CELL_RUNNABLE.bindTo(templateData.contextKeyService);

const metadata = element.getEvaluatedMetadata(this.notebookEditor.viewModel!.notebookDocument.metadata);
DOM.toggleClass(templateData.cellContainer, 'runnable', !!metadata.runnable);
this.updateExecutionOrder(metadata, templateData);
cellEditableKey.set(!!metadata.editable);
cellRunnableKey.set(!!metadata.runnable);
templateData.cellStatusMessageContainer.textContent = metadata?.statusMessage || '';

if (metadata.runState === NotebookCellRunState.Success) {
Expand Down Expand Up @@ -1091,19 +1060,13 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende
elementDisposables.add(this.instantiationService.createInstance(CodeCell, this.notebookEditor, element, templateData));
this.renderedEditors.set(element, templateData.editor);

elementDisposables.add(new CellContextKeyManager(templateData.contextKeyService, this.notebookEditor.viewModel?.notebookDocument!, element));

templateData.focusIndicator.style.height = `${element.layoutInfo.indicatorHeight}px`;
elementDisposables.add(element.onDidChangeLayout(() => {
templateData.focusIndicator.style.height = `${element.layoutInfo.indicatorHeight}px`;
}));

const cellHasOutputsContext = NOTEBOOK_CELL_HAS_OUTPUTS.bindTo(templateData.contextKeyService);
cellHasOutputsContext.set(element.outputs.length > 0);
elementDisposables.add(element.onDidChangeOutputs(() => {
cellHasOutputsContext.set(element.outputs.length > 0);
}));

NOTEBOOK_CELL_TYPE.bindTo(templateData.contextKeyService).set('code');
NOTEBOOK_VIEW_TYPE.bindTo(templateData.contextKeyService).set(element.viewType);
this.updateForMetadata(element, templateData);
elementDisposables.add(element.onDidChangeState((e) => {
if (e.metadataChanged) {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/notebook/common/notebookCommon.ts
Expand Up @@ -250,7 +250,7 @@ export interface IMetadata {
export interface INotebookTextModel {
handle: number;
viewType: string;
// metadata: IMetadata;
metadata: NotebookDocumentMetadata
readonly uri: URI;
readonly versionId: number;
languages: string[];
Expand Down

0 comments on commit 4615200

Please sign in to comment.