From 4bacba6b1d5c8ce6571396b9a6b0fd1e8a3e81e9 Mon Sep 17 00:00:00 2001
From: Lee Jarvis <lee@jarvis.to>
Date: Mon, 27 Dec 2021 17:42:27 +0000
Subject: [PATCH] Remove delete prompt for empty sections (#829)

* Remove delete prompt for empty sections

If a section has no cell views or only empty cell views, avoid
prompting the user to delete the section and just go ahead and delete
it.

Closes #800

* Move delete prompt logic to SessionLive

This avoids creating two separate paths in the view for displaying
delete buttons (triggering either a prompt, or deleting the empty
section).

Instead, the `delete_section` callback is always triggered, and the
"display prompt" logic is kept here.

Couple of things I'm unsure about so will discuss on the PR.

See https://github.com/livebook-dev/livebook/pull/829#discussion_r775560671

* Check only against empty cell list in `delete_section`

* Fix indentation

* Handle section not existing on deletion

* Match empty cell list in case expression

Also explicitly set the section and then re-use it. I think this is a
bit nicer than just matching against the empty list since it matches
the following match too
---
 lib/livebook_web/live/session_live.ex         | 25 +++++++++++++++++++
 .../live/session_live/section_component.ex    | 10 ++++----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex
index 09f1daa8e..62325aad8 100644
--- a/lib/livebook_web/live/session_live.ex
+++ b/lib/livebook_web/live/session_live.ex
@@ -664,6 +664,31 @@ defmodule LivebookWeb.SessionLive do
     {:noreply, socket}
   end
 
+  def handle_event("delete_section", %{"section_id" => section_id}, socket) do
+    socket =
+      case Notebook.fetch_section(socket.private.data.notebook, section_id) do
+        {:ok, %{cells: []} = section} ->
+          Session.delete_section(socket.assigns.session.pid, section.id, true)
+          socket
+
+        {:ok, section} ->
+          push_patch(socket,
+            to:
+              Routes.session_path(
+                socket,
+                :delete_section,
+                socket.assigns.session.id,
+                section.id
+              )
+          )
+
+        :error ->
+          socket
+      end
+
+    {:noreply, socket}
+  end
+
   def handle_event("queue_cell_evaluation", %{"cell_id" => cell_id}, socket) do
     Session.queue_cell_evaluation(socket.assigns.session.pid, cell_id)
 
diff --git a/lib/livebook_web/live/session_live/section_component.ex b/lib/livebook_web/live/session_live/section_component.ex
index b0ee42209..ec1dfe088 100644
--- a/lib/livebook_web/live/session_live/section_component.ex
+++ b/lib/livebook_web/live/session_live/section_component.ex
@@ -77,12 +77,12 @@ defmodule LivebookWeb.SessionLive.SectionComponent do
             {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"]}>
-            <%= live_patch to: Routes.session_path(@socket, :delete_section, @session_id, @section_view.id),
-                  class: "icon-button #{if @section_view.has_children?, do: "disabled"}",
-                  aria_label: "delete section",
-                  role: "button" do %>
+            <button class={"icon-button #{if @section_view.has_children?, do: "disabled"}"}
+              aria-label="delete section"
+              phx-click="delete_section"
+              phx-value-section_id={@section_view.id}>
               <.remix_icon icon="delete-bin-6-line" class="text-xl" />
-            <% end %>
+            </button>
           </span>
         </div>
       </div>