Skip to content

Commit

Permalink
Fix notebook output scrolling and text rendering (eclipse-theia#14016)
Browse files Browse the repository at this point in the history
  • Loading branch information
msujew authored Aug 6, 2024
1 parent 9e91241 commit 1e5fb53
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 59 deletions.
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

0 comments on commit 1e5fb53

Please sign in to comment.