Improve path selector (#159)

* Synchronously save file when the location is chosen and fix default path

* Handle enter press in the path selector

lib/livebook/session.ex

@@ -242,6 +242,14 @@ defmodule Livebook.Session do
     GenServer.cast(name(session_id), :save)
   end
 
+  @doc """
+  Synchronous version of `save/1`.
+  """
+  @spec save_sync(id()) :: :ok
+  def save_sync(session_id) do
+    GenServer.call(name(session_id), :save)
+  end
+
   @doc """
   Asynchronously sends a close request to the server.
 
@@ -316,6 +324,10 @@ defmodule Livebook.Session do
     {:reply, summary_from_state(state), state}
   end
 
+  def handle_call(:save, _from, state) do
+    {:reply, :ok, maybe_save_notebook(state)}
+  end
+
   @impl true
   def handle_cast({:insert_section, client_pid, index}, state) do
     # Include new id in the operation, so it's reproducible

lib/livebook_web/live/home_live.ex

@@ -43,7 +43,8 @@ defmodule LivebookWeb.HomeLive do
               path: @path,
               extnames: [LiveMarkdown.extension()],
               running_paths: paths(@session_summaries),
-              target: nil do %>
+              phx_target: nil,
+              phx_submit: nil do %>
               <div class="flex justify-end space-x-2">
                 <%= content_tag :button,
                   class: "button button-outlined-gray",

lib/livebook_web/live/path_select_component.ex

@@ -5,8 +5,9 @@ defmodule LivebookWeb.PathSelectComponent do
   #
   # * `path` - the currently entered path
   # * `running_paths` - the list of notebook paths that are already linked to running sessions
-  # * `target` - id of the component to send update events to or nil to send to the parent LV
   # * `extnames` - a list of file extensions that should be shown
+  # * `phx_target` - id of the component to send update events to or nil to send to the parent LV
+  # * `phx_submit` - the event name sent on form submission, use `nil` for no action
   #
   # The target receives `set_path` events with `%{"path" => path}` payload.
   #
@@ -26,8 +27,12 @@ defmodule LivebookWeb.PathSelectComponent do
       <div class="flex space-x-5 items-center mb-4">
         <form class="flex-grow"
           phx-change="set_path"
-          phx-submit="set_path"
-          <%= if @target, do: "phx-target=#{@target}" %>>
+          <%= if @phx_submit do %>
+            phx-submit="<%= @phx_submit %>"
+          <% else %>
+            onsubmit="return false"
+          <% end %>
+          <%= if @phx_target, do: "phx-target=#{@phx_target}" %>>
           <input class="input"
             id="input-path"
             phx-hook="FocusOnUpdate"
@@ -47,7 +52,7 @@ defmodule LivebookWeb.PathSelectComponent do
       <div class="flex-grow -m-1 p-1 overflow-y-auto tiny-scrollbar">
         <div class="grid grid-cols-4 gap-2">
           <%= for file <- list_matching_files(@path, @extnames, @running_paths) do %>
-            <%= render_file(file, @target) %>
+            <%= render_file(file, @phx_target) %>
           <% end %>
         </div>
       </div>
@@ -55,7 +60,7 @@ defmodule LivebookWeb.PathSelectComponent do
     """
   end
 
-  defp render_file(file, target) do
+  defp render_file(file, phx_target) do
     icon =
       case file do
         %{is_running: true} -> "play-circle-line"
@@ -69,7 +74,7 @@ defmodule LivebookWeb.PathSelectComponent do
     <button class="flex space-x-2 items-center p-2 rounded-lg hover:bg-gray-100 focus:ring-1 focus:ring-gray-400"
       phx-click="set_path"
       phx-value-path="<%= @file.path %>"
-      <%= if target, do: "phx-target=#{target}" %>>
+      <%= if phx_target, do: "phx-target=#{phx_target}" %>>
       <span class="block">
         <%= remix_icon(@icon, class: "text-xl align-middle #{if(@file.is_running, do: "text-green-300", else: "text-gray-400")}") %>
       </span>

lib/livebook_web/live/session_live/mix_standalone_live.ex

@@ -38,12 +38,13 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do
             path: @path,
             extnames: [],
             running_paths: [],
-            target: nil %>
+            phx_target: nil,
+            phx_submit: if(disabled?(@path), do: nil, else: "init") %>
         </div>
         <%= content_tag :button, if(matching_runtime?(@current_runtime, @path), do: "Reconnect", else: "Connect"),
           class: "button button-blue",
           phx_click: "init",
-          disabled: not mix_project_root?(@path) %>
+          disabled: disabled?(@path) %>
       <% end %>
       <%= if @status != :initial do %>
         <div class="markdown">
@@ -58,12 +59,6 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do
     """
   end
 
-  defp matching_runtime?(%Runtime.MixStandalone{} = runtime, path) do
-    Path.expand(runtime.project_path) == Path.expand(path)
-  end
-
-  defp matching_runtime?(_runtime, _path), do: false
-
   @impl true
   def handle_event("set_path", %{"path" => path}, socket) do
     {:noreply, assign(socket, path: path)}
@@ -109,4 +104,14 @@ defmodule LivebookWeb.SessionLive.MixStandaloneLive do
   defp mix_project_root?(path) do
     File.dir?(path) and File.exists?(Path.join(path, "mix.exs"))
   end
+
+  defp matching_runtime?(%Runtime.MixStandalone{} = runtime, path) do
+    Path.expand(runtime.project_path) == Path.expand(path)
+  end
+
+  defp matching_runtime?(_runtime, _path), do: false
+
+  defp disabled?(path) do
+    not mix_project_root?(path)
+  end
 end

lib/livebook_web/live/session_live/persistence_component.ex

@@ -36,7 +36,8 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
             path: @path,
             extnames: [LiveMarkdown.extension()],
             running_paths: @running_paths,
-            target: @myself %>
+            phx_target: @myself,
+            phx_submit: if(disabled?(@path, @current_path, @running_paths), do: nil, else: "save") %>
         </div>
       <% end %>
       <div class="flex flex-col space-y-2">
@@ -50,7 +51,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
             class: "button button-blue mt-2",
             phx_click: "save",
             phx_target: @myself,
-            disabled: not path_savable?(normalize_path(@path), @running_paths) or normalize_path(@path) == @current_path %>
+            disabled: disabled?(@path, @current_path, @running_paths) %>
         </div>
       </div>
     </div>
@@ -61,7 +62,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
   def handle_event("set_persistence_type", %{"type" => type}, socket) do
     path =
       case type do
-        "file" -> default_path()
+        "file" -> socket.assigns.current_path || default_path()
         "memory" -> nil
       end
 
@@ -75,6 +76,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
   def handle_event("save", %{}, socket) do
     path = normalize_path(socket.assigns.path)
     Session.set_path(socket.assigns.session_id, path)
+    Session.save_sync(socket.assigns.session_id)
 
     running_paths =
       if path do
@@ -109,4 +111,8 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
       path <> LiveMarkdown.extension()
     end
   end
+
+  defp disabled?(path, current_path, running_paths) do
+    not path_savable?(normalize_path(path), running_paths) or normalize_path(path) == current_path
+  end
 end

test/livebook_web/live/path_select_component_test.exs

@@ -29,7 +29,14 @@ defmodule LivebookWeb.PathSelectComponentTest do
 
   defp attrs(attrs) do
     Keyword.merge(
-      [id: 1, path: "/", extnames: [".livemd"], running_paths: [], target: nil],
+      [
+        id: 1,
+        path: "/",
+        extnames: [".livemd"],
+        running_paths: [],
+        phx_target: nil,
+        phx_submit: nil
+      ],
       attrs
     )
   end