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

Implements the go back and forward keyboard shortcuts #2789

Merged
merged 17 commits into from
Sep 25, 2024
Merged
9 changes: 9 additions & 0 deletions assets/js/hooks/cell.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ const Cell = {
// gives it focus
if (!this.isFocused || !this.insertMode) {
this.currentEditor().blur();
} else if (this.insertMode) {
const lineNumber = this.currentEditor().getLineNumberAtCursor();
aleDsz marked this conversation as resolved.
Show resolved Hide resolved

if (lineNumber !== null)
globalPubsub.broadcast("history", {
type: "navigation",
cellId: this.props.cellId,
line: lineNumber.toString(),
});
}
}, 0);
});
Expand Down
22 changes: 21 additions & 1 deletion assets/js/hooks/cell_editor/live_editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,18 @@ export default class LiveEditor {
return node.parentElement;
}

/**
* Returns line number from the current main cursor position.
*/
getLineNumberAtCursor() {
if (!this.isMounted()) {
return null;
}

const pos = this.view.state.selection.main.head;
return this.view.state.doc.lineAt(pos).number;
}

/**
* Focuses the editor.
*
Expand Down Expand Up @@ -389,11 +401,19 @@ export default class LiveEditor {
// We dispatch escape event, but only if it is not consumed by any
// registered handler in the editor, such as closing autocompletion
// or escaping Vim insert mode
const cmd = isMacOS() ? event.metaKey : event.ctrlKey;
const alt = event.altKey;
const key = event.key;

if (event.key === "Escape") {
if (key === "Escape") {
this.container.dispatchEvent(
new CustomEvent("lb:editor_escape", { bubbles: true }),
);
} else if (
key === "-" &&
((isMacOS() && cmd) || (!isMacOS() && cmd && alt))
) {
globalPubsub.broadcast("history", { type: "back" });
}

return false;
Expand Down
29 changes: 29 additions & 0 deletions assets/js/hooks/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { leaveChannel } from "./js_view/channel";
import { isDirectlyEditable, isEvaluable } from "../lib/notebook";
import { settingsStore } from "../lib/settings";
import { LiveStore } from "../lib/live_store";
import History from "../lib/history";

/**
* A hook managing the whole session.
Expand Down Expand Up @@ -81,6 +82,7 @@ const Session = {
this.viewOptions = null;
this.keyBuffer = new KeyBuffer();
this.lastLocationReportByClientId = {};
this.history = new History();
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
this.followedClientId = null;
this.store = LiveStore.create("session");

Expand Down Expand Up @@ -161,6 +163,7 @@ const Session = {
globalPubsub.subscribe("jump_to_editor", ({ line, file }) =>
this.jumpToLine(file, line),
),
globalPubsub.subscribe("history", this.handleHistoryEvent.bind(this)),
];

this.initializeDragAndDrop();
Expand Down Expand Up @@ -278,6 +281,7 @@ const Session = {

this.subscriptions.forEach((subscription) => subscription.destroy());
this.store.destroy();
this.history.destroy();
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
},

getProps() {
Expand Down Expand Up @@ -1227,6 +1231,8 @@ const Session = {
},

handleCellDeleted(cellId, siblingCellId) {
this.history.removeAllFromCell(cellId);

if (this.focusedId === cellId) {
if (this.view) {
const visibleSiblingId = this.ensureVisibleFocusableEl(siblingCellId);
Expand Down Expand Up @@ -1324,6 +1330,14 @@ const Session = {
}
},

handleHistoryEvent(payload) {
if (payload.type === "back") {
this.goBackNavigationHistory();
} else if (payload.type === "navigation") {
this.saveNavigationHistory(payload);
}
},

repositionJSViews() {
globalPubsub.broadcast("js_views", { type: "reposition" });
},
Expand Down Expand Up @@ -1447,7 +1461,22 @@ const Session = {

jumpToLine(file, line) {
const [_filename, cellId] = file.split("#cell:");
this.doJumpToLine(cellId, line);
},

saveNavigationHistory({ cellId, line }) {
if (cellId === null || line === null) return;
this.history.saveCell(cellId, line);
},

goBackNavigationHistory() {
if (this.history.canGoBack()) {
const { cellId, line } = this.history.goBack();
this.doJumpToLine(cellId, line);
}
},

doJumpToLine(cellId, line) {
this.setFocusedEl(cellId, { scroll: false });
this.setInsertMode(true);

Expand Down
103 changes: 103 additions & 0 deletions assets/js/lib/history.js
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* Allows for recording a sequence of focused cells with the focused line
* and navigate inside this stack.
*/
export default class History {
constructor() {
this.entries = [];
this.index = -1;
}

/**
* Adds a new cell to the stack.
*
* If the stack length is greater than the stack limit,
* it will remove the oldest entries.
*/
saveCell(cellId, line) {
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
if (this.isTheSameLastEntry(cellId, line)) return;

if (this.entries[this.index + 1] !== undefined)
this.entries = this.entries.slice(0, this.index + 1);

this.entries.push({ cellId, line });
this.index++;

if (this.entries.length > 20) {
this.entries.shift();
this.index--;
}
}

/**
* Immediately clears the stack and reset the current index.
*/
destroy() {
this.entries = [];
this.index = -1;
}

/**
* Removes all matching cells with given id from the stack.
*/
removeAllFromCell(cellId) {
// We need to make sure the last entry from history
// doesn't belong to the given cell id that we need
// to remove from the entries list.
let currentEntryIndex = this.index;
let currentEntry = this.entries[currentEntryIndex];

while (currentEntry.cellId === cellId) {
currentEntryIndex--;
currentEntry = this.entries[currentEntryIndex];
}

this.entries = this.entries.filter((entry) => entry.cellId !== cellId);
this.index = this.entries.lastIndexOf(currentEntry);
}

/**
* Checks if the current stack is available to navigate back.
*/
canGoBack() {
return this.canGetFromHistory(-1);
}

/**
* Navigates back in the current stack.
*
* If the navigation succeeds, it will return the entry from current index.
* Otherwise, returns null;
*/
goBack() {
return this.getFromHistory(-1);
}

/** @private **/
getFromHistory(direction) {
if (!this.canGetFromHistory(direction)) return null;

this.index = Math.max(0, this.index + direction);
return this.entries[this.index];
}

/** @private **/
canGetFromHistory(direction) {
if (this.index === -1) return false;
if (this.entries.length === 0) return false;
aleDsz marked this conversation as resolved.
Show resolved Hide resolved

const index = Math.max(0, this.index + direction);
return this.entries[index] !== undefined;
}

/** @private **/
isTheSameLastEntry(cellId, line) {
const lastEntry = this.entries[this.index];

return (
lastEntry !== undefined &&
cellId === lastEntry.cellId &&
line === lastEntry.line
);
}
}
94 changes: 94 additions & 0 deletions assets/test/lib/history.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import History from "../../js/lib/history";

test("goBack returns when there's at least one cell", () => {
const history = new History();
const expectedEntry = { cellId: "123", line: "1" };

expect(history.canGoBack()).toBe(false);

history.entries = [expectedEntry];
history.index = 0;

expect(history.index).toBe(0);
expect(history.canGoBack()).toBe(true);
expect(history.goBack()).toStrictEqual(expectedEntry);
});

describe("saveCell", () => {
test("does not add duplicated cells", () => {
const history = new History();
const entry = { cellId: "123", line: "1" };

expect(history.index).toBe(-1);

history.entries = [entry];
history.index = 0;

history.saveCell(entry.cellId, entry.line);
expect(history.index).toBe(0);

// It will only add a new cell if the line isn't the same
// as the last entry from the stack
history.saveCell(entry.cellId, "2");
expect(history.index).toBe(1);
});

test("removes oldest entries and keep it with a maximum of 20 entries", () => {
const history = new History();

for (let i = 0; i <= 19; i++) history.saveCell("123", (i + 1).toString());

expect(history.index).toBe(19);

history.saveCell("231", "1");

expect(history.index).toBe(19);
expect(history.entries[0]).toStrictEqual({ cellId: "123", line: "2" });
});

test("rewrites the subsequent cells if go back and saves a new cell", () => {
const history = new History();
expect(history.canGoBack()).toBe(false);

history.entries = [
{ cellId: "123", line: "1" },
{ cellId: "456", line: "1" },
{ cellId: "789", line: "2" },
];
history.index = 2;

expect(history.canGoBack()).toBe(true);

// Go back to cell id 456
history.goBack();
expect(history.index).toBe(1);

// Go back to cell id 123
history.goBack();
expect(history.index).toBe(0);

// Removes the subsequent cells from stack
// and adds the cell id 999 to the stack
history.saveCell("999", "1");
expect(history.index).toBe(1);
});
});

test("removeAllFromCell removes the cells with given id", () => {
const history = new History();
const cellId = "123456789";

history.entries = [
{ cellId: "123", line: "1" },
{ cellId: "456", line: "1" },
{ cellId: cellId, line: "1" },
{ cellId: "1234", line: "1" },
{ cellId: cellId, line: "1" },
{ cellId: "8901", line: "1" },
{ cellId: cellId, line: "1" },
];
history.index = 6;

history.removeAllFromCell(cellId);
expect(history.index).toBe(3);
});
6 changes: 6 additions & 0 deletions lib/livebook_web/live/session_live/shortcuts_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ defmodule LivebookWeb.SessionLive.ShortcutsComponent do
seq_mac: ["⌘", "k"],
press_all: true,
desc: "Toggle keyboard control in cell output"
},
%{
seq: ["ctrl", "alt", "-"],
seq_mac: ["⌥", "-"],
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
press_all: true,
desc: "Go back"
aleDsz marked this conversation as resolved.
Show resolved Hide resolved
}
],
universal: [
Expand Down
Loading