From 4615200c6113dfea647bf50575fa44d26907e814 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 8 Jun 2020 21:14:47 -0500 Subject: [PATCH] Get rid of cell focus, refactor context keys, fix #99390 --- .../notebook/browser/contrib/coreActions.ts | 14 ++- .../notebook/browser/notebookEditorWidget.ts | 10 ++ .../notebook/browser/view/notebookCellList.ts | 14 --- .../browser/view/renderers/cellContextKeys.ts | 97 +++++++++++++++++++ .../browser/view/renderers/cellRenderer.ts | 49 ++-------- .../contrib/notebook/common/notebookCommon.ts | 2 +- 6 files changed, 126 insertions(+), 60 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys.ts diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts index bbc008c7272ee..9562993708668 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/coreActions.ts @@ -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 } @@ -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 }, diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index dda12df9cb692..d7351babd5aad 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -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.$; @@ -96,6 +97,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor private _isDisposed: boolean = false; private readonly _onDidFocusWidget = this._register(new Emitter()); public get onDidFocus(): Event { return this._onDidFocusWidget.event; } + private _cellContextKeyManager: CellContextKeyManager | null = null; get isDisposed() { return this._isDisposed; @@ -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; diff --git a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts index 3917867db21d8..52a85ed40c266 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts @@ -56,7 +56,6 @@ export class NotebookCellList extends WorkbenchList 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(); @@ -67,19 +66,6 @@ export class NotebookCellList extends WorkbenchList 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); diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys.ts new file mode 100644 index 0000000000000..8626aadcf6f65 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellContextKeys.ts @@ -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; + private viewType: IContextKey; + private cellEditable: IContextKey; + private cellRunnable: IContextKey; + private cellRunState: IContextKey; + private cellHasOutputs: IContextKey; + + private markdownEditMode: IContextKey; + + 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); + } + } +} diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts index 4c7b746806178..c3e97c6651607 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts @@ -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'; @@ -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.$; @@ -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)); @@ -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); @@ -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); @@ -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)); @@ -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); @@ -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) { @@ -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) { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 4d7a8cb908859..9ef565a5608b2 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -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[];