Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't preload goal settings modal to avoid issues with opening in edit mode #5031

Merged
merged 5 commits into from
Jan 30, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/plausible_web/live/components/modal.ex
Original file line number Diff line number Diff line change
@@ -101,6 +101,8 @@ defmodule PlausibleWeb.Live.Components.Modal do

"""

@test_preload_override? Mix.env() in [:test, :ce_test]

use PlausibleWeb, :live_component

alias Phoenix.LiveView
@@ -131,7 +133,11 @@ defmodule PlausibleWeb.Live.Components.Modal do

@impl true
def update(assigns, socket) do
preload? = Map.get(assigns, :preload?, true)
# NOTE: This is a workaround for @test_preload_override? being computed
# at build time, where Mix.env() is available. Otherwise, dialyzer
# complains.
preload_override? = :erlang.phash2(1, 1) == 0 and @test_preload_override?
preload? = preload_override? || Map.get(assigns, :preload?, true)

socket =
assign(socket,
2 changes: 1 addition & 1 deletion lib/plausible_web/live/goal_settings.ex
Original file line number Diff line number Diff line change
@@ -51,7 +51,7 @@ defmodule PlausibleWeb.Live.GoalSettings do
<div id="goal-settings-main">
<.flash_messages flash={@flash} />

<.live_component :let={modal_unique_id} module={Modal} id="goals-form-modal">
<.live_component :let={modal_unique_id} module={Modal} preload?={false} id="goals-form-modal">
<.live_component
module={PlausibleWeb.Live.GoalSettings.Form}
id={"goals-form-#{modal_unique_id}"}
12 changes: 0 additions & 12 deletions test/plausible_web/live/goal_settings_test.exs
Original file line number Diff line number Diff line change
@@ -165,18 +165,6 @@ defmodule PlausibleWeb.Live.GoalSettingsTest do
refute html =~ "No goals found for this site. Please refine or"
end

test "Add Goal form view is rendered immediately, though hidden", %{conn: conn, site: site} do
{:ok, _goals} = setup_goals(site)
{_, html} = get_liveview(conn, site, with_html?: true)

assert html =~ "Add Goal for #{site.domain}"

assert element_exists?(
html,
~s/#goals-form-modal form[phx-submit="save-goal"]/
)
end

test "auto-configuring custom event goals", %{conn: conn, site: site} do
populate_stats(site, [
build(:event, name: "Signup"),

Unchanged files with check annotations Beta

if (currentTab == 'all' && isRemovingFilter('channel')) {
setTab('channels')()
}
}, [query, currentTab])

Check warning on line 183 in assets/js/dashboard/stats/sources/source-list.js

GitHub Actions / Build and test

React Hook useEffect has missing dependencies: 'previousQuery' and 'setTab'. Either include them or remove the dependency array
function setTab(tab) {
return () => {