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

Start new notebooks with a focused code cell #527

Merged
merged 2 commits into from
Aug 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed

- Improved intellisense to handle structs and sigils ([#513](https://github.com/livebook-dev/livebook/pull/513))
- Create new notebooks with an already focused code cell ([#527](https://github.com/livebook-dev/livebook/pull/527))

### Fixed

Expand Down
53 changes: 38 additions & 15 deletions assets/js/session/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
selectElementContent,
smoothlyScrollToElement,
} from "../lib/utils";
import { getAttributeOrDefault } from "../lib/attribute";
import KeyBuffer from "./key_buffer";
import { globalPubSub } from "../lib/pub_sub";
import monaco from "../cell/live_editor/monaco";
Expand All @@ -18,6 +19,11 @@ import monaco from "../cell/live_editor/monaco";
* to communicate between this global hook and cells and for
* that we use a simple local pubsub that the hooks subscribe to.
*
* Configuration:
*
* * `data-autofocus-cell-id` - id of the cell that gets initial
* focus once the notebook is loaded
*
* ## Shortcuts
*
* This hook registers session shortcut handlers,
Expand Down Expand Up @@ -51,6 +57,7 @@ import monaco from "../cell/live_editor/monaco";
*/
const Session = {
mounted() {
this.props = getProps(this);
this.state = {
focusedCellId: null,
focusedSectionId: null,
Expand Down Expand Up @@ -117,7 +124,7 @@ const Session = {
window.addEventListener(
"phx:page-loading-stop",
() => {
focusCellFromUrl(this);
initializeFocus(this);
},
{ once: true }
);
Expand Down Expand Up @@ -205,6 +212,16 @@ const Session = {
},
};

function getProps(hook) {
return {
autofocusCellId: getAttributeOrDefault(
hook.el,
"data-autofocus-cell-id",
null
),
};
}

/**
* Data of a specific LV client.
*
Expand Down Expand Up @@ -481,24 +498,30 @@ function handleCellIndicatorsClick(hook, event) {
}

/**
* Focuses cell based on the given URL.
* Focuses cell or any other element based on the current
* URL and hook attributes.
*/
function focusCellFromUrl(hook) {
function initializeFocus(hook) {
const hash = window.location.hash;

if (hash.startsWith("#cell-")) {
const cellId = hash.replace(/^#cell-/, "");
if (getCellById(cellId)) {
setFocusedCell(hook, cellId);
}
} else {
// Explicitly scroll to the target element
// after the loading finishes
const htmlId = hash.replace(/^#/, "");
const element = document.getElementById(htmlId);
if (element) {
element.scrollIntoView();
if (hash) {
if (hash.startsWith("#cell-")) {
const cellId = hash.replace(/^#cell-/, "");
if (getCellById(cellId)) {
setFocusedCell(hook, cellId);
}
} else {
// Explicitly scroll to the target element
// after the loading finishes
const htmlId = hash.replace(/^#/, "");
const element = document.getElementById(htmlId);
if (element) {
element.scrollIntoView();
}
}
} else if (hook.props.autofocusCellId) {
setFocusedCell(hook, hook.props.autofocusCellId, false);
setInsertMode(hook, true);
}
}

Expand Down
8 changes: 6 additions & 2 deletions lib/livebook/session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -398,11 +398,11 @@ defmodule Livebook.Session do
end

defp init_data(opts) do
notebook = opts[:notebook]
notebook = Keyword.get_lazy(opts, :notebook, &default_notebook/0)
file = opts[:file]
origin = opts[:origin]

data = if(notebook, do: Data.new(notebook), else: Data.new())
data = Data.new(notebook)
data = %{data | origin: origin}

if file do
Expand All @@ -418,6 +418,10 @@ defmodule Livebook.Session do
end
end

defp default_notebook() do
%{Notebook.new() | sections: [%{Section.new() | cells: [Cell.new(:elixir)]}]}
end

defp schedule_autosave(state) do
if interval_s = state.data.notebook.autosave_interval_s do
ref = Process.send_after(self(), :autosave, interval_s * 1000)
Expand Down
15 changes: 12 additions & 3 deletions lib/livebook_web/live/session_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ defmodule LivebookWeb.SessionLive do
session_pid: session_pid,
current_user: current_user,
self: self(),
data_view: data_to_view(data)
data_view: data_to_view(data),
autofocus_cell_id: autofocus_cell_id(data.notebook)
)
|> assign_private(data: data)
|> allow_upload(:cell_image,
Expand Down Expand Up @@ -74,7 +75,8 @@ defmodule LivebookWeb.SessionLive do
<div class="flex flex-grow h-full"
id="session"
data-element="session"
phx-hook="Session">
phx-hook="Session"
data-autofocus-cell-id={@autofocus_cell_id}>
<SidebarHelpers.sidebar>
<SidebarHelpers.logo_item socket={@socket} />
<SidebarHelpers.button_item
Expand Down Expand Up @@ -952,7 +954,11 @@ defmodule LivebookWeb.SessionLive do
end
end

defp after_operation(socket, _prev_socket, {:delete_section, _client_pid, section_id}) do
defp after_operation(
socket,
_prev_socket,
{:delete_section, _client_pid, section_id, _delete_cells}
) do
push_event(socket, "section_deleted", %{section_id: section_id})
end

Expand Down Expand Up @@ -1092,6 +1098,9 @@ defmodule LivebookWeb.SessionLive do

defp process_intellisense_response(response, _request), do: response

defp autofocus_cell_id(%Notebook{sections: [%{cells: [%{id: id, source: ""}]}]}), do: id
defp autofocus_cell_id(_notebook), do: nil

# Builds view-specific structure of data by cherry-picking
# only the relevant attributes.
# We then use `@data_view` in the templates and consequently
Expand Down
6 changes: 3 additions & 3 deletions test/livebook/session_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ defmodule Livebook.SessionTest do
Session.save(session_id)

assert_receive {:operation, {:mark_as_not_dirty, _}}
assert FileSystem.File.read(file) == {:ok, "# My notebook\n"}
assert {:ok, "# My notebook\n" <> _rest} = FileSystem.File.read(file)
end

@tag :tmp_dir
Expand All @@ -325,7 +325,7 @@ defmodule Livebook.SessionTest do
Session.save(session_id)

assert_receive {:operation, {:mark_as_not_dirty, _}}
assert FileSystem.File.read(file) == {:ok, "# My notebook\n"}
assert {:ok, "# My notebook\n" <> _rest} = FileSystem.File.read(file)
end
end

Expand All @@ -347,7 +347,7 @@ defmodule Livebook.SessionTest do
Session.close(session_id)

assert_receive :session_closed
assert FileSystem.File.read(file) == {:ok, "# My notebook\n"}
assert {:ok, "# My notebook\n" <> _rest} = FileSystem.File.read(file)
end

test "clears session temporary directory", %{session_id: session_id} do
Expand Down
8 changes: 8 additions & 0 deletions test/livebook_web/controllers/session_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ defmodule LivebookWeb.SessionControllerTest do

assert conn.resp_body == """
# Untitled notebook

## Section

```elixir

```
"""

SessionSupervisor.close_session(session_id)
Expand Down Expand Up @@ -124,6 +130,8 @@ defmodule LivebookWeb.SessionControllerTest do

assert conn.resp_body == """
# Title: Untitled notebook

# ── Section ──
"""

SessionSupervisor.close_session(session_id)
Expand Down
6 changes: 3 additions & 3 deletions test/livebook_web/live/session_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ defmodule LivebookWeb.SessionLiveTest do
alias Livebook.Users.User

setup do
{:ok, session_id} = SessionSupervisor.create_session()
{:ok, session_id} = SessionSupervisor.create_session(notebook: Livebook.Notebook.new())
%{session_id: session_id}
end

Expand Down Expand Up @@ -54,7 +54,7 @@ defmodule LivebookWeb.SessionLiveTest do

cell_id = insert_text_cell(session_id, section_id, :markdown)

assert render(view) =~ cell_id
assert render(view) =~ "cell-" <> cell_id
end

test "un-renders a deleted cell", %{conn: conn, session_id: session_id} do
Expand All @@ -66,7 +66,7 @@ defmodule LivebookWeb.SessionLiveTest do
Session.delete_cell(session_id, cell_id)
wait_for_session_update(session_id)

refute render(view) =~ cell_id
refute render(view) =~ "cell-" <> cell_id
end
end

Expand Down