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.

Sign up for GitHub

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

Notebooks: Aligned scroll into view behaviour with vscode #13742

Merged
merged 6 commits into from
May 30, 2024
Merged

Notebooks: Aligned scroll into view behaviour with vscode #13742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec44efa
Fixed focus loss of the notebook editor widget when a cell editor was…
jonah-iden May 29, 2024
0c8f522
aligned the scroll into view behaviour with vscode
jonah-iden May 29, 2024
c58e855
inserting new cell scrolls
jonah-iden May 29, 2024
c6e717a
fixed delete still scrolling
jonah-iden May 29, 2024
998f55c
dont scroll into cell when click into text
jonah-iden May 30, 2024
edc21f6
Merge branch 'master' into jiden/notebook-aligned-scroll-into-view
jonah-iden May 30, 2024
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: 1 addition & 2 deletions packages/notebook/src/browser/service/notebook-context-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export class NotebookContextManager {
// Cell Selection realted keys
this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!widget.model?.selectedCell);
widget.model?.onDidChangeSelectedCell(e => {
this.selectedCellChanged(e.cell);
this.scopedStore.setContext(NOTEBOOK_CELL_FOCUSED, !!e);
this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_CELL_FOCUSED]));
});
Expand All @@ -100,8 +101,6 @@ export class NotebookContextManager {
}
}));

widget.model?.onDidChangeSelectedCell(e => this.selectedCellChanged(e));

widget.onDidChangeOutputInputFocus(focus => {
this.scopedStore.setContext(NOTEBOOK_OUTPUT_INPUT_FOCUSED, focus);
this.onDidChangeContextEmitter.fire(this.createContextKeyChangedEvent([NOTEBOOK_OUTPUT_INPUT_FOCUSED]));
Expand Down
16 changes: 12 additions & 4 deletions packages/notebook/src/browser/view-model/notebook-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ export interface NotebookModelProps {
serializer: NotebookSerializer;
}

export interface SelectedCellChangeEvent {
cell: NotebookCellModel | undefined;
scrollIntoView: boolean;
}

@injectable()
export class NotebookModel implements Saveable, Disposable {

Expand All @@ -74,7 +79,7 @@ export class NotebookModel implements Saveable, Disposable {
protected readonly onContentChangedEmitter = new Emitter<void>();
readonly onContentChanged = this.onContentChangedEmitter.event;

protected readonly onDidChangeSelectedCellEmitter = new Emitter<NotebookCellModel | undefined>();
protected readonly onDidChangeSelectedCellEmitter = new Emitter<SelectedCellChangeEvent>();
readonly onDidChangeSelectedCell = this.onDidChangeSelectedCellEmitter.event;

protected readonly onDidDisposeEmitter = new Emitter<void>();
Expand Down Expand Up @@ -246,10 +251,10 @@ export class NotebookModel implements Saveable, Disposable {
this.undoRedoService.redo(this.uri);
}

setSelectedCell(cell: NotebookCellModel): void {
setSelectedCell(cell: NotebookCellModel, scrollIntoView?: boolean): void {
if (this.selectedCell !== cell) {
this.selectedCell = cell;
this.onDidChangeSelectedCellEmitter.fire(cell);
this.onDidChangeSelectedCellEmitter.fire({ cell, scrollIntoView: scrollIntoView ?? true });
}
}

Expand Down Expand Up @@ -285,9 +290,12 @@ export class NotebookModel implements Saveable, Disposable {
if (cell) {
this.cellDirtyChanged(cell, true);
}

let scrollIntoView = true;
switch (edit.editType) {
case CellEditType.Replace:
this.replaceCells(edit.index, edit.count, edit.cells, computeUndoRedo);
scrollIntoView = edit.cells.length > 0;
break;
case CellEditType.Output: {
if (edit.append) {
Expand Down Expand Up @@ -330,7 +338,7 @@ export class NotebookModel implements Saveable, Disposable {

// if selected cell is affected update it because it can potentially have been replaced
if (cell === this.selectedCell) {
this.setSelectedCell(this.cells[Math.min(cellIndex, this.cells.length - 1)]);
this.setSelectedCell(this.cells[Math.min(cellIndex, this.cells.length - 1)], scrollIntoView);
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/notebook/src/browser/view/notebook-cell-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ export class CellEditor extends React.Component<CellEditorProps, {}> {
this.editor?.setLanguage(language);
}));

this.toDispose.push(this.props.notebookModel.onDidChangeSelectedCell(cell => {
if (cell !== this.props.cell && this.editor?.getControl().hasTextFocus()) {
this.toDispose.push(this.props.notebookModel.onDidChangeSelectedCell(e => {
if (e.cell !== this.props.cell && this.editor?.getControl().hasTextFocus()) {
this.props.notebookContextManager.context?.focus();
}
}));
Expand Down Expand Up @@ -129,7 +129,7 @@ export class CellEditor extends React.Component<CellEditorProps, {}> {
}));
this.toDispose.push(this.editor.getControl().onDidFocusEditorText(() => {
this.props.notebookContextManager.onDidEditorTextFocus(true);
this.props.notebookModel.setSelectedCell(cell);
this.props.notebookModel.setSelectedCell(cell, false);
}));
this.toDispose.push(this.editor.getControl().onDidBlurEditorText(() => {
this.props.notebookContextManager.onDidEditorTextFocus(false);
Expand Down
27 changes: 20 additions & 7 deletions packages/notebook/src/browser/view/notebook-cell-list-view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ interface CellListProps {

interface NotebookCellListState {
selectedCell?: NotebookCellModel;
scrollIntoView: boolean;
dragOverIndicator: { cell: NotebookCellModel, position: 'top' | 'bottom' } | undefined;
}

Expand All @@ -48,17 +49,29 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo

constructor(props: CellListProps) {
super(props);
this.state = { selectedCell: props.notebookModel.selectedCell, dragOverIndicator: undefined };
this.state = { selectedCell: props.notebookModel.selectedCell, dragOverIndicator: undefined, scrollIntoView: true };
this.toDispose.push(props.notebookModel.onDidAddOrRemoveCell(e => {
if (e.newCellIds && e.newCellIds.length > 0) {
this.setState({ ...this.state, selectedCell: this.props.notebookModel.cells.find(model => model.handle === e.newCellIds![e.newCellIds!.length - 1]) });
this.setState({
...this.state,
selectedCell: this.props.notebookModel.cells.find(model => model.handle === e.newCellIds![e.newCellIds!.length - 1]),
scrollIntoView: true
});
} else {
this.setState({ ...this.state, selectedCell: this.props.notebookModel.cells.find(cell => cell === this.state.selectedCell) });
this.setState({
...this.state,
selectedCell: this.props.notebookModel.cells.find(cell => cell === this.state.selectedCell),
scrollIntoView: false
});
}
}));

this.toDispose.push(props.notebookModel.onDidChangeSelectedCell(cell => {
this.setState({ ...this.state, selectedCell: cell });
this.toDispose.push(props.notebookModel.onDidChangeSelectedCell(e => {
this.setState({
...this.state,
selectedCell: e.cell,
scrollIntoView: e.scrollIntoView
});
}));
}

Expand All @@ -80,13 +93,13 @@ export class NotebookCellListView extends React.Component<CellListProps, Noteboo
<li className={'theia-notebook-cell' + (this.state.selectedCell === cell ? ' focused' : '') + (this.isEnabled() ? ' draggable' : '')}
onClick={e => {
this.setState({ ...this.state, selectedCell: cell });
this.props.notebookModel.setSelectedCell(cell);
this.props.notebookModel.setSelectedCell(cell, false);
}}
onDragStart={e => this.onDragStart(e, index, cell)}
onDragOver={e => this.onDragOver(e, cell)}
onDrop={e => this.onDrop(e, index)}
draggable={true}
ref={ref => cell === this.state.selectedCell && ref?.scrollIntoView({ block: 'nearest' })}>
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'>
{this.renderCellContent(cell, index)}
Expand Down
Loading