From f62da92b04f49e80b50f9d69e582f69273c1435c Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Sun, 22 Mar 2026 13:23:25 -0700 Subject: [PATCH 1/9] fix: only show loading in FileSelectComponent when listing files Moves loading state management from event handlers into update_file_infos/2 to ensure loading indicator only appears when actually fetching directory listings, not when running_files changes (e.g., notebooks start/stop). - Remove upfront loading assignments in set_file_system and set_path - Set loading only when dir != current_dir in update_file_infos - Add tests for loading behavior with directory vs running_files changes Fixes #3111 --- .../live/file_select_component.ex | 11 +++- .../live/file_select_component_test.exs | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index dcdef0cf223..0e81d708fd4 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -523,7 +523,7 @@ defmodule LivebookWeb.FileSelectComponent do send_event(socket.assigns.target, {:mount_file_system, file_system}) send_event(socket.assigns.target, {:set_file, file, %{exists: true}}) - {:noreply, assign(socket, loading: true)} + {:noreply, socket} end def handle_event("set_path", %{"path" => path}, socket) do @@ -544,7 +544,7 @@ defmodule LivebookWeb.FileSelectComponent do end send_event(socket.assigns.target, {:set_file, file, info}) - {:noreply, assign(socket, loading: socket.assigns.file.path != path)} + {:noreply, socket} end def handle_event("clear_error", %{}, socket) do @@ -640,8 +640,13 @@ defmodule LivebookWeb.FileSelectComponent do current_file_infos = assigns[:file_infos] || [] {dir, prefix} = dir_and_prefix(assigns.file) + dir_changed? = dir != assigns.current_dir + + # Only show loading when changing directories (which requires listing files) + socket = if dir_changed?, do: assign(socket, :loading, true), else: socket + {file_infos, socket} = - if dir != assigns.current_dir or force_reload? do + if dir_changed? or force_reload? do case get_file_infos(dir, assigns.extnames, assigns.running_files) do {:ok, file_infos} -> {file_infos, assign(socket, :current_dir, dir)} diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index 946e38330e9..87b58fe98b8 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -23,6 +23,61 @@ defmodule LivebookWeb.FileSelectComponentTest do refute render_component(FileSelectComponent, attrs(file: file)) =~ ".." end + test "shows loading when changing directories" do + file = FileSystem.File.local(notebooks_path() <> "/") + + # Initial render in one directory + html = + render_component( + FileSelectComponent, + attrs(file: file, id: "test-loading-dir-change") + ) + + # Loading should not be shown after initial render + refute html =~ ~s(role="status") + + # Simulate changing to a different directory via send_update with a new file + new_file = FileSystem.File.local(p("/")) + + html = + render_component( + FileSelectComponent, + attrs(file: new_file, id: "test-loading-dir-change") + ) + + # After changing directory, loading should be reset to false after listing + refute html =~ ~s(role="status") + end + + test "does not show loading when only running_files changes" do + file = FileSystem.File.local(notebooks_path() <> "/") + running_file = FileSystem.File.local(notebooks_path() <> "/basic.livemd") + + # Initial render with no running files + html = + render_component( + FileSelectComponent, + attrs(file: file, id: "test-loading-running", running_files: []) + ) + + refute html =~ ~s(role="status") + # Verify basic.livemd is shown but not marked as running + assert html =~ "basic.livemd" + refute html =~ "play-circle-line" + + # Update with running_files changed + html = + render_component( + FileSelectComponent, + attrs(file: file, id: "test-loading-running", running_files: [running_file]) + ) + + # Loading should not be shown when only running_files changes + refute html =~ ~s(role="status") + # Verify basic.livemd is now marked as running + assert html =~ "play-circle-line" + end + defp attrs(attrs) do Keyword.merge( [ From 070f77e4ebeb97ad9ff3ac16cde1881838020278 Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Mon, 23 Mar 2026 06:59:11 -0700 Subject: [PATCH 2/9] refactor: remove redundant id from test attrs --- .../live/file_select_component_test.exs | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index 87b58fe98b8..b5986cb0a17 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -27,11 +27,7 @@ defmodule LivebookWeb.FileSelectComponentTest do file = FileSystem.File.local(notebooks_path() <> "/") # Initial render in one directory - html = - render_component( - FileSelectComponent, - attrs(file: file, id: "test-loading-dir-change") - ) + html = render_component(FileSelectComponent, attrs(file: file)) # Loading should not be shown after initial render refute html =~ ~s(role="status") @@ -39,11 +35,7 @@ defmodule LivebookWeb.FileSelectComponentTest do # Simulate changing to a different directory via send_update with a new file new_file = FileSystem.File.local(p("/")) - html = - render_component( - FileSelectComponent, - attrs(file: new_file, id: "test-loading-dir-change") - ) + html = render_component(FileSelectComponent, attrs(file: new_file)) # After changing directory, loading should be reset to false after listing refute html =~ ~s(role="status") @@ -54,11 +46,7 @@ defmodule LivebookWeb.FileSelectComponentTest do running_file = FileSystem.File.local(notebooks_path() <> "/basic.livemd") # Initial render with no running files - html = - render_component( - FileSelectComponent, - attrs(file: file, id: "test-loading-running", running_files: []) - ) + html = render_component(FileSelectComponent, attrs(file: file, running_files: [])) refute html =~ ~s(role="status") # Verify basic.livemd is shown but not marked as running @@ -66,11 +54,7 @@ defmodule LivebookWeb.FileSelectComponentTest do refute html =~ "play-circle-line" # Update with running_files changed - html = - render_component( - FileSelectComponent, - attrs(file: file, id: "test-loading-running", running_files: [running_file]) - ) + html = render_component(FileSelectComponent, attrs(file: file, running_files: [running_file])) # Loading should not be shown when only running_files changes refute html =~ ~s(role="status") From 44c68cdd985c786b557a61ea3fc4bdf80f4afe12 Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Mon, 23 Mar 2026 13:39:38 -0700 Subject: [PATCH 3/9] refactor: use start_async for proper async file listing --- .../live/file_select_component.ex | 60 ++++++++++++++----- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 0e81d708fd4..3d379fac7e9 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -642,22 +642,30 @@ defmodule LivebookWeb.FileSelectComponent do dir_changed? = dir != assigns.current_dir - # Only show loading when changing directories (which requires listing files) - socket = if dir_changed?, do: assign(socket, :loading, true), else: socket + if dir_changed? or force_reload? do + # Start async file listing operation + start_async_file_listing(socket, dir, prefix, current_file_infos) + else + # Just re-annotate with the current prefix (search filter changed) + update_file_display(socket, current_file_infos, prefix) + end + end - {file_infos, socket} = - if dir_changed? or force_reload? do - case get_file_infos(dir, assigns.extnames, assigns.running_files) do - {:ok, file_infos} -> - {file_infos, assign(socket, :current_dir, dir)} + defp start_async_file_listing(socket, dir, prefix, current_file_infos) do + extnames = socket.assigns.extnames + running_files = socket.assigns.running_files - {:error, error} -> - {current_file_infos, put_error(socket, error)} - end - else - {current_file_infos, socket} + socket + |> start_async(:list_files, fn -> + case get_file_infos(dir, extnames, running_files) do + {:ok, file_infos} -> {:ok, file_infos, dir, prefix} + {:error, error} -> {:error, error, current_file_infos, prefix} end + end) + |> assign(loading: true) + end + defp update_file_display(socket, file_infos, prefix) do file_infos = annotate_matching(file_infos, prefix) {unhighlighted_file_infos, highlighted_file_infos} = @@ -670,11 +678,35 @@ defmodule LivebookWeb.FileSelectComponent do assign(socket, file_infos: file_infos, unhighlighted_file_infos: unhighlighted_file_infos, - highlighted_file_infos: highlighted_file_infos, - loading: false + highlighted_file_infos: highlighted_file_infos ) end + @impl true + def handle_async(:list_files, {:ok, {:ok, file_infos, dir, prefix}}, socket) do + socket = + socket + |> assign(current_dir: dir) + |> update_file_display(file_infos, prefix) + |> assign(loading: false) + + {:noreply, socket} + end + + def handle_async(:list_files, {:ok, {:error, error, current_file_infos, prefix}}, socket) do + socket = + socket + |> update_file_display(current_file_infos, prefix) + |> put_error(error) + |> assign(loading: false) + + {:noreply, socket} + end + + def handle_async(:list_files, {:exit, _reason}, socket) do + {:noreply, assign(socket, loading: false)} + end + defp annotate_matching(file_infos, prefix) do for %{name: name} = info <- file_infos do case String.split(name, prefix, parts: 2) do From 52382418fe35c5059533a642f00a410f0b869020 Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Wed, 25 Mar 2026 09:43:16 -0700 Subject: [PATCH 4/9] Update lib/livebook_web/live/file_select_component.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonatan Kłosko --- lib/livebook_web/live/file_select_component.ex | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 3d379fac7e9..46959661a65 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -704,7 +704,12 @@ defmodule LivebookWeb.FileSelectComponent do end def handle_async(:list_files, {:exit, _reason}, socket) do - {:noreply, assign(socket, loading: false)} + socket = + socket + |> put_error("Listing files failed") + |> assign(loading: false) + + {:noreply, socket} end defp annotate_matching(file_infos, prefix) do From d89b3fef7255c248daf76f51b32ef20f9c0d98f8 Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Sun, 29 Mar 2026 18:52:43 -0700 Subject: [PATCH 5/9] test: improve comments explaining async loading behavior in FileSelectComponent --- .../livebook_web/live/file_select_component_test.exs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index b5986cb0a17..621864ca9ec 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -37,7 +37,12 @@ defmodule LivebookWeb.FileSelectComponentTest do html = render_component(FileSelectComponent, attrs(file: new_file)) - # After changing directory, loading should be reset to false after listing + # After changing directory, loading should be reset to false after async listing completes + # Note: render_component is synchronous, so by the time it returns, the async + # file listing via start_async has already completed and loading is false. + # The loading state is properly managed internally: + # 1. When directory changes, start_async is called with loading: true + # 2. When async completes, handle_async sets loading: false refute html =~ ~s(role="status") end @@ -53,10 +58,11 @@ defmodule LivebookWeb.FileSelectComponentTest do assert html =~ "basic.livemd" refute html =~ "play-circle-line" - # Update with running_files changed + # Update with running_files changed (same directory, no dir change) html = render_component(FileSelectComponent, attrs(file: file, running_files: [running_file])) - # Loading should not be shown when only running_files changes + # Loading should not be triggered when only running_files changes + # because update_file_infos detects no directory change refute html =~ ~s(role="status") # Verify basic.livemd is now marked as running assert html =~ "play-circle-line" From 1c467a497332c509c68262352059823a3b90e44a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 15 May 2026 15:21:42 +0200 Subject: [PATCH 6/9] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonatan Kłosko --- .../live/file_select_component.ex | 1 - .../live/file_select_component_test.exs | 47 +------------------ 2 files changed, 1 insertion(+), 47 deletions(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 46959661a65..2d59ba99d1a 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -643,7 +643,6 @@ defmodule LivebookWeb.FileSelectComponent do dir_changed? = dir != assigns.current_dir if dir_changed? or force_reload? do - # Start async file listing operation start_async_file_listing(socket, dir, prefix, current_file_infos) else # Just re-annotate with the current prefix (search filter changed) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index 621864ca9ec..8100d65da79 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -23,52 +23,7 @@ defmodule LivebookWeb.FileSelectComponentTest do refute render_component(FileSelectComponent, attrs(file: file)) =~ ".." end - test "shows loading when changing directories" do - file = FileSystem.File.local(notebooks_path() <> "/") - - # Initial render in one directory - html = render_component(FileSelectComponent, attrs(file: file)) - - # Loading should not be shown after initial render - refute html =~ ~s(role="status") - - # Simulate changing to a different directory via send_update with a new file - new_file = FileSystem.File.local(p("/")) - - html = render_component(FileSelectComponent, attrs(file: new_file)) - - # After changing directory, loading should be reset to false after async listing completes - # Note: render_component is synchronous, so by the time it returns, the async - # file listing via start_async has already completed and loading is false. - # The loading state is properly managed internally: - # 1. When directory changes, start_async is called with loading: true - # 2. When async completes, handle_async sets loading: false - refute html =~ ~s(role="status") - end - - test "does not show loading when only running_files changes" do - file = FileSystem.File.local(notebooks_path() <> "/") - running_file = FileSystem.File.local(notebooks_path() <> "/basic.livemd") - - # Initial render with no running files - html = render_component(FileSelectComponent, attrs(file: file, running_files: [])) - - refute html =~ ~s(role="status") - # Verify basic.livemd is shown but not marked as running - assert html =~ "basic.livemd" - refute html =~ "play-circle-line" - - # Update with running_files changed (same directory, no dir change) - html = render_component(FileSelectComponent, attrs(file: file, running_files: [running_file])) - - # Loading should not be triggered when only running_files changes - # because update_file_infos detects no directory change - refute html =~ ~s(role="status") - # Verify basic.livemd is now marked as running - assert html =~ "play-circle-line" - end - - defp attrs(attrs) do +defp attrs(attrs) do Keyword.merge( [ id: "1", From 978b07332d0319915d73f75aadf30e151bd0dde3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Fri, 15 May 2026 15:21:56 +0200 Subject: [PATCH 7/9] Apply suggestion from @jonatanklosko --- test/livebook_web/live/file_select_component_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index 8100d65da79..946e38330e9 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -23,7 +23,7 @@ defmodule LivebookWeb.FileSelectComponentTest do refute render_component(FileSelectComponent, attrs(file: file)) =~ ".." end -defp attrs(attrs) do + defp attrs(attrs) do Keyword.merge( [ id: "1", From e1afee07927c16d33c89c243094d72335f93cf4e Mon Sep 17 00:00:00 2001 From: dl-alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Sun, 24 May 2026 07:55:52 -0700 Subject: [PATCH 8/9] test: wait for async file select listing --- .../live/file_select_component_test.exs | 68 ++++++++++++++----- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/test/livebook_web/live/file_select_component_test.exs b/test/livebook_web/live/file_select_component_test.exs index 946e38330e9..bef899fad39 100644 --- a/test/livebook_web/live/file_select_component_test.exs +++ b/test/livebook_web/live/file_select_component_test.exs @@ -5,37 +5,69 @@ defmodule LivebookWeb.FileSelectComponentTest do import Livebook.TestHelpers alias Livebook.FileSystem - alias LivebookWeb.FileSelectComponent - test "when the path has a trailing slash, lists that directory" do + test "when the path has a trailing slash, lists that directory", %{conn: conn} do file = FileSystem.File.local(notebooks_path() <> "/") - assert render_component(FileSelectComponent, attrs(file: file)) =~ "basic.livemd" - assert render_component(FileSelectComponent, attrs(file: file)) =~ ".." + html = render_file_select(conn, file) + + assert html =~ "basic.livemd" + assert html =~ ".." end - test "when the path has no trailing slash, lists the parent directory" do + test "when the path has no trailing slash, lists the parent directory", %{conn: conn} do file = FileSystem.File.local(notebooks_path()) - assert render_component(FileSelectComponent, attrs(file: file)) =~ "notebooks" + + assert render_file_select(conn, file) =~ "notebooks" end - test "does not show parent directory when in root" do + test "does not show parent directory when in root", %{conn: conn} do file = FileSystem.File.local(p("/")) - refute render_component(FileSelectComponent, attrs(file: file)) =~ ".." + + refute render_file_select(conn, file) =~ ".." end - defp attrs(attrs) do - Keyword.merge( - [ - id: "1", - file: FileSystem.File.local(p("/")), - extnames: [".livemd"], - running_files: [] - ], - attrs - ) + defp render_file_select(conn, file) do + {:ok, view, _html} = + live_isolated(conn, LivebookWeb.FileSelectComponentTest.Live, + session: %{"path" => file.path} + ) + + render_async(view) end defp notebooks_path() do Path.expand("../../support/notebooks", __DIR__) end + + defmodule Live do + use Phoenix.LiveView, layout: false + + alias Livebook.FileSystem + alias LivebookWeb.FileSelectComponent + + @impl true + def mount(_params, %{"path" => path}, socket) do + socket = + assign(socket, + file: FileSystem.File.local(path), + extnames: [".livemd"], + running_files: [] + ) + + {:ok, socket} + end + + @impl true + def render(assigns) do + ~H""" + <.live_component + module={FileSelectComponent} + id="1" + file={@file} + extnames={@extnames} + running_files={@running_files} + /> + """ + end + end end From dd12aeb7a547261508b028dd06f3c26b0b438071 Mon Sep 17 00:00:00 2001 From: Dalton Alexandre <166029845+dl-alexandre@users.noreply.github.com> Date: Thu, 4 Jun 2026 20:16:44 +0000 Subject: [PATCH 9/9] refactor: set loading: true before start_async in file_select_component (addresses review feedback on loading state management) --- lib/livebook_web/live/file_select_component.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/livebook_web/live/file_select_component.ex b/lib/livebook_web/live/file_select_component.ex index 2d59ba99d1a..b8d901be095 100644 --- a/lib/livebook_web/live/file_select_component.ex +++ b/lib/livebook_web/live/file_select_component.ex @@ -655,13 +655,13 @@ defmodule LivebookWeb.FileSelectComponent do running_files = socket.assigns.running_files socket + |> assign(loading: true) |> start_async(:list_files, fn -> case get_file_infos(dir, extnames, running_files) do {:ok, file_infos} -> {:ok, file_infos, dir, prefix} {:error, error} -> {:error, error, current_file_infos, prefix} end end) - |> assign(loading: true) end defp update_file_display(socket, file_infos, prefix) do