From 31cd1ee042f2fe89ed71738c797974ed3faad1ca Mon Sep 17 00:00:00 2001 From: aamunger Date: Fri, 21 Apr 2023 13:26:35 -0700 Subject: [PATCH] ensure focus is set correctly in output, enable keyboard scrolling for outputs --- extensions/notebook-renderers/src/index.ts | 18 ++++++++++++++++++ .../notebook/browser/controller/editActions.ts | 7 ++++++- .../browser/view/renderers/webviewPreloads.ts | 14 ++++++++++---- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 973e65b3f9a..e5e015470db 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -196,6 +196,17 @@ function onScrollHandler(e: globalThis.Event) { } else { target.classList.add('more-above'); } + e.stopPropagation(); +} + +function onKeypressHandler(e: KeyboardEvent) { + if (e.ctrlKey || e.shiftKey) { + return; + } + if (e.code === 'ArrowDown' || e.code === 'End' || e.code === 'ArrowUp' || e.code === 'Home') { + // These should change the scroll position, not adjust the selected cell in the notebook + e.stopPropagation(); + } } // if there is a scrollable output, it will be scrolled to the given value if provided or the bottom of the element @@ -205,6 +216,9 @@ function initializeScroll(scrollableElement: HTMLElement, disposables: Disposabl scrollableElement.scrollTop = scrollTop !== undefined ? scrollTop : scrollableElement.scrollHeight; scrollableElement.addEventListener('scroll', onScrollHandler); disposables.push({ dispose: () => scrollableElement.removeEventListener('scroll', onScrollHandler) }); + scrollableElement.addEventListener('keydown', onKeypressHandler); + disposables.push({ dispose: () => scrollableElement.removeEventListener('keydown', onKeypressHandler) }); + scrollableElement.tabIndex = 0; } } @@ -351,6 +365,10 @@ export const activate: ActivationFunction = (ctx) => { #container div.output .scrollable.scrollbar-visible { border-color: var(--vscode-editorWidget-border); } + #container div.output .scrollable.scrollbar-visible:focus{ + outline: 0; + border-color: var(--theme-input-focus-border-color); + } .output-plaintext .code-bold, .output-stream .code-bold, .traceback .code-bold { diff --git a/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts index 8ebc9a8b5b4..14287b62b8e 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts @@ -19,7 +19,7 @@ import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegis import { IQuickInputService, IQuickPickItem, QuickPickInput } from 'vs/platform/quickinput/common/quickInput'; import { changeCellToKind, runDeleteAction } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; import { CellToolbarOrder, CELL_TITLE_CELL_GROUP_ID, CELL_TITLE_OUTPUT_GROUP_ID, executeNotebookCondition, INotebookActionContext, INotebookCellActionContext, NotebookAction, NotebookCellAction, NOTEBOOK_EDITOR_WIDGET_ACTION_WEIGHT } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; -import { NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; +import { NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_TYPE, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_OUTPUT_FOCUSED, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { CellEditState, CHANGE_CELL_LANGUAGE, DETECT_CELL_LANGUAGE, QUIT_EDIT_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { CellEditType, CellKind, ICellEditOperation, NotebookCellExecutionState, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -103,6 +103,11 @@ registerAction2(class QuitEditCellAction extends NotebookCellAction { primary: KeyCode.Escape, weight: NOTEBOOK_EDITOR_WIDGET_ACTION_WEIGHT - 5 }, + { + when: ContextKeyExpr.and(NOTEBOOK_OUTPUT_FOCUSED, NOTEBOOK_EDITOR_FOCUSED), + primary: KeyCode.Escape, + weight: NOTEBOOK_EDITOR_WIDGET_ACTION_WEIGHT - 5 + }, { when: ContextKeyExpr.and( quitEditCondition, diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index 4ea778aaefe..a9d374d9b55 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -453,15 +453,20 @@ async function webviewPreloads(ctx: PreloadContext) { }); }; - function focusFirstFocusableInCell(cellId: string) { + function focusFirstFocusableOrContainerInOutput(cellId: string) { const cellOutputContainer = document.getElementById(cellId); if (cellOutputContainer) { if (cellOutputContainer.contains(document.activeElement)) { return; } - const focusableElement = cellOutputContainer.querySelector('[tabindex="0"], [href], button, input, option, select, textarea') as HTMLElement | null; - focusableElement?.focus(); + let focusableElement = cellOutputContainer.querySelector('[tabindex="0"], [href], button, input, option, select, textarea') as HTMLElement | null; + if (!focusableElement) { + focusableElement = cellOutputContainer; + focusableElement.tabIndex = -1; + } + + focusableElement.focus(); } } @@ -1357,7 +1362,7 @@ async function webviewPreloads(ctx: PreloadContext) { break; } case 'focus-output': - focusFirstFocusableInCell(event.data.cellId); + focusFirstFocusableOrContainerInOutput(event.data.cellId); break; case 'decorations': { let outputContainer = document.getElementById(event.data.cellId); @@ -2283,6 +2288,7 @@ async function webviewPreloads(ctx: PreloadContext) { this.element = document.createElement('div'); this.element.style.position = 'absolute'; + this.element.style.outline = '0'; this.element.id = cellId; this.element.classList.add('cell_container');