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

Jump to bottom

Add button for inserting a branching section #2205

Merged
merged 2 commits into from
Sep 15, 2023
Merged

Add button for inserting a branching section #2205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c193afd
Add button for inserting a branching section
jonatanklosko Sep 15, 2023
53220f9
Up
jonatanklosko Sep 15, 2023
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
15 changes: 15 additions & 0 deletions lib/livebook/session.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,14 @@ defmodule Livebook.Session do
GenServer.cast(pid, {:insert_section_into, self(), section_id, index})
end

@doc """
Sends branching section insertion request to the server.
"""
@spec insert_branching_section_into(pid(), Section.id(), non_neg_integer()) :: :ok
def insert_branching_section_into(pid, section_id, index) do
GenServer.cast(pid, {:insert_branching_section_into, self(), section_id, index})
end

@doc """
Sends parent update request to the server.
"""
Expand Down Expand Up @@ -1066,6 +1074,13 @@ defmodule Livebook.Session do
{:noreply, handle_operation(state, operation)}
end

def handle_cast({:insert_branching_section_into, client_pid, section_id, index}, state) do
client_id = client_id(state, client_pid)
# Include new id in the operation, so it's reproducible
operation = {:insert_branching_section_into, client_id, section_id, index, Utils.random_id()}
{:noreply, handle_operation(state, operation)}
end

def handle_cast({:set_section_parent, client_pid, section_id, parent_id}, state) do
client_id = client_id(state, client_pid)
# Include new id in the operation, so it's reproducible
Expand Down
23 changes: 23 additions & 0 deletions lib/livebook/session/data.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ defmodule Livebook.Session.Data do
{:set_notebook_attributes, client_id(), map()}
| {:insert_section, client_id(), index(), Section.id()}
| {:insert_section_into, client_id(), Section.id(), index(), Section.id()}
| {:insert_branching_section_into, client_id(), Section.id(), index(), Section.id()}
| {:set_section_parent, client_id(), Section.id(), parent_id :: Section.id()}
| {:unset_section_parent, client_id(), Section.id()}
| {:insert_cell, client_id(), Section.id(), index(), Cell.type(), Cell.id(), map()}
Expand Down Expand Up @@ -402,6 +403,19 @@ defmodule Livebook.Session.Data do
end
end

def apply_operation(data, {:insert_branching_section_into, _client_id, section_id, index, id}) do
with {:ok, _section} <- Notebook.fetch_section(data.notebook, section_id) do
section = %{Section.new() | id: id}

data
|> with_actions()
|> insert_section_into(section_id, index, section)
|> set_default_section_parent(section)
|> set_dirty()
|> wrap_ok()
end
end

def apply_operation(data, {:set_section_parent, _client_id, section_id, parent_id}) do
with {:ok, section} <- Notebook.fetch_section(data.notebook, section_id),
{:ok, parent_section} <- Notebook.fetch_section(data.notebook, parent_id),
Expand Down Expand Up @@ -1037,6 +1051,15 @@ defmodule Livebook.Session.Data do
)
end

def set_default_section_parent({data, _actions} = data_actions, section) do
parent =
data.notebook
|> Notebook.valid_parents_for(section.id)
|> List.last()

set_section_parent(data_actions, section, parent)
end

defp insert_cell({data, _} = data_actions, section_id, index, cell) do
data_actions
|> set!(
Expand Down
13 changes: 13 additions & 0 deletions lib/livebook_web/live/session_live.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,19 @@ defmodule LivebookWeb.SessionLive do
{:noreply, socket}
end

def handle_event("insert_branching_section_below", params, socket) do
with {:ok, section, index} <-
section_with_next_index(
socket.private.data.notebook,
params["section_id"],
params["cell_id"]
) do
Session.insert_branching_section_into(socket.assigns.session.pid, section.id, index)
end

{:noreply, socket}
end

def handle_event(
"set_section_parent",
%{"section_id" => section_id, "parent_id" => parent_id},
Expand Down
11 changes: 11 additions & 0 deletions lib/livebook_web/live/session_live/insert_buttons_component.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,17 @@ defmodule LivebookWeb.SessionLive.InsertButtonsComponent do
<span>Section</span>
</button>
</.menu_item>
<.menu_item>
<button
role="menuitem"
phx-click="insert_branching_section_below"
phx-value-section_id={@section_id}
phx-value-cell_id={@cell_id}
>
<.remix_icon icon="git-branch-line" />
<span>Branching section</span>
</button>
</.menu_item>
<div class="flex items-center mt-4 mb-1 px-5 text-xs text-gray-400 font-light">
MARKDOWN
</div>
Expand Down
127 changes: 84 additions & 43 deletions lib/livebook_web/live/session_live/section_component.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,44 +60,24 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
<.remix_icon icon="link" class="text-xl" />
</a>
</span>
<.menu
:if={@section_view.valid_parents != [] and not @section_view.has_children?}
id={"section-#{@section_view.id}-branch-menu"}
<.branching_menu
section_view={@section_view}
scope="actions"
position={:bottom_right}
disabled={cannot_branch_out_reason(@section_view) != nil}
>
<:toggle>
<span class="tooltip top" data-tooltip="Branch out from">
<button class="icon-button" aria-label="branch out from other section">
<.remix_icon icon="git-branch-line" class="text-xl flip-horizontally" />
</button>
</span>
</:toggle>
<.menu_item :for={parent <- @section_view.valid_parents}>
<%= if @section_view.parent && @section_view.parent.id == parent.id do %>
<button
class="text-gray-900"
phx-click="unset_section_parent"
phx-value-section_id={@section_view.id}
>
<.remix_icon icon="arrow-right-s-line" />
<span><%= parent.name %></span>
</button>
<% else %>
<button
class="text-gray-500"
phx-click="set_section_parent"
phx-value-section_id={@section_view.id}
phx-value-parent_id={parent.id}
>
<.remix_icon
:if={@section_view.parent && @section_view.parent.id}
icon="arrow-right-s-line"
class="invisible"
/>
<span><%= parent.name %></span>
</button>
<% end %>
</.menu_item>
</.menu>
<span
class="tooltip top"
data-tooltip={cannot_branch_out_reason(@section_view) || "Branch out from"}
>
<button
class={["icon-button", cannot_branch_out_reason(@section_view) && "disabled"]}
aria-label="branch out from other section"
>
<.remix_icon icon="git-branch-line" class="text-xl flip-horizontally" />
</button>
</span>
</.branching_menu>
<span class="tooltip top" data-tooltip="Move up">
<button
class="icon-button"
Expand All @@ -121,8 +101,8 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
</button>
</span>
<span {if @section_view.has_children?,
do: [class: "tooltip left", data_tooltip: "Cannot delete this section because\nother sections branch from it"],
else: [class: "tooltip top", data_tooltip: "Delete"]}>
do: [class: "tooltip left", "data-tooltip": "Cannot delete this section because\nother sections branch from it"],
else: [class: "tooltip top", "data-tooltip": "Delete"]}>
<button
class={["icon-button", @section_view.has_children? && "disabled"]}
aria-label="delete section"
Expand All @@ -136,10 +116,10 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
</div>
<h3
:if={@section_view.parent}
class="mt-1 flex items-end space-x-1 text-sm font-semibold text-gray-800"
class="mt-1 flex items-end space-x-1 font-semibold text-gray-800"
data-el-section-subheadline
>
<span
<div
class="tooltip bottom"
data-tooltip="This section branches out from the main flow
and can be evaluated in parallel"
Expand All @@ -148,8 +128,12 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
icon="git-branch-line"
class="text-lg font-normal flip-horizontally leading-none"
/>
</span>
<span class="leading-none">from ”<%= @section_view.parent.name %>”</span>
</div>
<.branching_menu section_view={@section_view} scope="subheading" position={:bottom_left}>
<div class="text-sm leading-none cursor-pointer">
from ”<%= @section_view.parent.name %>”
</div>
</.branching_menu>
</h3>

<h3
Expand Down Expand Up @@ -202,4 +186,61 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
</section>
"""
end

attr :section_view, :map, required: true
attr :scope, :string, required: true
attr :position, :atom, required: true
attr :disabled, :boolean, default: false

slot :inner_block, required: true

defp branching_menu(assigns) do
~H"""
<.menu
id={"section-#{@section_view.id}-branch-menu-#{@scope}"}
position={@position}
disabled={@disabled}
>
<:toggle>
<%= render_slot(@inner_block) %>
</:toggle>
<%= if @section_view.parent do %>
<.menu_item>
<button
class="text-gray-500"
phx-click="unset_section_parent"
phx-value-section_id={@section_view.id}
>
<.remix_icon icon="close-line" />
<span>Clear</span>
</button>
</.menu_item>
<div class="my-1 border-t border-gray-200"></div>
<% end %>
<.menu_item :for={parent <- @section_view.valid_parents}>
<button
class="text-gray-500"
phx-click="set_section_parent"
phx-value-section_id={@section_view.id}
phx-value-parent_id={parent.id}
>
<.remix_icon
:if={@section_view.parent}
icon="arrow-right-s-line"
class={[(@section_view.parent && @section_view.parent.id == parent.id) || "invisible"]}
/>
<span><%= parent.name %></span>
</button>
</.menu_item>
</.menu>
"""
end

defp cannot_branch_out_reason(%{valid_parents: []}),
do: "No section to branch out from"

defp cannot_branch_out_reason(%{has_children?: true}),
do: "Cannot branch out this section because\nother sections branch from it"

defp cannot_branch_out_reason(_section_view), do: nil
end
45 changes: 45 additions & 0 deletions test/livebook/session/data_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,51 @@ defmodule Livebook.Session.DataTest do
end
end

describe "apply_operation/2 given :insert_branching_section_into" do
test "returns an error given invalid section id" do
data = Data.new()
operation = {:insert_branching_section_into, @cid, "nonexistent", 0, "s1"}
assert :error = Data.apply_operation(data, operation)
end

test "sets the insertion section as parent if it is not branching" do
data =
data_after_operations!([
{:insert_section, @cid, 0, "s1"}
])

operation = {:insert_branching_section_into, @cid, "s1", 0, "s2"}

assert {:ok,
%{
notebook: %{
sections: [%{id: "s1"}, %{id: "s2", parent_id: "s1"}]
},
section_infos: %{"s2" => _}
}, []} = Data.apply_operation(data, operation)
end

test "sets the closest regular section as parent" do
data =
data_after_operations!([
{:insert_section, @cid, 0, "s1"},
{:insert_section, @cid, 1, "s2"},
{:insert_section, @cid, 2, "s3"},
{:set_section_parent, @cid, "s3", "s1"}
])

operation = {:insert_branching_section_into, @cid, "s3", 0, "s4"}

assert {:ok,
%{
notebook: %{
sections: [%{id: "s1"}, %{id: "s2"}, %{id: "s3"}, %{id: "s4", parent_id: "s2"}]
},
section_infos: %{"s4" => _}
}, []} = Data.apply_operation(data, operation)
end
end

describe "apply_operation/2 given :set_section_parent" do
test "returns an error given invalid section id" do
data =
Expand Down
Loading