From 266bf35bd067809f449e71f893fd5c75c408fa40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Thu, 11 Mar 2021 15:28:18 +0100 Subject: [PATCH] Move focus navigation to the client (#74) * Show all sections and enable cross-section focus navigation * Move focus to the client * Add shortcut for evaluating all cells * Fix and expand tests * Make section links scroll to the given section --- assets/css/app.css | 1 + assets/css/session.css | 40 ++ assets/js/app.js | 11 + assets/js/cell/index.js | 113 ++-- assets/js/lib/pub_sub.js | 50 ++ assets/js/lib/utils.js | 35 ++ assets/js/session/index.js | 535 ++++++++++++++---- assets/js/virtualized_lines/index.js | 36 +- assets/package.json | 2 +- assets/test/lib/pub_sub.test.js | 27 + lib/livebook/notebook.ex | 71 ++- lib/livebook/session/data.ex | 15 +- lib/livebook_web/live/cell_component.ex | 113 ++-- .../live/path_select_component.ex | 3 +- lib/livebook_web/live/section_component.ex | 11 +- lib/livebook_web/live/session_live.ex | 278 +++------ .../live/session_live/shortcuts_component.ex | 1 + test/livebook/session/data_test.exs | 25 + test/livebook_web/live/session_live_test.exs | 42 +- 19 files changed, 912 insertions(+), 497 deletions(-) create mode 100644 assets/css/session.css create mode 100644 assets/js/lib/pub_sub.js create mode 100644 assets/test/lib/pub_sub.test.js diff --git a/assets/css/app.css b/assets/css/app.css index 19363f706..09abb17c5 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -10,3 +10,4 @@ @import "./live_view.css"; @import "./markdown.css"; @import "./ansi.css"; +@import "./session.css"; diff --git a/assets/css/session.css b/assets/css/session.css new file mode 100644 index 000000000..000cc6680 --- /dev/null +++ b/assets/css/session.css @@ -0,0 +1,40 @@ +/* +Conditional notebook elements display. + +The Section and Cell hooks dynamically set attributes +based on which we hide/show certain elements. +This way we don't have to engage the server in +solely client-side operations like moving the focus +and entering/escaping insert mode. +*/ + +[data-element="session"]:not([data-js-insert-mode]) + [data-element="insert-indicator"] { + @apply hidden; +} + +[data-element="session"] + [data-element="cell"][data-type="markdown"] + [data-element="editor-box"] { + @apply hidden; +} + +[data-element="session"][data-js-insert-mode] + [data-element="cell"][data-type="markdown"][data-js-focused] + [data-element="editor-box"] { + @apply block; +} + +[data-element="session"][data-js-insert-mode] + [data-element="cell"][data-type="markdown"][data-js-focused] + [data-element="enable-insert-mode-button"] { + @apply hidden; +} + +[data-element="cell"][data-js-focused] { + @apply border-blue-300 border-opacity-100; +} + +[data-element="cell"]:not([data-js-focused]) [data-element="actions"] { + @apply hidden; +} diff --git a/assets/js/app.js b/assets/js/app.js index eac70c907..ed39a6194 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -27,6 +27,17 @@ const csrfToken = document const liveSocket = new LiveSocket("/live", Socket, { params: { _csrf_token: csrfToken }, hooks: Hooks, + dom: { + onBeforeElUpdated(from, to) { + // Keep element attributes starting with data-js- + // which we set on the client. + for (const attr of from.attributes) { + if (attr.name.startsWith("data-js-")) { + to.setAttribute(attr.name, attr.value); + } + } + }, + }, }); // Show progress bar on live navigation and form submits diff --git a/assets/js/cell/index.js b/assets/js/cell/index.js index d2a53a38d..351250c1a 100644 --- a/assets/js/cell/index.js +++ b/assets/js/cell/index.js @@ -1,10 +1,8 @@ -import { - getAttributeOrThrow, - parseBoolean, - parseInteger, -} from "../lib/attribute"; +import { getAttributeOrThrow } from "../lib/attribute"; import LiveEditor from "./live_editor"; import Markdown from "./markdown"; +import { globalPubSub } from "../lib/pub_sub"; +import { smoothlyScrollToElement } from "../lib/utils"; /** * A hook managing a single cell. @@ -16,24 +14,27 @@ import Markdown from "./markdown"; * * * `data-cell-id` - id of the cell being edited * * `data-type` - editor type (i.e. language), either "markdown" or "elixir" is expected - * * `data-focused` - whether the cell is currently focused - * * `data-insert-mode` - whether insert mode is currently enabled */ const Cell = { mounted() { this.props = getProps(this); + this.state = { + liveEditor: null, + isFocused: false, + insertMode: false, + }; this.pushEvent("cell_init", { cell_id: this.props.cellId }, (payload) => { const { source, revision } = payload; - const editorContainer = this.el.querySelector("[data-editor-container]"); + const editorContainer = this.el.querySelector(`[data-element="editor-container"]`); // Remove the content placeholder. editorContainer.firstElementChild.remove(); // Create an empty container for the editor to be mounted in. const editorElement = document.createElement("div"); editorContainer.appendChild(editorElement); // Setup the editor instance. - this.liveEditor = new LiveEditor( + this.state.liveEditor = new LiveEditor( this, editorElement, this.props.cellId, @@ -45,52 +46,43 @@ const Cell = { // Setup markdown rendering. if (this.props.type === "markdown") { const markdownContainer = this.el.querySelector( - "[data-markdown-container]" + `[data-element="markdown-container"]` ); const markdown = new Markdown(markdownContainer, source); - this.liveEditor.onChange((newSource) => { + this.state.liveEditor.onChange((newSource) => { markdown.setContent(newSource); }); } - // New cells are initially focused, so check for such case. - - if (isEditorActive(this.props)) { - this.liveEditor.focus(); + // Once the editor is created, reflect the current state. + if (this.state.isFocused && this.state.insertMode) { + this.state.liveEditor.focus(); + // If the element is being scrolled to, focus interrupts it, + // so ensure the scrolling continues. + smoothlyScrollToElement(this.el); } - if (this.props.isFocused) { - this.el.scrollIntoView({ behavior: "smooth", block: "center" }); - } - - this.liveEditor.onBlur(() => { - if (isEditorActive(this.props)) { - this.liveEditor.focus(); + this.state.liveEditor.onBlur(() => { + // Prevent from blurring unless the state changes. + // For example when we move cell using buttons + // the editor should keep focus. + if (this.state.isFocused && this.state.insertMode) { + this.state.liveEditor.focus(); } }); }); + + this.handleSessionEvent = (event) => handleSessionEvent(this, event); + globalPubSub.subscribe("session", this.handleSessionEvent); + }, + + destroyed() { + globalPubSub.unsubscribe("session", this.handleSessionEvent); }, updated() { - const prevProps = this.props; this.props = getProps(this); - - // Note: this.liveEditor is crated once we receive initial data - // so here we have to make sure it's defined. - - if (!isEditorActive(prevProps) && isEditorActive(this.props)) { - this.liveEditor && this.liveEditor.focus(); - } - - if (isEditorActive(prevProps) && !isEditorActive(this.props)) { - this.liveEditor && this.liveEditor.blur(); - } - - if (!prevProps.isFocused && this.props.isFocused) { - // Note: it's important to trigger scrolling after focus, so it doesn't get interrupted. - this.el.scrollIntoView({ behavior: "smooth", block: "center" }); - } }, }; @@ -98,16 +90,49 @@ function getProps(hook) { return { cellId: getAttributeOrThrow(hook.el, "data-cell-id"), type: getAttributeOrThrow(hook.el, "data-type"), - isFocused: getAttributeOrThrow(hook.el, "data-focused", parseBoolean), - insertMode: getAttributeOrThrow(hook.el, "data-insert-mode", parseBoolean), }; } /** - * Checks if the cell editor is active and should have focus. + * Handles client-side session event. */ -function isEditorActive(props) { - return props.isFocused && props.insertMode; +function handleSessionEvent(hook, event) { + if (event.type === "cell_focused") { + handleCellFocused(hook, event.cellId); + } else if (event.type === "insert_mode_changed") { + handleInsertModeChanged(hook, event.enabled); + } else if (event.type === "cell_moved") { + handleCellMoved(hook, event.cellId); + } +} + +function handleCellFocused(hook, cellId) { + if (hook.props.cellId === cellId) { + hook.state.isFocused = true; + hook.el.setAttribute("data-js-focused", "true"); + smoothlyScrollToElement(hook.el); + } else if (hook.state.isFocused) { + hook.state.isFocused = false; + hook.el.removeAttribute("data-js-focused"); + } +} + +function handleInsertModeChanged(hook, insertMode) { + if (hook.state.isFocused) { + hook.state.insertMode = insertMode; + + if (hook.state.insertMode) { + hook.state.liveEditor && hook.state.liveEditor.focus(); + } else { + hook.state.liveEditor && hook.state.liveEditor.blur(); + } + } +} + +function handleCellMoved(hook, cellId) { + if (hook.state.isFocused && cellId === hook.props.cellId) { + smoothlyScrollToElement(hook.el); + } } export default Cell; diff --git a/assets/js/lib/pub_sub.js b/assets/js/lib/pub_sub.js new file mode 100644 index 000000000..8ced346a0 --- /dev/null +++ b/assets/js/lib/pub_sub.js @@ -0,0 +1,50 @@ +/** + * A basic pub-sub implementation for client-side communication. + */ +export default class PubSub { + constructor() { + this.subscribersByTopic = {}; + } + + /** + * Links the given function to the given topic. + * + * Subsequent calls to `broadcast` with this topic + * will result in this function being called. + */ + subscribe(topic, callback) { + if (!Array.isArray(this.subscribersByTopic[topic])) { + this.subscribersByTopic[topic] = []; + } + + this.subscribersByTopic[topic].push(callback); + } + + /** + * Unlinks the given function from the given topic. + * + * Note that you must pass the same function reference + * as you passed to `subscribe`. + */ + unsubscribe(topic, callback) { + const idx = this.subscribersByTopic[topic].indexOf(callback); + + if (idx !== -1) { + this.subscribersByTopic[topic].splice(idx, 1); + } + } + + /** + * Calls all functions linked to the given topic + * and passes `payload` as the argument. + */ + broadcast(topic, payload) { + if (Array.isArray(this.subscribersByTopic[topic])) { + this.subscribersByTopic[topic].forEach((callback) => { + callback(payload); + }); + } + } +} + +export const globalPubSub = new PubSub(); diff --git a/assets/js/lib/utils.js b/assets/js/lib/utils.js index 772507d10..22e59220d 100644 --- a/assets/js/lib/utils.js +++ b/assets/js/lib/utils.js @@ -8,3 +8,38 @@ export function isEditableElement(element) { element.contentEditable === "true" ); } + +export function clamp(n, x, y) { + return Math.min(Math.max(n, x), y); +} + +export function getLineHeight(element) { + const computedStyle = window.getComputedStyle(element); + const lineHeight = parseInt(computedStyle.lineHeight, 10); + + if (Number.isNaN(lineHeight)) { + const clone = element.cloneNode(); + clone.innerHTML = "
"; + element.appendChild(clone); + const singleLineHeight = clone.clientHeight; + clone.innerHTML = "

"; + const doubleLineHeight = clone.clientHeight; + element.removeChild(clone); + const lineHeight = doubleLineHeight - singleLineHeight; + return lineHeight; + } else { + return lineHeight; + } +} + +export function selectElementContent(element) { + const selection = window.getSelection(); + const range = document.createRange(); + range.selectNodeContents(element); + selection.removeAllRanges(); + selection.addRange(range); +} + +export function smoothlyScrollToElement(element) { + element.scrollIntoView({ behavior: "smooth", block: "center" }); +} diff --git a/assets/js/session/index.js b/assets/js/session/index.js index 34b9d9a1f..411688eaa 100644 --- a/assets/js/session/index.js +++ b/assets/js/session/index.js @@ -1,141 +1,448 @@ -import { getAttributeOrThrow, parseBoolean } from "../lib/attribute"; -import { isMacOS, isEditableElement } from "../lib/utils"; +import { + isMacOS, + isEditableElement, + clamp, + selectElementContent, + smoothlyScrollToElement, +} from "../lib/utils"; import KeyBuffer from "./key_buffer"; +import { globalPubSub } from "../lib/pub_sub"; /** * A hook managing the whole session. * - * Registers event listeners to handle keybindings and focus events. - * - * Configuration: - * - * * `data-focused-cell-id` - id of the cell currently being focused + * Handles keybindings, focus changes and insert mode changes. */ const Session = { mounted() { - this.props = getProps(this); - - // Keybindings - // Note: make sure to keep the shortcuts help modal up to date. - - const keyBuffer = new KeyBuffer(); - - this.handleDocumentKeydown = (event) => { - if (event.repeat) { - return; - } - - const cmd = isMacOS() ? event.metaKey : event.ctrlKey; - const key = event.key; - - if (this.props.insertMode) { - keyBuffer.reset(); - - if (key === "Escape") { - this.pushEvent("set_insert_mode", { enabled: false }); - } else if ( - this.props.focusedCellType === "elixir" && - cmd && - key === "Enter" - ) { - cancelEvent(event); - this.pushEvent("queue_focused_cell_evaluation"); - } - } else { - if (isEditableElement(event.target)) { - keyBuffer.reset(); - return; - } - - keyBuffer.push(event.key); - - if (keyBuffer.tryMatch(["d", "d"])) { - this.pushEvent("delete_focused_cell", {}); - } else if ( - this.props.focusedCellType === "elixir" && - (keyBuffer.tryMatch(["e", "e"]) || (cmd && key === "Enter")) - ) { - this.pushEvent("queue_focused_cell_evaluation", {}); - } else if (keyBuffer.tryMatch(["e", "s"])) { - this.pushEvent("queue_section_cells_evaluation", {}); - } else if (keyBuffer.tryMatch(["e", "j"])) { - this.pushEvent("queue_child_cells_evaluation", {}); - } else if (keyBuffer.tryMatch(["e", "x"])) { - this.pushEvent("cancel_focused_cell_evaluation", {}); - } else if (keyBuffer.tryMatch(["?"])) { - this.pushEvent("show_shortcuts", {}); - } else if (key === "i") { - this.pushEvent("set_insert_mode", { enabled: true }); - } else if (key === "j") { - this.pushEvent("move_cell_focus", { offset: 1 }); - } else if (key === "k") { - this.pushEvent("move_cell_focus", { offset: -1 }); - } else if (key === "J") { - this.pushEvent("move_focused_cell", { offset: 1 }); - } else if (key === "K") { - this.pushEvent("move_focused_cell", { offset: -1 }); - } else if (key === "n") { - this.pushEvent("insert_cell_below_focused", { type: "elixir" }); - } else if (key === "N") { - this.pushEvent("insert_cell_above_focused", { type: "elixir" }); - } else if (key === "m") { - this.pushEvent("insert_cell_below_focused", { type: "markdown" }); - } else if (key === "M") { - this.pushEvent("insert_cell_above_focused", { type: "markdown" }); - } - } + this.state = { + focusedCellId: null, + focusedSectionId: null, + focusedCellType: null, + insertMode: false, + keyBuffer: new KeyBuffer(), }; - document.addEventListener("keydown", this.handleDocumentKeydown, true); + // DOM events - // Focus/unfocus a cell when the user clicks somewhere - // Note: we use mousedown event to more reliably focus editor - // (e.g. if the user starts selecting some text within the editor) + this.handleDocumentKeyDown = (event) => { + handleDocumentKeyDown(this, event); + }; + + document.addEventListener("keydown", this.handleDocumentKeyDown, true); this.handleDocumentMouseDown = (event) => { - // If click targets cell actions, keep the focus as is - if (event.target.closest("[data-cell-actions]")) { - return; - } - - // Find the parent with cell id info, if there is one - const cell = event.target.closest("[data-cell-id]"); - const cellId = cell ? cell.dataset.cellId : null; - if (cellId !== this.props.focusedCellId) { - this.pushEvent("focus_cell", { cell_id: cellId }); - } - - // Depending if the click targets editor or not disable/enable insert mode. - if (cell) { - const editorContainer = cell.querySelector("[data-editor-container]"); - const editorClicked = editorContainer.contains(event.target); - this.pushEvent("set_insert_mode", { enabled: editorClicked }); - } + handleDocumentMouseDown(this, event); }; document.addEventListener("mousedown", this.handleDocumentMouseDown); - }, - updated() { - this.props = getProps(this); + this.el.querySelector(`[data-element="section-list"]`).addEventListener("click", event => { + handleSectionListClick(this, event); + }); + + // Server events + + this.handleEvent("cell_inserted", ({ cell_id: cellId }) => { + handleCellInserted(this, cellId); + }); + + this.handleEvent( + "cell_deleted", + ({ cell_id: cellId, sibling_cell_id: siblingCellId }) => { + handleCellDeleted(this, cellId, siblingCellId); + } + ); + + this.handleEvent("cell_moved", ({ cell_id: cellId }) => { + handleCellMoved(this, cellId); + }); + + this.handleEvent("section_inserted", ({ section_id: sectionId }) => { + handleSectionInserted(this, sectionId); + }); + + this.handleEvent("section_deleted", ({ section_id: sectionId }) => { + handleSectionDeleted(this, sectionId); + }); }, destroyed() { - document.removeEventListener("keydown", this.handleDocumentKeydown); + document.removeEventListener("keydown", this.handleDocumentKeyDown); document.removeEventListener("mousedown", this.handleDocumentMouseDown); }, }; -function getProps(hook) { - return { - insertMode: getAttributeOrThrow(hook.el, "data-insert-mode", parseBoolean), - focusedCellId: getAttributeOrThrow( - hook.el, - "data-focused-cell-id", - (value) => value || null - ), - focusedCellType: getAttributeOrThrow(hook.el, "data-focused-cell-type"), - }; +/** + * Handles session keybindings. + * + * Make sure to keep the shortcuts help modal up to date. + */ +function handleDocumentKeyDown(hook, event) { + if (event.repeat) { + return; + } + + const cmd = isMacOS() ? event.metaKey : event.ctrlKey; + const key = event.key; + const keyBuffer = hook.state.keyBuffer; + + if (hook.state.insertMode) { + keyBuffer.reset(); + + if (key === "Escape") { + // Ignore Escape if it's supposed to close some Monaco input (like the find/replace box) + if (!event.target.closest(".monaco-inputbox")) { + escapeInsertMode(hook); + } + } else if ( + hook.state.focusedCellType === "elixir" && + cmd && + key === "Enter" + ) { + cancelEvent(event); + queueFocusedCellEvaluation(hook); + } + } else { + // Ignore inputs and notebook/section title fields + if (isEditableElement(event.target)) { + keyBuffer.reset(); + return; + } + + keyBuffer.push(event.key); + + if (keyBuffer.tryMatch(["d", "d"])) { + deleteFocusedCell(hook); + } else if ( + hook.state.focusedCellType === "elixir" && + (keyBuffer.tryMatch(["e", "e"]) || (cmd && key === "Enter")) + ) { + queueFocusedCellEvaluation(hook); + } else if (keyBuffer.tryMatch(["e", "a"])) { + queueAllCellsEvaluation(hook); + } else if (keyBuffer.tryMatch(["e", "s"])) { + queueFocusedSectionEvaluation(hook); + } else if (keyBuffer.tryMatch(["e", "j"])) { + queueChildCellsEvaluation(hook); + } else if (keyBuffer.tryMatch(["e", "x"])) { + cancelFocusedCellEvaluation(hook); + } else if (keyBuffer.tryMatch(["?"])) { + showShortcuts(hook); + } else if (keyBuffer.tryMatch(["i"])) { + cancelEvent(event); + enterInsertMode(hook); + } else if (keyBuffer.tryMatch(["j"])) { + moveCellFocus(hook, 1); + } else if (keyBuffer.tryMatch(["k"])) { + moveCellFocus(hook, -1); + } else if (keyBuffer.tryMatch(["J"])) { + moveFocusedCell(hook, 1); + } else if (keyBuffer.tryMatch(["K"])) { + moveFocusedCell(hook, -1); + } else if (keyBuffer.tryMatch(["n"])) { + insertCellBelowFocused(hook, "elixir"); + } else if (keyBuffer.tryMatch(["N"])) { + insertCellAboveFocused(hook, "elixir"); + } else if (keyBuffer.tryMatch(["m"])) { + insertCellBelowFocused(hook, "markdown"); + } else if (keyBuffer.tryMatch(["M"])) { + insertCellAboveFocused(hook, "markdown"); + } + } +} + +/** + * Focuses/blurs a cell when the user clicks somewhere. + * + * Note: we use mousedown event to more reliably focus editor + * (e.g. if the user starts selecting some text within the editor) + */ +function handleDocumentMouseDown(hook, event) { + // If click targets cell actions, keep the focus as is + if (event.target.closest(`[data-element="actions"]`)) { + // If the pencil icon is clicked, enter insert mode + if (event.target.closest(`[data-element="enable-insert-mode-button"]`)) { + setInsertMode(hook, true); + } + + return; + } + + // Find the cell element, if one was clicked + const cell = event.target.closest(`[data-element="cell"]`); + const cellId = cell ? cell.dataset.cellId : null; + if (cellId !== hook.state.focusedCellId) { + setFocusedCell(hook, cellId); + } + + // Depending on whether the click targets editor disable/enable insert mode + if (cell) { + const editorContainer = cell.querySelector(`[data-element="editor-container"]`); + const editorClicked = editorContainer.contains(event.target); + const insertMode = editorClicked; + if (hook.state.insertMode !== insertMode) { + setInsertMode(hook, insertMode); + } + } +} + +/** + * Handles section link clicks in the section list. + */ +function handleSectionListClick(hook, event) { + const sectionButton = event.target.closest(`[data-element="section-list-item"]`); + if (sectionButton) { + const sectionId = sectionButton.getAttribute("data-section-id"); + const section = getSectionById(sectionId); + section.scrollIntoView({ behavior: "smooth", block: "start" }); + } +} + +// User action handlers (mostly keybindings) + +function deleteFocusedCell(hook) { + if (hook.state.focusedCellId) { + hook.pushEvent("delete_cell", { cell_id: hook.state.focusedCellId }); + } +} + +function queueFocusedCellEvaluation(hook) { + if (hook.state.focusedCellId) { + hook.pushEvent("queue_cell_evaluation", { + cell_id: hook.state.focusedCellId, + }); + } +} + +function queueAllCellsEvaluation(hook) { + hook.pushEvent("queue_all_cells_evaluation", {}); +} + +function queueFocusedSectionEvaluation(hook) { + if (hook.state.focusedSectionId) { + hook.pushEvent("queue_section_cells_evaluation", { + section_id: hook.state.focusedSectionId, + }); + } +} + +function queueChildCellsEvaluation(hook) { + if (hook.state.focusedCellId) { + hook.pushEvent("queue_child_cells_evaluation", { + cell_id: hook.state.focusedCellId, + }); + } +} + +function cancelFocusedCellEvaluation(hook) { + if (hook.state.focusedCellId) { + hook.pushEvent("cancel_cell_evaluation", { + cell_id: hook.state.focusedCellId, + }); + } +} + +function showShortcuts(hook) { + hook.pushEvent("show_shortcuts", {}); +} + +function enterInsertMode(hook) { + if (hook.state.focusedCellId) { + setInsertMode(hook, true); + } +} + +function escapeInsertMode(hook) { + setInsertMode(hook, false); +} + +function moveCellFocus(hook, offset) { + const cellId = nearbyCellId(hook.state.focusedCellId, offset); + setFocusedCell(hook, cellId); +} + +function moveFocusedCell(hook, offset) { + if (hook.state.focusedCellId) { + hook.pushEvent("move_cell", { cell_id: hook.state.focusedCellId, offset }); + } +} + +function insertCellBelowFocused(hook, type) { + if (hook.state.focusedCellId) { + hook.pushEvent("insert_cell_below", { + cell_id: hook.state.focusedCellId, + type, + }); + } else { + // If no cell is focused, insert below the last cell + const cellIds = getCellIds(); + if (cellIds.length > 0) { + const lastCellId = cellIds[cellIds.length - 1]; + hook.pushEvent("insert_cell_below", { cell_id: lastCellId, type }); + } else { + insertFirstCell(hook, type); + } + } +} + +function insertCellAboveFocused(hook, type) { + if (hook.state.focusedCellId) { + hook.pushEvent("insert_cell_above", { + cell_id: hook.state.focusedCellId, + type, + }); + } else { + // If no cell is focused, insert above the first cell + const cellIds = getCellIds(); + if (cellIds.length > 0) { + const lastCellId = cellIds[0]; + hook.pushEvent("insert_cell_above", { cell_id: lastCellId, type }); + } else { + insertFirstCell(hook, type); + } + } +} + +function insertFirstCell(hook, type) { + const sectionIds = getSectionIds(); + + if (sectionIds.length > 0) { + hook.pushEvent("insert_cell", { + section_id: sectionIds[0], + index: 0, + type, + }); + } +} + +function setFocusedCell(hook, cellId) { + hook.state.focusedCellId = cellId; + + if (hook.state.focusedCellId) { + const cell = getCellById(hook.state.focusedCellId); + hook.state.focusedCellType = cell.getAttribute("data-type"); + hook.state.focusedSectionId = getSectionIdByCellId( + hook.state.focusedCellId + ); + } else { + hook.state.focusedCellType = null; + hook.state.focusedSectionId = null; + } + + globalPubSub.broadcast("session", { type: "cell_focused", cellId }); + + setInsertMode(hook, false); +} + +function setInsertMode(hook, insertModeEnabled) { + hook.state.insertMode = insertModeEnabled; + + if (insertModeEnabled) { + hook.el.setAttribute("data-js-insert-mode", "true"); + } else { + hook.el.removeAttribute("data-js-insert-mode"); + } + + globalPubSub.broadcast("session", { + type: "insert_mode_changed", + enabled: insertModeEnabled, + }); +} + +// Server event handlers + +function handleCellInserted(hook, cellId) { + setFocusedCell(hook, cellId); + setInsertMode(hook, true); +} + +function handleCellDeleted(hook, cellId, siblingCellId) { + if (hook.state.focusedCellId === cellId) { + setFocusedCell(hook, siblingCellId); + } +} + +function handleCellMoved(hook, cellId) { + if (hook.state.focusedCellId === cellId) { + globalPubSub.broadcast("session", { type: "cell_moved", cellId }); + + // The cell may have moved to another section, so update this information. + hook.state.focusedSectionId = getSectionIdByCellId( + hook.state.focusedCellId + ); + } +} + +function handleSectionInserted(hook, sectionId) { + const section = getSectionById(sectionId); + const nameElement = section.querySelector("[data-section-name]"); + nameElement.focus({ preventScroll: true }); + selectElementContent(nameElement); + smoothlyScrollToElement(nameElement); +} + +function handleSectionDeleted(hook, sectionId) { + if (hook.state.focusedSectionId === sectionId) { + setFocusedCell(hook, null); + } +} + +// Helpers + +function nearbyCellId(cellId, offset) { + const cellIds = getCellIds(); + + if (cellIds.length === 0) { + return null; + } + + const idx = cellIds.indexOf(cellId); + + if (idx === -1) { + return cellIds[0]; + } else { + const siblingIdx = clamp(idx + offset, 0, cellIds.length - 1); + return cellIds[siblingIdx]; + } +} + +function getCellIds() { + const cells = getCells(); + return cells.map((cell) => cell.getAttribute("data-cell-id")); +} + +function getCells() { + return Array.from(document.querySelectorAll(`[data-element="cell"]`)); +} + +function getCellById(cellId) { + return document.querySelector( + `[data-element="cell"][data-cell-id="${cellId}"]` + ); +} + +function getSectionIdByCellId(cellId) { + const cell = document.querySelector( + `[data-element="cell"][data-cell-id="${cellId}"]` + ); + const section = cell.closest(`[data-element="section"]`); + return section.getAttribute("data-section-id"); +} + +function getSectionIds() { + const sections = getSections(); + return sections.map((section) => section.getAttribute("data-section-id")); +} + +function getSections() { + return Array.from(document.querySelectorAll(`[data-element="section"]`)); +} + +function getSectionById(sectionId) { + return document.querySelector( + `[data-element="section"][data-section-id="${sectionId}"]` + ); } function cancelEvent(event) { diff --git a/assets/js/virtualized_lines/index.js b/assets/js/virtualized_lines/index.js index 491082838..fd38d42f9 100644 --- a/assets/js/virtualized_lines/index.js +++ b/assets/js/virtualized_lines/index.js @@ -1,5 +1,6 @@ import HyperList from "hyperlist"; import { getAttributeOrThrow, parseInteger } from "../lib/attribute"; +import { getLineHeight } from "../lib/utils"; /** * A hook used to render text lines as a virtual list, @@ -20,39 +21,46 @@ import { getAttributeOrThrow, parseInteger } from "../lib/attribute"; const VirtualizedLines = { mounted() { this.props = getProps(this); + this.state = { + lineHeight: null, + templateElement: null, + contentElement: null, + virtualizedList: null, + }; - const computedStyle = window.getComputedStyle(this.el); - this.lineHeight = parseInt(computedStyle.lineHeight, 10); + this.state.lineHeight = getLineHeight(this.el); - this.templateElement = this.el.querySelector('[data-template]'); + this.state.templateElement = this.el.querySelector("[data-template]"); - if (!this.templateElement) { - throw new Error('VirtualizedLines must have a child with data-template attribute'); + if (!this.state.templateElement) { + throw new Error( + "VirtualizedLines must have a child with data-template attribute" + ); } - this.contentElement = this.el.querySelector('[data-content]'); + this.state.contentElement = this.el.querySelector("[data-content]"); - if (!this.templateElement) { - throw new Error('VirtualizedLines must have a child with data-content'); + if (!this.state.templateElement) { + throw new Error("VirtualizedLines must have a child with data-content"); } const config = hyperListConfig( - this.templateElement, + this.state.templateElement, this.props.maxHeight, - this.lineHeight + this.state.lineHeight ); - this.virtualizedList = new HyperList(this.contentElement, config); + this.virtualizedList = new HyperList(this.state.contentElement, config); }, updated() { this.props = getProps(this); const config = hyperListConfig( - this.templateElement, + this.state.templateElement, this.props.maxHeight, - this.lineHeight + this.state.lineHeight ); - this.virtualizedList.refresh(this.contentElement, config); + this.virtualizedList.refresh(this.state.contentElement, config); }, }; diff --git a/assets/package.json b/assets/package.json index 5130b3e49..2eb42f146 100644 --- a/assets/package.json +++ b/assets/package.json @@ -7,7 +7,7 @@ "watch": "webpack --mode development --watch", "format": "prettier --trailing-comma es5 --write {js,test,css}/**/*.{js,json,css,scss,md}", "test": "jest", - "test:watch": "jest" + "test:watch": "jest --watch" }, "dependencies": { "dompurify": "^2.2.6", diff --git a/assets/test/lib/pub_sub.test.js b/assets/test/lib/pub_sub.test.js new file mode 100644 index 000000000..02575f0b2 --- /dev/null +++ b/assets/test/lib/pub_sub.test.js @@ -0,0 +1,27 @@ +import PubSub from "../../js/lib/pub_sub"; + +describe("PubSub", () => { + test("subscribed callback is called on the specified topic", () => { + const pubsub = new PubSub(); + const callback1 = jest.fn(); + const callback2 = jest.fn(); + + pubsub.subscribe('topic1', callback1); + pubsub.subscribe('topic2', callback2); + pubsub.broadcast('topic1', { data: 1 }); + + expect(callback1).toHaveBeenCalledWith({ data: 1 }); + expect(callback2).not.toHaveBeenCalled(); + }); + + test("unsubscribed callback is not called on the specified topic", () => { + const pubsub = new PubSub(); + const callback1 = jest.fn(); + + pubsub.subscribe('topic1', callback1); + pubsub.unsubscribe('topic1', callback1); + pubsub.broadcast('topic1', {}); + + expect(callback1).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/livebook/notebook.ex b/lib/livebook/notebook.ex index 333e13eae..d549cf11a 100644 --- a/lib/livebook/notebook.ex +++ b/lib/livebook/notebook.ex @@ -67,19 +67,18 @@ defmodule Livebook.Notebook do end @doc """ - Finds a cell being `offset` from the given cell within the same section. + Finds a cell being `offset` from the given cell (with regard to all sections). """ @spec fetch_cell_sibling(t(), Cell.id(), integer()) :: {:ok, Cell.t()} | :error def fetch_cell_sibling(notebook, cell_id, offset) do - with {:ok, cell, section} <- fetch_cell_and_section(notebook, cell_id) do - idx = Enum.find_index(section.cells, &(&1 == cell)) - sibling_idx = idx + offset + all_cells = for(section <- notebook.sections, cell <- section.cells, do: cell) - if sibling_idx >= 0 and sibling_idx < length(section.cells) do - {:ok, Enum.at(section.cells, sibling_idx)} - else - :error - end + with idx when idx != nil <- Enum.find_index(all_cells, &(&1.id == cell_id)), + sibling_idx <- idx + offset, + true <- 0 <= sibling_idx and sibling_idx < length(all_cells) do + {:ok, Enum.at(all_cells, sibling_idx)} + else + _ -> :error end end @@ -165,20 +164,48 @@ defmodule Livebook.Notebook do end @doc """ - Moves cell within the given section at the specified position to a new position. - """ - @spec move_cell(t(), Section.id(), non_neg_integer(), non_neg_integer()) :: t() - def move_cell(notebook, section_id, from_idx, to_idx) do - update_section(notebook, section_id, fn section -> - {cell, cells} = List.pop_at(section.cells, from_idx) + Moves cell by the given offset. - if cell do - cells = List.insert_at(cells, to_idx, cell) - %{section | cells: cells} - else - section - end - end) + The cell may move to another section if the offset indicates so. + """ + @spec move_cell(t(), Cell.id(), integer()) :: t() + def move_cell(notebook, cell_id, offset) do + # We firstly create a flat list of cells interspersed with `:separator` + # at section boundaries. Then we move the given cell by the given offset. + # Finally we split the flat list back into cell lists + # and put them in the corresponding sections. + + separated_cells = + notebook.sections + |> Enum.map_intersperse(:separator, & &1.cells) + |> List.flatten() + + idx = + Enum.find_index(separated_cells, fn + :separator -> false + cell -> cell.id == cell_id + end) + + new_idx = (idx + offset) |> clamp_index(separated_cells) + + {cell, separated_cells} = List.pop_at(separated_cells, idx) + separated_cells = List.insert_at(separated_cells, new_idx, cell) + + cell_groups = + separated_cells + |> Enum.chunk_by(&(&1 == :separator)) + |> Enum.reject(&(&1 == [:separator])) + + sections = + notebook.sections + |> Enum.zip(cell_groups) + |> Enum.map(fn {section, cells} -> %{section | cells: cells} end) + + %{notebook | sections: sections} + end + + defp clamp_index(index, list) do + index |> max(0) |> min(length(list) - 1) end @doc """ diff --git a/lib/livebook/session/data.ex b/lib/livebook/session/data.ex index 9e14fe627..f476d4a8b 100644 --- a/lib/livebook/session/data.ex +++ b/lib/livebook/session/data.ex @@ -215,11 +215,11 @@ defmodule Livebook.Session.Data do end def apply_operation(data, {:move_cell, _client_pid, id, offset}) do - with {:ok, cell, section} <- Notebook.fetch_cell_and_section(data.notebook, id), + with {:ok, cell, _section} <- Notebook.fetch_cell_and_section(data.notebook, id), true <- offset != 0 do data |> with_actions() - |> move_cell(cell, section, offset) + |> move_cell(cell, offset) |> set_dirty() |> wrap_ok() else @@ -443,11 +443,8 @@ defmodule Livebook.Session.Data do |> set!(cell_infos: Map.delete(data.cell_infos, cell.id)) end - defp move_cell({data, _} = data_actions, cell, section, offset) do - idx = Enum.find_index(section.cells, &(&1 == cell)) - new_idx = (idx + offset) |> clamp_index(section.cells) - - updated_notebook = Notebook.move_cell(data.notebook, section.id, idx, new_idx) + defp move_cell({data, _} = data_actions, cell, offset) do + updated_notebook = Notebook.move_cell(data.notebook, cell.id, offset) cells_with_section_before = Notebook.elixir_cells_with_section(data.notebook) cells_with_section_after = Notebook.elixir_cells_with_section(updated_notebook) @@ -466,10 +463,6 @@ defmodule Livebook.Session.Data do |> unqueue_cells_evaluation(affected_cells_with_section) end - defp clamp_index(index, list) do - index |> max(0) |> min(length(list) - 1) - end - defp queue_cell_evaluation(data_actions, cell, section) do data_actions |> update_section_info!(section.id, fn section -> diff --git a/lib/livebook_web/live/cell_component.ex b/lib/livebook_web/live/cell_component.ex index e8e11358a..ed6cfd97c 100644 --- a/lib/livebook_web/live/cell_component.ex +++ b/lib/livebook_web/live/cell_component.ex @@ -3,13 +3,12 @@ defmodule LivebookWeb.CellComponent do def render(assigns) do ~L""" -
" +
+ data-type="<%= @cell.type %>"> <%= render_cell_content(assigns) %>
""" @@ -17,34 +16,34 @@ defmodule LivebookWeb.CellComponent do def render_cell_content(%{cell: %{type: :markdown}} = assigns) do ~L""" - <%= if @focused do %> -
- <%= unless @insert_mode do %> - - <% end %> - - - -
- <% end %> +
+ + + + +
-
"> +
<%= render_editor(@cell, @cell_info) %>
-
+
<%= render_markdown_content_placeholder(@cell.source) %>
""" @@ -52,35 +51,41 @@ defmodule LivebookWeb.CellComponent do def render_cell_content(%{cell: %{type: :elixir}} = assigns) do ~L""" - <%= if @focused do %> -
- <%= if @cell_info.evaluation_status == :ready do %> - - <% else %> - - <% end %> - - <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell.id), class: "text-gray-500 hover:text-current" do %> - <%= Icons.svg(:adjustments, class: "h-6") %> - <% end %> +
+ <%= if @cell_info.evaluation_status == :ready do %> + <% else %> -
- <% end %> + <% end %> + + <%= live_patch to: Routes.session_path(@socket, :cell_settings, @session_id, @cell.id), class: "text-gray-500 hover:text-current" do %> + <%= Icons.svg(:adjustments, class: "h-6") %> + <% end %> + + +
<%= render_editor(@cell, @cell_info, show_status: true) %> @@ -100,7 +105,7 @@ defmodule LivebookWeb.CellComponent do
<%= render_editor_content_placeholder(@cell.source) %>
diff --git a/lib/livebook_web/live/path_select_component.ex b/lib/livebook_web/live/path_select_component.ex index 984a5cd7c..d74e341f8 100644 --- a/lib/livebook_web/live/path_select_component.ex +++ b/lib/livebook_web/live/path_select_component.ex @@ -21,7 +21,8 @@ defmodule LivebookWeb.PathSelectComponent do name="path" placeholder="File" value="<%= @path %>" - spellcheck="false" /> + spellcheck="false" + autocomplete="off" />
diff --git a/lib/livebook_web/live/section_component.ex b/lib/livebook_web/live/section_component.ex index 1164ec043..6c8c5bd29 100644 --- a/lib/livebook_web/live/section_component.ex +++ b/lib/livebook_web/live/section_component.ex @@ -3,10 +3,11 @@ defmodule LivebookWeb.SectionComponent do def render(assigns) do ~L""" -
"> +

-
-
+
<%= live_component @socket, LivebookWeb.InsertCellComponent, id: "#{@section.id}:0", section_id: @section.id, @@ -35,9 +36,7 @@ defmodule LivebookWeb.SectionComponent do id: cell.id, session_id: @session_id, cell: cell, - cell_info: @cell_infos[cell.id], - focused: @selected and cell.id == @focused_cell_id, - insert_mode: @insert_mode %> + cell_info: @cell_infos[cell.id] %> <%= live_component @socket, LivebookWeb.InsertCellComponent, id: "#{@section.id}:#{index + 1}", section_id: @section.id, diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index cffb80690..eb603e07b 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -34,20 +34,10 @@ defmodule LivebookWeb.SessionLive do end defp initial_assigns(session_id, data, platform) do - first_section_id = - case data.notebook.sections do - [section | _] -> section.id - [] -> nil - end - %{ platform: platform, session_id: session_id, - data: data, - selected_section_id: first_section_id, - focused_cell_id: nil, - focused_cell_type: nil, - insert_mode: false + data: data } end @@ -87,28 +77,17 @@ defmodule LivebookWeb.SessionLive do
+ data-element="session" + phx-hook="Session">
-

<%= @data.notebook.name %>

- -
+
<%= for section <- @data.notebook.sections do %> -
"> - - <%= section.name %> - -
+ <% end %>
- - <%= if @insert_mode do %> <%# Show a tiny insert indicator for clarity %> -
+
insert
- <% end %> +
""" end @@ -217,40 +201,34 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end - def handle_event("select_section", %{"section_id" => section_id}, socket) do - {:noreply, assign(socket, selected_section_id: section_id)} - end - def handle_event( "insert_cell", %{"section_id" => section_id, "index" => index, "type" => type}, socket ) do - index = String.to_integer(index) |> max(0) + index = ensure_integer(index) |> max(0) type = String.to_atom(type) Session.insert_cell(socket.assigns.session_id, section_id, index, type) {:noreply, socket} end - def handle_event("insert_cell_below_focused", %{"type" => type}, socket) do + def handle_event("insert_cell_below", %{"cell_id" => cell_id, "type" => type}, socket) do type = String.to_atom(type) - insert_cell_next_to_focused(socket.assigns, type, idx_offset: 1) + insert_cell_next_to(socket.assigns, cell_id, type, idx_offset: 1) {:noreply, socket} end - def handle_event("insert_cell_above_focused", %{"type" => type}, socket) do + def handle_event("insert_cell_above", %{"cell_id" => cell_id, "type" => type}, socket) do type = String.to_atom(type) - insert_cell_next_to_focused(socket.assigns, type, idx_offset: 0) + insert_cell_next_to(socket.assigns, cell_id, type, idx_offset: 0) {:noreply, socket} end - def handle_event("delete_focused_cell", %{}, socket) do - if socket.assigns.focused_cell_id do - Session.delete_cell(socket.assigns.session_id, socket.assigns.focused_cell_id) - end + def handle_event("delete_cell", %{"cell_id" => cell_id}, socket) do + Session.delete_cell(socket.assigns.session_id, cell_id) {:noreply, socket} end @@ -290,74 +268,20 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end - def handle_event("focus_cell", %{"cell_id" => nil}, socket) do - {:noreply, focus_cell(socket, nil)} - end - - def handle_event("focus_cell", %{"cell_id" => cell_id}, socket) do - case Notebook.fetch_cell_and_section(socket.assigns.data.notebook, cell_id) do - {:ok, cell, _section} -> - {:noreply, focus_cell(socket, cell)} - - :error -> - {:noreply, socket} - end - end - - def handle_event("move_cell_focus", %{"offset" => offset}, socket) do + def handle_event("move_cell", %{"cell_id" => cell_id, "offset" => offset}, socket) do offset = ensure_integer(offset) - - case new_focused_cell_from_offset(socket.assigns, offset) do - {:ok, cell} -> - {:noreply, focus_cell(socket, cell)} - - :error -> - {:noreply, socket} - end - end - - def handle_event("move_focused_cell", %{"offset" => offset}, socket) do - offset = ensure_integer(offset) - - if socket.assigns.focused_cell_id do - Session.move_cell(socket.assigns.session_id, socket.assigns.focused_cell_id, offset) - end + Session.move_cell(socket.assigns.session_id, cell_id, offset) {:noreply, socket} end - def handle_event("set_insert_mode", %{"enabled" => enabled}, socket) do - if socket.assigns.focused_cell_id do - {:noreply, assign(socket, insert_mode: enabled)} - else - {:noreply, socket} - end - end - - def handle_event("enable_insert_mode", %{}, socket) do - if socket.assigns.focused_cell_id do - {:noreply, assign(socket, insert_mode: true)} - else - {:noreply, socket} - end - end - - def handle_event("queue_focused_cell_evaluation", %{}, socket) do - if socket.assigns.focused_cell_id do - Session.queue_cell_evaluation(socket.assigns.session_id, socket.assigns.focused_cell_id) - end - + def handle_event("queue_cell_evaluation", %{"cell_id" => cell_id}, socket) do + Session.queue_cell_evaluation(socket.assigns.session_id, cell_id) {:noreply, socket} end - def handle_event("queue_section_cells_evaluation", %{}, socket) do - if socket.assigns.selected_section_id do - {:ok, section} = - Notebook.fetch_section( - socket.assigns.data.notebook, - socket.assigns.selected_section_id - ) - + def handle_event("queue_section_cells_evaluation", %{"section_id" => section_id}, socket) do + with {:ok, section} <- Notebook.fetch_section(socket.assigns.data.notebook, section_id) do for cell <- section.cells, cell.type == :elixir do Session.queue_cell_evaluation(socket.assigns.session_id, cell.id) end @@ -366,14 +290,20 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end - def handle_event("queue_child_cells_evaluation", %{}, socket) do - if socket.assigns.focused_cell_id do - {:ok, cell, _section} = - Notebook.fetch_cell_and_section( - socket.assigns.data.notebook, - socket.assigns.focused_cell_id - ) + def handle_event("queue_all_cells_evaluation", _params, socket) do + data = socket.assigns.data + for {cell, _} <- Notebook.elixir_cells_with_section(data.notebook), + data.cell_infos[cell.id].validity_status != :evaluated do + Session.queue_cell_evaluation(socket.assigns.session_id, cell.id) + end + + {:noreply, socket} + end + + def handle_event("queue_child_cells_evaluation", %{"cell_id" => cell_id}, socket) do + with {:ok, cell, _section} <- + Notebook.fetch_cell_and_section(socket.assigns.data.notebook, cell_id) do for {cell, _} <- Notebook.child_cells_with_section(socket.assigns.data.notebook, cell.id) do Session.queue_cell_evaluation(socket.assigns.session_id, cell.id) end @@ -382,10 +312,8 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end - def handle_event("cancel_focused_cell_evaluation", %{}, socket) do - if socket.assigns.focused_cell_id do - Session.cancel_cell_evaluation(socket.assigns.session_id, socket.assigns.focused_cell_id) - end + def handle_event("cancel_cell_evaluation", %{"cell_id" => cell_id}, socket) do + Session.cancel_cell_evaluation(socket.assigns.session_id, cell_id) {:noreply, socket} end @@ -435,46 +363,44 @@ defmodule LivebookWeb.SessionLive do defp after_operation(socket, _prev_socket, {:insert_section, client_pid, _index, section_id}) do if client_pid == self() do - assign(socket, selected_section_id: section_id) + push_event(socket, "section_inserted", %{section_id: section_id}) else socket end end defp after_operation(socket, _prev_socket, {:delete_section, _client_pid, section_id}) do - if section_id == socket.assigns.selected_section_id do - assign(socket, selected_section_id: nil) - else - socket - end + push_event(socket, "section_deleted", %{section_id: section_id}) end defp after_operation(socket, _prev_socket, {:insert_cell, client_pid, _, _, _, cell_id}) do if client_pid == self() do - {:ok, cell, _section} = - Notebook.fetch_cell_and_section(socket.assigns.data.notebook, cell_id) - - focus_cell(socket, cell, insert_mode: true) + push_event(socket, "cell_inserted", %{cell_id: cell_id}) else socket end end defp after_operation(socket, prev_socket, {:delete_cell, _client_pid, cell_id}) do - if cell_id == socket.assigns.focused_cell_id do + # Find a sibling cell that the client would focus if the deleted cell has focus. + sibling_cell_id = case Notebook.fetch_cell_sibling(prev_socket.assigns.data.notebook, cell_id, 1) do {:ok, next_cell} -> - focus_cell(socket, next_cell) + next_cell.id :error -> case Notebook.fetch_cell_sibling(prev_socket.assigns.data.notebook, cell_id, -1) do - {:ok, previous_cell} -> - focus_cell(socket, previous_cell) - - :error -> - focus_cell(socket, nil) + {:ok, previous_cell} -> previous_cell.id + :error -> nil end end + + push_event(socket, "cell_deleted", %{cell_id: cell_id, sibling_cell_id: sibling_cell_id}) + end + + defp after_operation(socket, _prev_socket, {:move_cell, client_pid, cell_id, _offset}) do + if client_pid == self() do + push_event(socket, "cell_moved", %{cell_id: cell_id}) else socket end @@ -506,60 +432,10 @@ defmodule LivebookWeb.SessionLive do end end - defp focus_cell(socket, cell, opts \\ []) - - defp focus_cell(socket, nil = _cell, _opts) do - assign(socket, focused_cell_id: nil, focused_cell_type: nil, insert_mode: false) - end - - defp focus_cell(socket, cell, opts) do - insert_mode? = Keyword.get(opts, :insert_mode, false) - - assign(socket, - focused_cell_id: cell.id, - focused_cell_type: cell.type, - insert_mode: insert_mode? - ) - end - - defp insert_cell_next_to_focused(assigns, type, idx_offset: idx_offset) do - if assigns.focused_cell_id do - {:ok, cell, section} = - Notebook.fetch_cell_and_section(assigns.data.notebook, assigns.focused_cell_id) - - index = Enum.find_index(section.cells, &(&1 == cell)) - Session.insert_cell(assigns.session_id, section.id, index + idx_offset, type) - else - append_cell_to_section(assigns, type) - end - end - - defp append_cell_to_section(assigns, type) do - if assigns.selected_section_id do - {:ok, section} = Notebook.fetch_section(assigns.data.notebook, assigns.selected_section_id) - - end_index = length(section.cells) - - Session.insert_cell(assigns.session_id, section.id, end_index, type) - end - end - - defp new_focused_cell_from_offset(assigns, offset) do - cond do - assigns.focused_cell_id -> - # If a cell is focused, look up the appropriate sibling - Notebook.fetch_cell_sibling(assigns.data.notebook, assigns.focused_cell_id, offset) - - assigns.selected_section_id -> - # If no cell is focused, focus the first one for easier keyboard navigation. - {:ok, section} = - Notebook.fetch_section(assigns.data.notebook, assigns.selected_section_id) - - Enum.fetch(section.cells, 0) - - true -> - :error - end + defp insert_cell_next_to(assigns, cell_id, type, idx_offset: idx_offset) do + {:ok, cell, section} = Notebook.fetch_cell_and_section(assigns.data.notebook, cell_id) + index = Enum.find_index(section.cells, &(&1 == cell)) + Session.insert_cell(assigns.session_id, section.id, index + idx_offset, type) end defp runtime_description(nil), do: "No runtime" diff --git a/lib/livebook_web/live/session_live/shortcuts_component.ex b/lib/livebook_web/live/session_live/shortcuts_component.ex index adbaa89f1..0cbf644e5 100644 --- a/lib/livebook_web/live/session_live/shortcuts_component.ex +++ b/lib/livebook_web/live/session_live/shortcuts_component.ex @@ -20,6 +20,7 @@ defmodule LivebookWeb.SessionLive.ShortcutsComponent do %{seq: "dd", desc: "Delete cell"}, %{seq: "ee", desc: "Evaluate cell"}, %{seq: "es", desc: "Evaluate section"}, + %{seq: "ea", desc: "Evaluate all stale/new cells"}, %{seq: "ej", desc: "Evaluate cells below"}, %{seq: "ex", desc: "Cancel cell evaluation"} ] diff --git a/test/livebook/session/data_test.exs b/test/livebook/session/data_test.exs index a53f544f0..2512f5186 100644 --- a/test/livebook/session/data_test.exs +++ b/test/livebook/session/data_test.exs @@ -309,6 +309,31 @@ defmodule Livebook.Session.DataTest do }, []} = Data.apply_operation(data, operation) end + test "allows moving cells between sections" do + data = + data_after_operations!([ + {:insert_section, self(), 0, "s1"}, + {:insert_section, self(), 1, "s2"}, + # Add cells + {:insert_cell, self(), "s1", 0, :elixir, "c1"}, + {:insert_cell, self(), "s1", 1, :elixir, "c2"}, + {:insert_cell, self(), "s2", 0, :elixir, "c3"}, + {:insert_cell, self(), "s2", 1, :elixir, "c4"} + ]) + + operation = {:move_cell, self(), "c2", 1} + + assert {:ok, + %{ + notebook: %{ + sections: [ + %{cells: [%{id: "c1"}]}, + %{cells: [%{id: "c2"}, %{id: "c3"}, %{id: "c4"}]} + ] + } + }, []} = Data.apply_operation(data, operation) + end + test "marks relevant cells in further sections as stale" do data = data_after_operations!([ diff --git a/test/livebook_web/live/session_live_test.exs b/test/livebook_web/live/session_live_test.exs index ea650c05c..234d594df 100644 --- a/test/livebook_web/live/session_live_test.exs +++ b/test/livebook_web/live/session_live_test.exs @@ -92,85 +92,75 @@ defmodule LivebookWeb.SessionLiveTest do Session.get_data(session_id) end - test "queueing focused cell evaluation", %{conn: conn, session_id: session_id} do + test "queueing cell evaluation", %{conn: conn, session_id: session_id} do section_id = insert_section(session_id) cell_id = insert_cell(session_id, section_id, :elixir, "Process.sleep(10)") {:ok, view, _} = live(conn, "/sessions/#{session_id}") - focus_cell(view, cell_id) - view |> element("#session") - |> render_hook("queue_focused_cell_evaluation", %{}) + |> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id}) assert %{cell_infos: %{^cell_id => %{evaluation_status: :evaluating}}} = Session.get_data(session_id) end - test "cancelling focused cell evaluation", %{conn: conn, session_id: session_id} do + test "cancelling cell evaluation", %{conn: conn, session_id: session_id} do section_id = insert_section(session_id) cell_id = insert_cell(session_id, section_id, :elixir, "Process.sleep(2000)") {:ok, view, _} = live(conn, "/sessions/#{session_id}") - focus_cell(view, cell_id) + view + |> element("#session") + |> render_hook("queue_cell_evaluation", %{"cell_id" => cell_id}) view |> element("#session") - |> render_hook("queue_focused_cell_evaluation", %{}) - - view - |> element("#session") - |> render_hook("cancel_focused_cell_evaluation", %{}) + |> render_hook("cancel_cell_evaluation", %{"cell_id" => cell_id}) assert %{cell_infos: %{^cell_id => %{evaluation_status: :ready}}} = Session.get_data(session_id) end - test "inserting a cell below the focused cell", %{conn: conn, session_id: session_id} do + test "inserting a cell below the given cell", %{conn: conn, session_id: session_id} do section_id = insert_section(session_id) cell_id = insert_cell(session_id, section_id, :elixir) {:ok, view, _} = live(conn, "/sessions/#{session_id}") - focus_cell(view, cell_id) - view |> element("#session") - |> render_hook("insert_cell_below_focused", %{"type" => "markdown"}) + |> render_hook("insert_cell_below", %{"cell_id" => cell_id, "type" => "markdown"}) assert %{notebook: %{sections: [%{cells: [_first_cell, %{type: :markdown}]}]}} = Session.get_data(session_id) end - test "inserting a cell above the focused cell", %{conn: conn, session_id: session_id} do + test "inserting a cell above the given cell", %{conn: conn, session_id: session_id} do section_id = insert_section(session_id) cell_id = insert_cell(session_id, section_id, :elixir) {:ok, view, _} = live(conn, "/sessions/#{session_id}") - focus_cell(view, cell_id) - view |> element("#session") - |> render_hook("insert_cell_above_focused", %{"type" => "markdown"}) + |> render_hook("insert_cell_above", %{"cell_id" => cell_id, "type" => "markdown"}) assert %{notebook: %{sections: [%{cells: [%{type: :markdown}, _first_cell]}]}} = Session.get_data(session_id) end - test "deleting the focused cell", %{conn: conn, session_id: session_id} do + test "deleting the given cell", %{conn: conn, session_id: session_id} do section_id = insert_section(session_id) cell_id = insert_cell(session_id, section_id, :elixir) {:ok, view, _} = live(conn, "/sessions/#{session_id}") - focus_cell(view, cell_id) - view |> element("#session") - |> render_hook("delete_focused_cell", %{}) + |> render_hook("delete_cell", %{"cell_id" => cell_id}) assert %{notebook: %{sections: [%{cells: []}]}} = Session.get_data(session_id) end @@ -201,10 +191,4 @@ defmodule LivebookWeb.SessionLiveTest do cell.id end - - defp focus_cell(view, cell_id) do - view - |> element("#session") - |> render_hook("focus_cell", %{"cell_id" => cell_id}) - end end