From 0983a9df77f25f7551da0455c897dc1b0023378b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonatan=20K=C5=82osko?= <jonatanklosko@gmail.com>
Date: Thu, 25 Mar 2021 17:39:18 +0100
Subject: [PATCH] Add confirmation for session deletion (#117)

* Update sections panel

* Add deletion confirm and unify buttons

* Align notebook title

* Fix whitespace shrinking in the editor

* Update session deletion route
---
 assets/css/components.css                     | 46 ++++++++++---------
 assets/css/js_interop.css                     |  2 +-
 assets/js/cell/live_editor/monaco.js          | 15 ++++++
 .../live/delete_session_component.ex          | 36 +++++++++++++++
 lib/livebook_web/live/home_live.ex            | 20 ++++++--
 .../live/insert_buttons_component.ex          |  6 +--
 lib/livebook_web/live/session_live.ex         | 37 ++++++++-------
 .../live/session_live/attached_live.ex        |  2 +-
 .../session_live/cell_settings_component.ex   |  4 +-
 .../session_live/elixir_standalone_live.ex    |  2 +-
 .../live/session_live/mix_standalone_live.ex  |  2 +-
 .../session_live/persistence_component.ex     |  2 +-
 .../live/session_live/runtime_component.ex    |  2 +-
 lib/livebook_web/live/sessions_component.ex   |  8 ++--
 lib/livebook_web/router.ex                    |  1 +
 15 files changed, 127 insertions(+), 58 deletions(-)
 create mode 100644 lib/livebook_web/live/delete_session_component.ex

diff --git a/assets/css/components.css b/assets/css/components.css
index 8533f6cba..05c952664 100644
--- a/assets/css/components.css
+++ b/assets/css/components.css
@@ -1,35 +1,39 @@
 /* Buttons */
 
 .button {
-  @apply px-5 py-2 bg-white rounded-lg border border-gray-200 font-medium text-sm text-gray-600;
+  @apply px-5 py-2 rounded-lg border border-transparent font-medium text-sm;
 }
 
-.button:not(:disabled) {
-  @apply hover:bg-gray-100 focus:bg-gray-100;
+.button-blue {
+  @apply border-transparent bg-blue-600 text-white hover:bg-blue-700 focus:bg-blue-700;
+}
+
+.button-red {
+  @apply border-transparent bg-red-600 text-white hover:bg-red-700 focus:bg-red-700;
+}
+
+.button-gray {
+  @apply border-gray-200 bg-gray-100 text-gray-600 hover:bg-gray-200 focus:bg-gray-200;
+}
+
+.button-outlined-blue {
+  @apply bg-blue-50 border-blue-600 text-blue-600 hover:bg-blue-100 focus:bg-blue-100;
+}
+
+.button-outlined-red {
+  @apply bg-red-50 border-red-600 text-red-600 hover:bg-red-100 focus:bg-red-100;
+}
+
+.button-outlined-gray {
+  @apply bg-white border-gray-300 text-gray-600 hover:bg-gray-100 focus:bg-gray-100;
 }
 
 .button:disabled {
   @apply cursor-default pointer-events-none border-transparent bg-gray-100 text-gray-400;
 }
 
-.button-sm {
-  @apply px-2 py-1;
-}
-
-.button-danger {
-  @apply bg-red-50 border border-red-600 text-red-600;
-}
-
-.button-danger:not(:disabled) {
-  @apply hover:bg-red-100 focus:bg-red-100;
-}
-
-.button-primary {
-  @apply border-transparent bg-blue-600 text-white;
-}
-
-.button-primary:not(:disabled) {
-  @apply hover:bg-blue-700 focus:bg-blue-700;
+.button-small {
+  @apply px-2 py-1 bg-gray-50 border-gray-200 text-gray-600 hover:bg-gray-100 focus:bg-gray-100;
 }
 
 .choice-button {
diff --git a/assets/css/js_interop.css b/assets/css/js_interop.css
index 4a1c21345..9ac882400 100644
--- a/assets/css/js_interop.css
+++ b/assets/css/js_interop.css
@@ -55,7 +55,7 @@ solely client-side operations.
 }
 
 [data-element="section-list-item"][data-js-is-viewed] {
-  @apply text-gray-800;
+  @apply text-gray-900;
 }
 
 [data-element="cell"]:not([data-js-focused]):hover
diff --git a/assets/js/cell/live_editor/monaco.js b/assets/js/cell/live_editor/monaco.js
index 9debf2b71..f494b13e7 100644
--- a/assets/js/cell/live_editor/monaco.js
+++ b/assets/js/cell/live_editor/monaco.js
@@ -64,4 +64,19 @@ monaco.editor.defineTheme("custom", {
   },
 });
 
+// See https://github.com/microsoft/monaco-editor/issues/648#issuecomment-564978560
+// Without this selecting text with whitespace shrinks the whitespace.
+document.fonts.addEventListener("loadingdone", (event) => {
+  const jetBrainsMonoLoaded = event.fontfaces.some(
+    (fontFace) => fontFace.family === "JetBrains Mono"
+  );
+
+  if (jetBrainsMonoLoaded) {
+    // We use JetBrains Mono in all instances of the editor,
+    // so we wait until it loads and then tell Monaco to remeasure
+    // fonts and updates its cache.
+    monaco.editor.remeasureFonts();
+  }
+});
+
 export default monaco;
diff --git a/lib/livebook_web/live/delete_session_component.ex b/lib/livebook_web/live/delete_session_component.ex
new file mode 100644
index 000000000..da96a08e3
--- /dev/null
+++ b/lib/livebook_web/live/delete_session_component.ex
@@ -0,0 +1,36 @@
+defmodule LivebookWeb.DeleteSessionComponent do
+  use LivebookWeb, :live_component
+
+  alias Livebook.SessionSupervisor
+
+  @impl true
+  def render(assigns) do
+    ~L"""
+    <div class="p-6 pb-4 max-w-md w-screen flex flex-col space-y-8">
+      <h3 class="text-2xl font-semibold text-gray-800">
+        Delete session
+      </h3>
+      <p class="text-gray-700">
+        Are you sure you want to delete this section -
+        <span class="font-semibold">
+          “<%= @session_summary.notebook_name %>”
+        </span>?
+        This won't delete any persisted files.
+      </p>
+      <div class="mt-8 flex justify-end space-x-2">
+      <button class="button button-red" phx-click="delete" phx-target="<%= @myself %>">
+        <%= remix_icon("delete-bin-6-line", class: "align-middle mr-1") %>
+          Delete session
+        </button>
+        <%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %>
+      </div>
+    </div>
+    """
+  end
+
+  @impl true
+  def handle_event("delete", %{}, socket) do
+    SessionSupervisor.delete_session(socket.assigns.session_summary.session_id)
+    {:noreply, push_redirect(socket, to: socket.assigns.return_to)}
+  end
+end
diff --git a/lib/livebook_web/live/home_live.ex b/lib/livebook_web/live/home_live.ex
index 44f5c075b..620abd293 100644
--- a/lib/livebook_web/live/home_live.ex
+++ b/lib/livebook_web/live/home_live.ex
@@ -17,6 +17,13 @@ defmodule LivebookWeb.HomeLive do
   @impl true
   def render(assigns) do
     ~L"""
+    <%= if @live_action == :delete_session do %>
+      <%= live_modal @socket, LivebookWeb.DeleteSessionComponent,
+            id: :delete_session_modal,
+            return_to: Routes.home_path(@socket, :page),
+            session_summary: @session_summary %>
+    <% end %>
+
     <div class="flex flex-grow h-full">
       <div class="flex flex-col items-center space-y-6 px-3 py-8 bg-gray-900">
         <%= live_patch to: Routes.home_path(@socket, :page) do %>
@@ -29,7 +36,7 @@ defmodule LivebookWeb.HomeLive do
             <div class="text-2xl text-gray-800 font-semibold">
               Livebook
             </div>
-            <button class="button button-primary"
+            <button class="button button-blue"
               phx-click="new">
               New Notebook
             </button>
@@ -43,7 +50,7 @@ defmodule LivebookWeb.HomeLive do
               target: nil do %>
               <div class="flex justify-end space-x-2">
                 <%= content_tag :button,
-                  class: "button",
+                  class: "button button-outlined-gray",
                   phx_click: "fork",
                   disabled: not path_forkable?(@path) do %>
                   <%= remix_icon("git-branch-line", class: "align-middle mr-1") %>
@@ -51,10 +58,10 @@ defmodule LivebookWeb.HomeLive do
                 <% end %>
                 <%= if path_running?(@path, @session_summaries) do %>
                   <%= live_patch "Join session", to: Routes.session_path(@socket, :page, session_id_by_path(@path, @session_summaries)),
-                    class: "button button-primary" %>
+                    class: "button button-blue" %>
                 <% else %>
                   <%= content_tag :button, "Open",
-                    class: "button button-primary",
+                    class: "button button-blue",
                     phx_click: "open",
                     disabled: not path_openable?(@path, @session_summaries) %>
                 <% end %>
@@ -89,6 +96,11 @@ defmodule LivebookWeb.HomeLive do
   end
 
   @impl true
+  def handle_params(%{"session_id" => session_id}, _url, socket) do
+    session_summary = Enum.find(socket.assigns.session_summaries, &(&1.session_id == session_id))
+    {:noreply, assign(socket, session_summary: session_summary)}
+  end
+
   def handle_params(_params, _url, socket), do: {:noreply, socket}
 
   @impl true
diff --git a/lib/livebook_web/live/insert_buttons_component.ex b/lib/livebook_web/live/insert_buttons_component.ex
index caaec7711..e16c1a249 100644
--- a/lib/livebook_web/live/insert_buttons_component.ex
+++ b/lib/livebook_web/live/insert_buttons_component.ex
@@ -5,20 +5,20 @@ defmodule LivebookWeb.InsertButtonsComponent do
     ~L"""
     <div class="relative top-0.5 m-0 flex justify-center">
       <div class="absolute z-10 <%= if(@persistent, do: "opacity-100", else: "opacity-0") %> hover:opacity-100 focus-within:opacity-100 flex space-x-2 justify-center items-center">
-        <button class="button button-sm"
+        <button class="button button-small"
           phx-click="insert_cell"
           phx-value-type="markdown"
           phx-value-section_id="<%= @section_id %>"
           phx-value-index="<%= @insert_cell_index %>"
           >+ Markdown</button>
-        <button class="button button-sm"
+        <button class="button button-small"
           phx-click="insert_cell"
           phx-value-type="elixir"
           phx-value-section_id="<%= @section_id %>"
           phx-value-index="<%= @insert_cell_index %>"
           >+ Elixir</button>
         <%= if @insert_section_index do %>
-          <button class="button button-sm"
+          <button class="button button-small"
             phx-click="insert_section"
             phx-value-index="<%= @insert_section_index %>"
             >+ Section</button>
diff --git a/lib/livebook_web/live/session_live.ex b/lib/livebook_web/live/session_live.ex
index c099df816..b9e8c4c68 100644
--- a/lib/livebook_web/live/session_live.ex
+++ b/lib/livebook_web/live/session_live.ex
@@ -95,26 +95,29 @@ defmodule LivebookWeb.SessionLive do
           <% end %>
         </span>
       </div>
-      <div class="flex flex-col w-1/5 bg-gray-50 border-r border-gray-100" data-element="sections-panel">
-        <div class="flex-grow flex flex-col space-y-2 pl-4 pt-4"
-          data-element="section-list">
-          <%= for section <- @data.notebook.sections do %>
-            <button class="py-2 px-4 text-left hover:text-gray-800 text-gray-500"
-              data-element="section-list-item"
-              data-section-id="<%= section.id %>">
-              <%= section.name %>
-            </button>
-          <% end %>
-          <button phx-click="add_section" class="py-2 px-4 rounded-l-md cursor-pointer text-gray-300 hover:text-gray-400">
-            <div class="flex items-center space-x-2">
-              <%= remix_icon("add-line", class: "text-2xl") %>
-              <span>New section</span>
-            </div>
+      <div class="flex flex-col w-1/5 bg-gray-50 border-r border-gray-100 px-6 py-10" data-element="sections-panel">
+        <div class="flex-grow flex flex-col">
+          <h3 class="font-semibold text-gray-800 text-lg">
+            Sections
+          </h3>
+          <div class="mt-4 flex flex-col space-y-4" data-element="section-list">
+            <%= for section <- @data.notebook.sections do %>
+              <button class="text-left hover:text-gray-900 text-gray-500"
+                data-element="section-list-item"
+                data-section-id="<%= section.id %>">
+                <%= section.name %>
+              </button>
+            <% end %>
+          </div>
+          <button class="mt-8 p-8 py-1 text-gray-500 text-sm font-medium rounded-xl border border-gray-400 border-dashed hover:bg-gray-100 inline-flex items-center justify-center space-x-2"
+            phx-click="add_section" >
+            <%= remix_icon("add-line", class: "text-lg align-center") %>
+            <span>New section</span>
           </button>
         </div>
       </div>
       <div class="flex-grow overflow-y-auto" data-element="notebook">
-        <div class="py-8 px-16 max-w-screen-lg w-full mx-auto">
+        <div class="py-7 px-16 max-w-screen-lg w-full mx-auto">
           <div class="pb-4 mb-6 border-b border-gray-200">
             <h1 class="text-gray-800 font-semibold text-3xl p-1 -ml-1 rounded-lg border border-transparent hover:border-blue-200 focus:border-blue-300"
               id="notebook-name"
@@ -127,7 +130,7 @@ defmodule LivebookWeb.SessionLive do
           <div class="flex flex-col w-full space-y-16">
             <%= if @data.notebook.sections == [] do %>
               <div class="flex justify-center">
-                <button class="button button-sm"
+                <button class="button button-small"
                   phx-click="insert_section"
                   phx-value-index="0"
                   >+ Section</button>
diff --git a/lib/livebook_web/live/session_live/attached_live.ex b/lib/livebook_web/live/session_live/attached_live.ex
index 6882c3d9a..572c87928 100644
--- a/lib/livebook_web/live/session_live/attached_live.ex
+++ b/lib/livebook_web/live/session_live/attached_live.ex
@@ -43,7 +43,7 @@ defmodule LivebookWeb.SessionLive.AttachedLive do
         <%= text_input f, :name, value: @name, class: "input",
               placeholder: if(Livebook.Config.shortnames?, do: "test", else: "test@127.0.0.1") %>
 
-        <%= submit "Connect", class: "mt-3 button button-primary" %>
+        <%= submit "Connect", class: "mt-3 button button-blue" %>
       </form>
     </div>
     """
diff --git a/lib/livebook_web/live/session_live/cell_settings_component.ex b/lib/livebook_web/live/session_live/cell_settings_component.ex
index b8f0e11c6..5ae44de33 100644
--- a/lib/livebook_web/live/session_live/cell_settings_component.ex
+++ b/lib/livebook_web/live/session_live/cell_settings_component.ex
@@ -31,8 +31,8 @@ defmodule LivebookWeb.SessionLive.CellSettingsComponent do
           </div>
         </div>
         <div class="mt-8 flex justify-end space-x-2">
-          <%= live_patch "Cancel", to: @return_to, class: "button" %>
-          <button class="button button-primary" type="submit">
+          <%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %>
+          <button class="button button-blue" type="submit">
             Save
           </button>
         </div>
diff --git a/lib/livebook_web/live/session_live/elixir_standalone_live.ex b/lib/livebook_web/live/session_live/elixir_standalone_live.ex
index 6bef1084d..821c9462b 100644
--- a/lib/livebook_web/live/session_live/elixir_standalone_live.ex
+++ b/lib/livebook_web/live/session_live/elixir_standalone_live.ex
@@ -21,7 +21,7 @@ defmodule LivebookWeb.SessionLive.ElixirStandaloneLive do
         This is the default runtime and is started automatically
         as soon as you evaluate the first cell.
       </p>
-      <button class="button button-primary" phx-click="init">
+      <button class="button button-blue" phx-click="init">
         <%= if(matching_runtime?(@current_runtime), do: "Reconnect", else: "Connect") %>
       </button>
       <%= if @output do %>
diff --git a/lib/livebook_web/live/session_live/mix_standalone_live.ex b/lib/livebook_web/live/session_live/mix_standalone_live.ex
index 74275e0e2..0cc1b8511 100644
--- a/lib/livebook_web/live/session_live/mix_standalone_live.ex
+++ b/lib/livebook_web/live/session_live/mix_standalone_live.ex
@@ -41,7 +41,7 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do
             target: nil %>
         </div>
         <%= content_tag :button, if(matching_runtime?(@current_runtime, @path), do: "Reconnect", else: "Connect"),
-          class: "button button-primary",
+          class: "button button-blue",
           phx_click: "init",
           disabled: not mix_project_root?(@path) %>
       <% end %>
diff --git a/lib/livebook_web/live/session_live/persistence_component.ex b/lib/livebook_web/live/session_live/persistence_component.ex
index 3ca07954d..aa30c0005 100644
--- a/lib/livebook_web/live/session_live/persistence_component.ex
+++ b/lib/livebook_web/live/session_live/persistence_component.ex
@@ -46,7 +46,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
         <% end %>
         <div>
           <%= content_tag :button, "Save",
-            class: "button button-primary",
+            class: "button button-blue",
             phx_click: "save",
             phx_target: @myself,
             disabled: not path_savable?(normalize_path(@path), @session_summaries) or normalize_path(@path) == @current_path %>
diff --git a/lib/livebook_web/live/session_live/runtime_component.ex b/lib/livebook_web/live/session_live/runtime_component.ex
index aeba74ffa..6d3930ecc 100644
--- a/lib/livebook_web/live/session_live/runtime_component.ex
+++ b/lib/livebook_web/live/session_live/runtime_component.ex
@@ -53,7 +53,7 @@ defmodule LivebookWeb.SessionLive.RuntimeComponent do
               <%= @runtime.node %>
             </span>
           </div>
-          <button class="button text-sm button-danger"
+          <button class="button button-outlined-red"
             type="button"
             phx-click="disconnect"
             phx-target="<%= @myself %>">
diff --git a/lib/livebook_web/live/sessions_component.ex b/lib/livebook_web/live/sessions_component.ex
index 1ac937b8f..deb9dbb69 100644
--- a/lib/livebook_web/live/sessions_component.ex
+++ b/lib/livebook_web/live/sessions_component.ex
@@ -28,13 +28,11 @@ defmodule LivebookWeb.SessionsComponent do
                 <%= remix_icon("git-branch-line") %>
                 <span class="font-medium">Fork</span>
               </button>
-              <button class="flex space-x-3 px-5 py-2 items-center text-red-600 hover:bg-gray-50"
-                phx-click="delete_session"
-                phx-value-id="<%= summary.session_id %>"
-                phx-target="<%= @myself %>">
+              <%= live_patch to: Routes.home_path(@socket, :delete_session, summary.session_id),
+                    class: "flex space-x-3 px-5 py-2 items-center text-red-600 hover:bg-gray-50" do %>
                 <%= remix_icon("delete-bin-6-line") %>
                 <span class="font-medium">Delete</span>
-              </button>
+              <% end %>
             </div>
           </div>
         </div>
diff --git a/lib/livebook_web/router.ex b/lib/livebook_web/router.ex
index 54e3838a8..78a560b00 100644
--- a/lib/livebook_web/router.ex
+++ b/lib/livebook_web/router.ex
@@ -18,6 +18,7 @@ defmodule LivebookWeb.Router do
     pipe_through :browser
 
     live "/", HomeLive, :page
+    live "/home/sessions/:session_id/delete", HomeLive, :delete_session
     live "/sessions/:id", SessionLive, :page
     live "/sessions/:id/shortcuts", SessionLive, :shortcuts
     live "/sessions/:id/settings/:tab", SessionLive, :settings