Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix notebook output scrolling and text rendering #14016

Merged
merged 2 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/core/src/browser/style/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ blockquote {
-o-user-select: none;
}

:focus {
/* Since an iframe has its own focus tracking, we don't show focus on iframes */
:focus:not(iframe) {
outline-width: 1px;
outline-style: solid;
outline-offset: -1px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ export function changeCellType(notebookModel: NotebookModel, cell: NotebookCellM
if (cell.cellKind === type) {
return;
}
if (type === CellKind.Markup) {
language = 'markdown';
} else {
language ??= cell.language;
}
notebookModel.applyEdits([{
editType: CellEditType.Replace,
index: notebookModel.cells.indexOf(cell),
count: 1,
cells: [{
...cell.getData(),
cellKind: type,
language: language ?? cell.language
language
}]
}], true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ export class NotebookActionsContribution implements CommandContribution, MenuCon

let cellLanguage: string = 'markdown';
if (cellKind === CellKind.Code) {
const firstCodeCell = notebookModel.cells.find(cell => cell.cellKind === CellKind.Code);
cellLanguage = firstCodeCell?.language ?? 'plaintext';
cellLanguage = this.notebookService.getCodeCellLanguage(notebookModel);
}

notebookModel.applyEdits([{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { NotebookEditorWidgetService } from '../service/notebook-editor-widget-s
import { NotebookCommands } from './notebook-actions-contribution';
import { changeCellType } from './cell-operations';
import { EditorLanguageQuickPickService } from '@theia/editor/lib/browser/editor-language-quick-pick-service';
import { NotebookService } from '../service/notebook-service';

export namespace NotebookCellCommands {
/** Parameters: notebookModel: NotebookModel | undefined, cell: NotebookCellModel */
Expand Down Expand Up @@ -118,7 +119,7 @@ export namespace NotebookCellCommands {

export const TO_MARKDOWN_CELL_COMMAND = Command.toLocalizedCommand({
id: 'notebook.cell.changeToMarkdown',
label: 'Change Cell to Mardown'
label: 'Change Cell to Markdown'
});

export const TOGGLE_CELL_OUTPUT = Command.toDefaultLocalizedCommand({
Expand Down Expand Up @@ -147,6 +148,9 @@ export class NotebookCellActionContribution implements MenuContribution, Command
@inject(ContextKeyService)
protected contextKeyService: ContextKeyService;

@inject(NotebookService)
protected notebookService: NotebookService;

@inject(NotebookExecutionService)
protected notebookExecutionService: NotebookExecutionService;

Expand Down Expand Up @@ -346,7 +350,7 @@ export class NotebookCellActionContribution implements MenuContribution, Command
commands.registerCommand(NotebookCellCommands.INSERT_MARKDOWN_CELL_BELOW_COMMAND, insertCommand(CellKind.Markup, 'below'));

commands.registerCommand(NotebookCellCommands.TO_CODE_CELL_COMMAND, this.editableCellCommandHandler((notebookModel, cell) => {
changeCellType(notebookModel, cell, CellKind.Code);
changeCellType(notebookModel, cell, CellKind.Code, this.notebookService.getCodeCellLanguage(notebookModel));
}));
commands.registerCommand(NotebookCellCommands.TO_MARKDOWN_CELL_COMMAND, this.editableCellCommandHandler((notebookModel, cell) => {
changeCellType(notebookModel, cell, CellKind.Markup);
Expand Down Expand Up @@ -376,9 +380,7 @@ export class NotebookCellActionContribution implements MenuContribution, Command
const isMarkdownCell = selectedCell.cellKind === CellKind.Markup;
const isMarkdownLanguage = language.value.id === 'markdown';
if (isMarkdownLanguage) {
if (!isMarkdownCell) {
changeCellType(activeNotebook, selectedCell, CellKind.Markup, language.value.id);
}
changeCellType(activeNotebook, selectedCell, CellKind.Markup, language.value.id);
} else {
if (isMarkdownCell) {
changeCellType(activeNotebook, selectedCell, CellKind.Code, language.value.id);
Expand Down
3 changes: 2 additions & 1 deletion packages/notebook/src/browser/service/notebook-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const notebookOutputOptionsRelevantPreferences = [

export interface NotebookOutputOptions {
// readonly outputNodePadding: number;
// readonly outputNodeLeftPadding: number;
readonly outputNodeLeftPadding: number;
// readonly previewNodePadding: number;
// readonly markdownLeftMargin: number;
// readonly leftMargin: number;
Expand Down Expand Up @@ -95,6 +95,7 @@ export class NotebookOptionsService {
fontSize,
outputFontSize: outputFontSize,
fontFamily: this.preferenceService.get<string>('editor.fontFamily')!,
outputNodeLeftPadding: 8,
outputFontFamily: this.getNotebookPreferenceWithDefault<string>(NotebookPreferences.OUTPUT_FONT_FAMILY),
outputLineHeight: this.computeOutputLineHeight(outputLineHeight, outputFontSize ?? fontSize),
outputScrolling: this.getNotebookPreferenceWithDefault<boolean>(NotebookPreferences.OUTPUT_SCROLLING)!,
Expand Down
8 changes: 7 additions & 1 deletion packages/notebook/src/browser/service/notebook-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { Disposable, DisposableCollection, Emitter, Resource, URI } from '@theia/core';
import { inject, injectable } from '@theia/core/shared/inversify';
import { BinaryBuffer } from '@theia/core/lib/common/buffer';
import { NotebookData, TransientOptions } from '../../common';
import { CellKind, NotebookData, TransientOptions } from '../../common';
import { NotebookModel, NotebookModelFactory, NotebookModelProps } from '../view-model/notebook-model';
import { FileService } from '@theia/filesystem/lib/browser/file-service';
import { NotebookCellModel, NotebookCellModelFactory, NotebookCellModelProps } from '../view-model/notebook-cell-model';
Expand Down Expand Up @@ -206,4 +206,10 @@ export class NotebookService implements Disposable {
return false;
}
}

getCodeCellLanguage(model: NotebookModel): string {
const firstCodeCell = model.cells.find(cellModel => cellModel.cellKind === CellKind.Code);
const cellLanguage = firstCodeCell?.language ?? 'plaintext';
return cellLanguage;
}
}
26 changes: 15 additions & 11 deletions packages/notebook/src/browser/style/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
margin: 10px 0px;
}

.theia-notebook-cell:focus {
outline: none;
}

.theia-notebook-cell.draggable {
cursor: grab;
}
Expand Down Expand Up @@ -85,7 +89,7 @@

/* Markdown cell edit mode */
.theia-notebook-cell-content:has(.theia-notebook-markdown-editor-container>.theia-notebook-cell-editor) {
margin-left: 37px;
margin-left: 36px;
margin-right: var(--theia-notebook-cell-editor-margin-right);
outline: 1px solid var(--theia-notebook-cellBorderColor);
}
Expand All @@ -107,7 +111,7 @@
width: calc(100% - 46px);
flex: 1;
outline: 1px solid var(--theia-notebook-cellBorderColor);
margin: 0px 10px;
margin: 0px 16px 0px 10px;
}

.theia-notebook-cell.focused .theia-notebook-cell-editor-container {
Expand Down Expand Up @@ -288,7 +292,7 @@

.theia-notebook-cell-output-webview {
padding: 5px 0px;
margin: 0px 10px;
margin: 0px 15px 0px 9px;
width: 100%;
}

Expand Down Expand Up @@ -350,7 +354,7 @@
transform: translateY(calc(-100% - 10px));
}

.theia-notebook-find-widget.search-mode > * > *:nth-child(2) {
.theia-notebook-find-widget.search-mode>*>*:nth-child(2) {
display: none;
}

Expand Down Expand Up @@ -379,9 +383,9 @@
align-items: center;
}

.theia-notebook-find-widget-buttons-first > div,
.theia-notebook-find-widget-buttons-second > div {
margin-right: 4px;
.theia-notebook-find-widget-buttons-first>div,
.theia-notebook-find-widget-buttons-second>div {
margin-right: 4px;
}

.theia-notebook-find-widget-buttons-second {
Expand Down Expand Up @@ -457,11 +461,11 @@
}

mark.theia-find-match {
color: var(--theia-editor-findMatchHighlightForeground);
background-color: var(--theia-editor-findMatchHighlightBackground);
color: var(--theia-editor-findMatchHighlightForeground);
background-color: var(--theia-editor-findMatchHighlightBackground);
}

mark.theia-find-match.theia-find-match-selected {
color: var(--theia-editor-findMatchForeground);
background-color: var(--theia-editor-findMatchBackground);
color: var(--theia-editor-findMatchForeground);
background-color: var(--theia-editor-findMatchBackground);
}
39 changes: 21 additions & 18 deletions packages/notebook/src/browser/view/notebook-cell-list-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
protected toDispose = new DisposableCollection();

protected static dragGhost: HTMLElement | undefined;
protected cellListRef: React.RefObject<HTMLUListElement> = React.createRef();

constructor(props: CellListProps) {
super(props);
Expand Down Expand Up @@ -82,31 +83,32 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
scrollIntoView: e.scrollIntoView
});
}));

this.toDispose.push(onDomEvent(document, 'focusin', () => {
animationFrame().then(() => {
if (!this.cellListRef.current) {
return;
}
let hasCellFocus = false;
let hasFocus = false;
if (this.cellListRef.current.contains(document.activeElement)) {
if (this.props.notebookModel.selectedCell) {
hasCellFocus = true;
}
hasFocus = true;
}
this.props.notebookContext.changeCellFocus(hasCellFocus);
this.props.notebookContext.changeCellListFocus(hasFocus);
});
}));
}

override componentWillUnmount(): void {
this.toDispose.dispose();
}

override render(): React.ReactNode {
return <ul className='theia-notebook-cell-list' ref={
ref => {
this.toDispose.push(onDomEvent(document, 'focusin', () => {
animationFrame().then(() => {
let hasCellFocus = false;
let hasFocus = false;
if (ref?.contains(document.activeElement)) {
if (this.props.notebookModel.selectedCell) {
hasCellFocus = true;
}
hasFocus = true;
}
this.props.notebookContext.changeCellFocus(hasCellFocus);
this.props.notebookContext.changeCellListFocus(hasFocus);
});
}));
}
}>
return <ul className='theia-notebook-cell-list' ref={this.cellListRef}>
{this.props.notebookModel.cells
.map((cell, index) =>
<React.Fragment key={'cell-' + cell.handle}>
Expand All @@ -129,6 +131,7 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
onDragOver={e => this.onDragOver(e, cell)}
onDrop={e => this.onDrop(e, index)}
draggable={true}
tabIndex={-1}
ref={ref => cell === this.state.selectedCell && this.state.scrollIntoView && ref?.scrollIntoView({ block: 'nearest' })}>
<div className={'theia-notebook-cell-marker' + (this.state.selectedCell === cell ? ' theia-notebook-cell-marker-selected' : '')}></div>
<div className='theia-notebook-cell-content'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,83 @@ export function createCellOutputWebviewContainer(ctx: interfaces.Container, cell
return child;
}

// Should be kept up-to-date with:
// https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping.ts
const mapping: ReadonlyMap<string, string> = new Map([
['theme-font-family', 'vscode-font-family'],
['theme-font-weight', 'vscode-font-weight'],
['theme-font-size', 'vscode-font-size'],
['theme-code-font-family', 'vscode-editor-font-family'],
['theme-code-font-weight', 'vscode-editor-font-weight'],
['theme-code-font-size', 'vscode-editor-font-size'],
['theme-scrollbar-background', 'vscode-scrollbarSlider-background'],
['theme-scrollbar-hover-background', 'vscode-scrollbarSlider-hoverBackground'],
['theme-scrollbar-active-background', 'vscode-scrollbarSlider-activeBackground'],
['theme-quote-background', 'vscode-textBlockQuote-background'],
['theme-quote-border', 'vscode-textBlockQuote-border'],
['theme-code-foreground', 'vscode-textPreformat-foreground'],
// Editor
['theme-background', 'vscode-editor-background'],
['theme-foreground', 'vscode-editor-foreground'],
['theme-ui-foreground', 'vscode-foreground'],
['theme-link', 'vscode-textLink-foreground'],
['theme-link-active', 'vscode-textLink-activeForeground'],
// Buttons
['theme-button-background', 'vscode-button-background'],
['theme-button-hover-background', 'vscode-button-hoverBackground'],
['theme-button-foreground', 'vscode-button-foreground'],
['theme-button-secondary-background', 'vscode-button-secondaryBackground'],
['theme-button-secondary-hover-background', 'vscode-button-secondaryHoverBackground'],
['theme-button-secondary-foreground', 'vscode-button-secondaryForeground'],
['theme-button-hover-foreground', 'vscode-button-foreground'],
['theme-button-focus-foreground', 'vscode-button-foreground'],
['theme-button-secondary-hover-foreground', 'vscode-button-secondaryForeground'],
['theme-button-secondary-focus-foreground', 'vscode-button-secondaryForeground'],
// Inputs
['theme-input-background', 'vscode-input-background'],
['theme-input-foreground', 'vscode-input-foreground'],
['theme-input-placeholder-foreground', 'vscode-input-placeholderForeground'],
['theme-input-focus-border-color', 'vscode-focusBorder'],
// Menus
['theme-menu-background', 'vscode-menu-background'],
['theme-menu-foreground', 'vscode-menu-foreground'],
['theme-menu-hover-background', 'vscode-menu-selectionBackground'],
['theme-menu-focus-background', 'vscode-menu-selectionBackground'],
['theme-menu-hover-foreground', 'vscode-menu-selectionForeground'],
['theme-menu-focus-foreground', 'vscode-menu-selectionForeground'],
// Errors
['theme-error-background', 'vscode-inputValidation-errorBackground'],
['theme-error-foreground', 'vscode-foreground'],
['theme-warning-background', 'vscode-inputValidation-warningBackground'],
['theme-warning-foreground', 'vscode-foreground'],
['theme-info-background', 'vscode-inputValidation-infoBackground'],
['theme-info-foreground', 'vscode-foreground'],
// Notebook:
['theme-notebook-output-background', 'vscode-notebook-outputContainerBackgroundColor'],
['theme-notebook-output-border', 'vscode-notebook-outputContainerBorderColor'],
['theme-notebook-cell-selected-background', 'vscode-notebook-selectedCellBackground'],
['theme-notebook-symbol-highlight-background', 'vscode-notebook-symbolHighlightBackground'],
['theme-notebook-diff-removed-background', 'vscode-diffEditor-removedTextBackground'],
['theme-notebook-diff-inserted-background', 'vscode-diffEditor-insertedTextBackground'],
]);

const constants: Record<string, string> = {
'theme-input-border-width': '1px',
'theme-button-primary-hover-shadow': 'none',
'theme-button-secondary-hover-shadow': 'none',
'theme-input-border-color': 'transparent',
};

export const DEFAULT_NOTEBOOK_OUTPUT_CSS = `
:root {
${Array.from(mapping.entries()).map(([key, value]) => `--${key}: var(--${value});`).join('\n')}
${Object.entries(constants).map(([key, value]) => `--${key}: ${value};`).join('\n')}
}

body {
padding: 0;
}

table {
border-collapse: collapse;
border-spacing: 0;
Expand Down Expand Up @@ -323,6 +399,7 @@ export class CellOutputWebviewImpl implements CellOutputWebview, Disposable {

protected generateStyles(): { [key: string]: string } {
return {
'notebook-output-node-left-padding': `${this.options.outputNodeLeftPadding}px`,
'notebook-cell-output-font-size': `${this.options.outputFontSize || this.options.fontSize}px`,
'notebook-cell-output-line-height': `${this.options.outputLineHeight}px`,
'notebook-cell-output-max-height': `${this.options.outputLineHeight * this.options.outputLineLimit}px`,
Expand Down
Loading
Loading