Add button for inserting a branching section (#2205)

lib/livebook/session.ex

@@ -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.
   """
@@ -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

lib/livebook/session/data.ex

@@ -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()}
@@ -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),
@@ -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!(

lib/livebook_web/live/session_live.ex

@@ -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},

lib/livebook_web/live/session_live/insert_buttons_component.ex

@@ -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>

lib/livebook_web/live/session_live/section_component.ex

@@ -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"
@@ -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"
@@ -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"
@@ -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
@@ -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

test/livebook/session/data_test.exs

@@ -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 =