Skip to content

Commit

Permalink
Fix scrolling on Shift+Enter in defer and Ctrl+Enter in both modes.
Browse files Browse the repository at this point in the history
  • Loading branch information
krassowski committed Jan 26, 2024
1 parent b89a99d commit 56c0d23
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 88 deletions.
35 changes: 25 additions & 10 deletions packages/notebook/src/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,10 @@ export namespace NotebookActions {
translator
);

void Private.handleRunState(notebook, state, false);
// While we do not want to scroll if cell is visible,
// we still need to invoke the scrolling logic
// in case if cell is outside of viewport.
void Private.handleRunState(notebook, state, true);
return promise;
}

Expand Down Expand Up @@ -587,7 +590,11 @@ export namespace NotebookActions {
sessionDialogs,
translator
);
void Private.handleRunState(notebook, state, false);

// While we do not want to scroll if cell is visible,
// we still need to invoke the scrolling logic
// in case if cell is outside of viewport.
void Private.handleRunState(notebook, state, true);
return promise;
}

Expand Down Expand Up @@ -652,7 +659,10 @@ export namespace NotebookActions {
notebook.activeCellIndex++;
}

void Private.handleRunState(notebook, state, true);
// If a cell is outside of viewport and scrolling is needed,
// prefer to align the cell to the top viewport edge,
// rather than to the bottom (so that editor is visible)
void Private.handleRunState(notebook, state, true, 'start');
return promise;
}

Expand Down Expand Up @@ -709,7 +719,7 @@ export namespace NotebookActions {
);
}
notebook.mode = 'edit';
void Private.handleRunState(notebook, state, true);
void Private.handleRunState(notebook, state, true, 'start');
return promise;
}

Expand Down Expand Up @@ -2303,13 +2313,17 @@ namespace Private {
export async function handleRunState(
notebook: Notebook,
state: IState,
scroll = false
scroll = false,
alignPreference?: 'start' | 'end'
): Promise<void> {
const { activeCell, activeCellIndex } = notebook;

if (scroll && activeCell) {
await notebook.scrollToItem(activeCellIndex, 'smart', 0).catch(reason => {
// no-op
});
await notebook
.scrollToItem(activeCellIndex, 'smart', 0, alignPreference)
.catch(reason => {
// no-op
});
}
if (state.wasFocused || notebook.mode === 'edit') {
notebook.activate();
Expand All @@ -2333,7 +2347,7 @@ namespace Private {
translator?: ITranslator
): Promise<boolean> {
const lastCell = cells[-1];
notebook.mode = 'command';
notebook.setMode('command', { focus: true, preventScrollOnFocus: true });

let initializingDialogShown = false;
return Promise.all(
Expand Down Expand Up @@ -2428,7 +2442,8 @@ namespace Private {
sessionDialogs?: ISessionContextDialogs,
translator?: ITranslator
): Promise<boolean> {
notebook.mode = 'command';
// Note: focusing a node can invoke scrolling by the browser
notebook.setMode('command', { focus: true, preventScrollOnFocus: true });

let lastIndex = notebook.activeCellIndex;
const selected = notebook.widgets.filter((child, index) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/notebook/src/toc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,8 @@ export class NotebookToCFactory extends TableOfContentsFactory<NotebookPanel> {
console.debug('scrolling to heading: using fallback strategy');
await widget.content.scrollToItem(
widget.content.activeCellIndex,
this.scrollToTop ? 'start' : undefined
this.scrollToTop ? 'start' : undefined,
0
);
}
};
Expand Down
9 changes: 6 additions & 3 deletions packages/notebook/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1417,12 +1417,14 @@ export class Notebook extends StaticNotebook {
* @param newValue Notebook mode
* @param options Control mode side-effect
* @param options.focus Whether to ensure focus (default) or not when setting the mode.
* @param options.preventScrollOnFocus If focusing to ensure focus (default) or not when setting the mode.
*/
protected setMode(
setMode(
newValue: NotebookMode,
options: { focus?: boolean } = {}
options: { focus?: boolean; preventScrollOnFocus?: boolean } = {}
): void {
const setFocus = options.focus ?? true;
const preventScrollOnFocus = options.preventScrollOnFocus ?? false;
const activeCell = this.activeCell;
if (!activeCell) {
newValue = 'command';
Expand Down Expand Up @@ -1457,7 +1459,8 @@ export class Notebook extends StaticNotebook {
// activeCell.node.focus() is called, which closes the command palette.
// To the end user, it looks as if all the keyboard shortcut did was
// move focus from the cell editor to the cell as a whole.
waitUntilReady: false
waitUntilReady: false,
preventScroll: preventScrollOnFocus
});
}
}
Expand Down
Loading

0 comments on commit 56c0d23

Please sign in to comment.