Skip to content

Commit

Permalink
Simplify components push patch (livebook-dev#2510)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonatanklosko authored Mar 13, 2024
1 parent 1a8d46a commit 622dbae
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 58 deletions.
23 changes: 7 additions & 16 deletions lib/livebook_web/live/file_select_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ defmodule LivebookWeb.FileSelectComponent do
#
# * `:extnames` - a list of file extensions that should be shown
#
# * `:submit_event` - the process event sent on form submission,
# use `nil` for no action
# * `:on_submit` - `%JS{}` to execute on form submission
#
# * `:target` - either a pid or `{component_module, id}` to send
# events to
Expand All @@ -41,7 +40,7 @@ defmodule LivebookWeb.FileSelectComponent do
# Component default attribute values
inner_block: nil,
file_system_select_disabled: false,
submit_event: nil,
on_submit: nil,
# State
current_dir: nil,
deleting_file: nil,
Expand Down Expand Up @@ -100,14 +99,14 @@ defmodule LivebookWeb.FileSelectComponent do
myself={@myself}
/>
<form
id={"#{@id}-path-form"}
class="grow"
phx-change="set_path"
phx-submit={if @submit_event, do: "submit"}
phx-nosubmit={!@submit_event}
phx-target={@myself}
phx-change={JS.push("set_path", target: @myself)}
phx-submit={@on_submit}
phx-nosubmit={@on_submit == nil}
>
<.text_field
id={"#{@id}-input-path"}
id={"#{@id}-path-input"}
aria-label="file path"
phx-hook="FocusOnUpdate"
name="path"
Expand Down Expand Up @@ -518,14 +517,6 @@ defmodule LivebookWeb.FileSelectComponent do
{:noreply, socket}
end

def handle_event("submit", %{}, socket) do
if submit_event = socket.assigns.submit_event do
send_event(socket, submit_event)
end

{:noreply, socket}
end

def handle_event("clear_error", %{}, socket) do
{:noreply, put_error(socket, nil)}
end
Expand Down
13 changes: 0 additions & 13 deletions lib/livebook_web/live/session_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -867,15 +867,6 @@ defmodule LivebookWeb.SessionLive do
def handle_info({:error, error}, socket) do
message = error |> to_string() |> upcase_first()
socket = put_flash(socket, :error, message)

# If there is an immediate patch, we apply it to keep the flash
socket =
receive do
{:push_patch, to} -> push_patch(socket, to: to)
after
0 -> socket
end

{:noreply, socket}
end

Expand Down Expand Up @@ -1014,10 +1005,6 @@ defmodule LivebookWeb.SessionLive do
end
end

def handle_info({:push_patch, to}, socket) do
{:noreply, push_patch(socket, to: to)}
end

def handle_info({:put_flash, kind, message}, socket) do
{:noreply, put_flash(socket, kind, message)}
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,7 @@ defmodule LivebookWeb.SessionLive.AddFileEntryFileComponent do

defp add_file_entry(socket, file_entry) do
Livebook.Session.add_file_entries(socket.assigns.session.pid, [file_entry])
# We can't do push_patch from update/2, so we ask the LV to do so
send(self(), {:push_patch, ~p"/sessions/#{socket.assigns.session.id}"})
socket
push_patch(socket, to: ~p"/sessions/#{socket.assigns.session.id}")
end

defp regular?(file, file_info) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ defmodule LivebookWeb.SessionLive.AddFileEntryUrlComponent do

defp add_file_entry(socket, file_entry) do
Livebook.Session.add_file_entries(socket.assigns.session.pid, [file_entry])
# We can't do push_patch from update/2, so we ask the LV to do so
send(self(), {:push_patch, ~p"/sessions/#{socket.assigns.session.id}"})
socket
push_patch(socket, to: ~p"/sessions/#{socket.assigns.session.id}")
end
end
11 changes: 2 additions & 9 deletions lib/livebook_web/live/session_live/persistence_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
|> put_new_attr(:autosave_interval_s, autosave_interval_s)}
end

def update(%{event: :confirm_file}, socket) do
{:ok, save(socket)}
end

def update(assigns, socket) do
{file, assigns} = Map.pop!(assigns, :file)
{persist_outputs, assigns} = Map.pop!(assigns, :persist_outputs)
Expand Down Expand Up @@ -80,7 +76,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do
hub={@hub}
extnames={[LiveMarkdown.extension()]}
running_files={@running_files}
submit_event={:confirm_file}
on_submit={JS.push("save", target: @myself)}
target={{__MODULE__, @id}}
/>
</div>
Expand Down Expand Up @@ -181,10 +177,7 @@ defmodule LivebookWeb.SessionLive.PersistenceComponent do

Session.save_sync(assigns.session.pid)

# We can't do push_patch from update/2, so we ask the LV to do so
send(self(), {:push_patch, return_to(assigns)})

socket
push_patch(socket, to: return_to(assigns))
end

defp return_to(assigns) do
Expand Down
6 changes: 1 addition & 5 deletions lib/livebook_web/live/settings_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ defmodule LivebookWeb.SettingsLive do
file={@state.file}
extnames={[]}
running_files={[]}
submit_event={:set_autosave_path}
on_submit={JS.push("set_autosave_path")}
file_system_select_disabled={true}
target={self()}
>
Expand Down Expand Up @@ -328,10 +328,6 @@ defmodule LivebookWeb.SettingsLive do
{:noreply, update(socket, :autosave_path_state, &%{&1 | file: file})}
end

def handle_info(:set_autosave_path, socket) do
handle_event("set_autosave_path", %{}, socket)
end

def handle_info({:env_var_set, env_var}, socket) do
idx = Enum.find_index(socket.assigns.env_vars, &(&1.name == env_var.name))

Expand Down
10 changes: 5 additions & 5 deletions test/livebook_web/live/open_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ defmodule LivebookWeb.OpenLiveTest do
path = Path.expand("../../../lib", __DIR__) <> "/"

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

# Render the view separately to make sure it received the :set_file event
Expand All @@ -25,7 +25,7 @@ defmodule LivebookWeb.OpenLiveTest do
path = test_notebook_path("basic")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: Path.dirname(path) <> "/"})

view
Expand All @@ -48,7 +48,7 @@ defmodule LivebookWeb.OpenLiveTest do
{:ok, view, _} = live(conn, ~p"/open/storage")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: tmp_dir <> "/"})

assert view
Expand All @@ -62,7 +62,7 @@ defmodule LivebookWeb.OpenLiveTest do
path = File.cwd!() |> Path.join("nonexistent.livemd")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

assert view
Expand All @@ -80,7 +80,7 @@ defmodule LivebookWeb.OpenLiveTest do
File.chmod!(path, 0o444)

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: tmp_dir <> "/"})

view
Expand Down
8 changes: 4 additions & 4 deletions test/livebook_web/live/session_live_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ defmodule LivebookWeb.SessionLiveTest do
path = Path.join(tmp_dir, "notebook.livemd")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

view
Expand Down Expand Up @@ -971,7 +971,7 @@ defmodule LivebookWeb.SessionLiveTest do
path = Path.join(tmp_dir, "notebook.livemd")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

view
Expand Down Expand Up @@ -1832,7 +1832,7 @@ defmodule LivebookWeb.SessionLiveTest do
{:ok, view, _} = live(conn, ~p"/sessions/#{session.id}/add-file/storage")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

# Validations
Expand Down Expand Up @@ -1862,7 +1862,7 @@ defmodule LivebookWeb.SessionLiveTest do
{:ok, view, _} = live(conn, ~p"/sessions/#{session.id}/add-file/storage")

view
|> element(~s{form[phx-change="set_path"]})
|> element(~s{form[id*="path-form"]})
|> render_change(%{path: path})

view
Expand Down

0 comments on commit 622dbae

Please sign in to comment.