From eac88b1ec6295c9151960a93a9c696132e2d152b Mon Sep 17 00:00:00 2001 From: Lee Jarvis Date: Mon, 27 Dec 2021 15:45:15 +0000 Subject: [PATCH 1/6] 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 --- lib/livebook_web/live/session_live.ex | 6 ++++++ .../live/session_live/section_component.ex | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 09f1daa8ecb..d27b206b0fd 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -664,6 +664,12 @@ defmodule LivebookWeb.SessionLive do {:noreply, socket} end + def handle_event("delete_empty_section", %{"section_id" => section_id}, socket) do + Session.delete_section(socket.assigns.session.pid, section_id, true) + + {: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 b0ee4220913..0b0788ca2fc 100644 --- a/lib/livebook_web/live/session_live/section_component.ex +++ b/lib/livebook_web/live/session_live/section_component.ex @@ -77,11 +77,19 @@ 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 %> - <.remix_icon icon="delete-bin-6-line" class="text-xl" /> + <%= if @section_view.cell_views == [] or Enum.all?(@section_view.cell_views, & &1.empty?) do %> + From 75409ff764a508e35eef8467dcbe4f3affdd0009 Mon Sep 17 00:00:00 2001 From: Lee Jarvis Date: Mon, 27 Dec 2021 17:19:11 +0000 Subject: [PATCH 3/6] Check only against empty cell list in `delete_section` --- lib/livebook_web/live/session_live.ex | 33 ++++++++----------- .../live/session_live/section_component.ex | 2 +- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index ea563944f09..baec3ce76c0 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -665,28 +665,23 @@ defmodule LivebookWeb.SessionLive do end def handle_event("delete_section", %{"section_id" => section_id}, socket) do - %{section_views: section_views} = socket.assigns.data_view - section_view = Enum.find(section_views, &(&1.id == section_id)) + {:ok, section} = Notebook.fetch_section(socket.private.data.notebook, section_id) - if section_view do - if section_view.cell_views == [] or Enum.all?(section_view.cell_views, & &1.empty?) do - Session.delete_section(socket.assigns.session.pid, section_id, true) + if section.cells == [] do + Session.delete_section(socket.assigns.session.pid, section_id, true) - {:noreply, socket} - else - {:noreply, - push_patch(socket, - to: - Routes.session_path( - socket, - :delete_section, - socket.assigns.session.id, - section_view.id - ) - )} - end - else {:noreply, socket} + else + {:noreply, + push_patch(socket, + to: + Routes.session_path( + socket, + :delete_section, + socket.assigns.session.id, + section.id + ) + )} end end diff --git a/lib/livebook_web/live/session_live/section_component.ex b/lib/livebook_web/live/session_live/section_component.ex index 72ad580cd4f..061aba5a63d 100644 --- a/lib/livebook_web/live/session_live/section_component.ex +++ b/lib/livebook_web/live/session_live/section_component.ex @@ -77,7 +77,7 @@ 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"]}> - + From 0c17920f182c082e008f7df45e4886f1396001f0 Mon Sep 17 00:00:00 2001 From: Lee Jarvis Date: Mon, 27 Dec 2021 17:32:01 +0000 Subject: [PATCH 5/6] Handle section not existing on deletion --- lib/livebook_web/live/session_live.ex | 37 +++++++++++++++------------ 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index baec3ce76c0..37cf9707eaf 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -665,24 +665,29 @@ defmodule LivebookWeb.SessionLive do end def handle_event("delete_section", %{"section_id" => section_id}, socket) do - {:ok, section} = Notebook.fetch_section(socket.private.data.notebook, section_id) + socket = + case Notebook.fetch_section(socket.private.data.notebook, section_id) do + {:ok, section} -> + if section.cells == [] do + Session.delete_section(socket.assigns.session.pid, section_id, true) + socket + else + push_patch(socket, + to: + Routes.session_path( + socket, + :delete_section, + socket.assigns.session.id, + section.id + ) + ) + end - if section.cells == [] do - Session.delete_section(socket.assigns.session.pid, section_id, true) + :error -> + socket + end - {:noreply, socket} - else - {:noreply, - push_patch(socket, - to: - Routes.session_path( - socket, - :delete_section, - socket.assigns.session.id, - section.id - ) - )} - end + {:noreply, socket} end def handle_event("queue_cell_evaluation", %{"cell_id" => cell_id}, socket) do From 2269ded747fbd1f274b52e0cc04326936e903f3e Mon Sep 17 00:00:00 2001 From: Lee Jarvis Date: Mon, 27 Dec 2021 17:39:03 +0000 Subject: [PATCH 6/6] 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 | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex index 37cf9707eaf..62325aad803 100644 --- a/lib/livebook_web/live/session_live.ex +++ b/lib/livebook_web/live/session_live.ex @@ -667,21 +667,20 @@ defmodule LivebookWeb.SessionLive do 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} -> - if section.cells == [] do - Session.delete_section(socket.assigns.session.pid, section_id, true) - socket - else - push_patch(socket, - to: - Routes.session_path( - socket, - :delete_section, - socket.assigns.session.id, - section.id - ) - ) - end + push_patch(socket, + to: + Routes.session_path( + socket, + :delete_section, + socket.assigns.session.id, + section.id + ) + ) :error -> socket