-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improve Page Scroll Goals UX #5066
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
Changes from 9 commits
c43b7f9
9f7b983
981b17c
ce1e125
2e0af1f
218c8fc
d557776
db7d989
886beff
838c5d6
bd74c68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,10 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| |> to_form() | ||
|
|
||
| selected_tab = | ||
| if assigns.goal && assigns.goal.page_path do | ||
| "pageviews" | ||
| else | ||
| "custom_events" | ||
| case assigns.goal do | ||
| %{page_path: p, scroll_threshold: s} when not is_nil(p) and s > -1 -> "scroll" | ||
| %{page_path: p} when not is_nil(p) -> "pageviews" | ||
| _goal_or_nil -> "custom_events" | ||
| end | ||
|
|
||
| socket = | ||
|
|
@@ -84,7 +84,13 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| f={f} | ||
| goal={@goal} | ||
| suffix={@context_unique_id} | ||
| current_user={@current_user} | ||
| site={@site} | ||
| /> | ||
| <.scroll_fields | ||
| :if={@selected_tab == "scroll"} | ||
| f={f} | ||
| goal={@goal} | ||
| suffix={@context_unique_id} | ||
| site={@site} | ||
| /> | ||
|
|
||
|
|
@@ -108,7 +114,7 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
|
|
||
| <.title>Add Goal for {@domain}</.title> | ||
|
|
||
| <.tabs selected_tab={@selected_tab} myself={@myself} /> | ||
| <.tabs current_user={@current_user} site={@site} selected_tab={@selected_tab} myself={@myself} /> | ||
|
|
||
| <.custom_event_fields | ||
| :if={@selected_tab == "custom_events"} | ||
|
|
@@ -128,7 +134,14 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| x-show="!tabSelectionInProgress" | ||
| f={f} | ||
| suffix={suffix(@context_unique_id, @tab_sequence_id)} | ||
| current_user={@current_user} | ||
| site={@site} | ||
| x-init="tabSelectionInProgress = false" | ||
| /> | ||
| <.scroll_fields | ||
| :if={@selected_tab == "scroll"} | ||
| x-show="!tabSelectionInProgress" | ||
| f={f} | ||
| suffix={suffix(@context_unique_id, @tab_sequence_id)} | ||
| site={@site} | ||
| x-init="tabSelectionInProgress = false" | ||
| /> | ||
|
|
@@ -158,7 +171,6 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| end | ||
|
|
||
| attr(:f, Phoenix.HTML.Form) | ||
| attr(:current_user, Plausible.Auth.User) | ||
| attr(:site, Plausible.Site) | ||
| attr(:suffix, :string) | ||
| attr(:goal, Plausible.Goal, default: nil) | ||
|
|
@@ -196,16 +208,91 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| x-data="{ firstFocus: true }" | ||
| x-on:focus="if (firstFocus) { $el.select(); firstFocus = false; }" | ||
| /> | ||
| </div> | ||
| """ | ||
| end | ||
|
|
||
| attr(:f, Phoenix.HTML.Form) | ||
| attr(:site, Plausible.Site) | ||
| attr(:suffix, :string) | ||
| attr(:goal, Plausible.Goal, default: nil) | ||
| attr(:rest, :global) | ||
|
|
||
| def scroll_fields(assigns) do | ||
| js = | ||
| if is_nil(assigns.goal) do | ||
| """ | ||
| { | ||
| scrollThreshold: '90', | ||
| pagePath: '', | ||
| displayName: '', | ||
| updateDisplayName() { | ||
| if (this.scrollThreshold && this.pagePath) { | ||
| this.displayName = `Scroll ${this.scrollThreshold}% on ${this.pagePath}` | ||
| } | ||
| } | ||
| } | ||
| """ | ||
| else | ||
| """ | ||
| { | ||
| scrollThreshold: '#{assigns.goal.scroll_threshold}', | ||
| pagePath: '#{assigns.goal.page_path}', | ||
| displayName: '#{assigns.goal.display_name}', | ||
| updateDisplayName() {} | ||
| } | ||
| """ | ||
| end | ||
|
|
||
| assigns = assign(assigns, :js, js) | ||
|
|
||
| ~H""" | ||
| <div id="scroll-form" class="py-2" x-data={@js} {@rest}> | ||
| <.label for={"scroll_threshold_input_#{@suffix}"}> | ||
| Scroll Percentage Threshold (0-100) | ||
| </.label> | ||
|
|
||
| <.input | ||
| :if={Plausible.Stats.ScrollDepth.feature_visible?(@site, @current_user)} | ||
| label="Scroll Depth Threshold (optional)" | ||
| id={"scroll_threshold_input_#{@suffix}"} | ||
| required | ||
| field={@f[:scroll_threshold]} | ||
| type="number" | ||
| value={if @goal && @goal.scroll_threshold > -1, do: @goal.scroll_threshold, else: nil} | ||
| min="0" | ||
macobo marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| max="100" | ||
| step="1" | ||
| x-model="scrollThreshold" | ||
| x-on:change="updateDisplayName" | ||
| /> | ||
|
|
||
| <.label for={"scroll_page_path_input_#{@suffix}"} class="mt-3"> | ||
| Page Path | ||
| </.label> | ||
|
|
||
| <.live_component | ||
| id={"scroll_page_path_input_#{@suffix}"} | ||
| submit_name="goal[page_path]" | ||
| class={[ | ||
| "py-2" | ||
| ]} | ||
| module={ComboBox} | ||
| suggest_fun={fn input, _options -> suggest_page_paths(input, @site) end} | ||
| selected={if @goal && @goal.page_path, do: @goal.page_path} | ||
| creatable | ||
| x-on-selection-change="pagePath = $event.detail.value.displayValue; updateDisplayName()" | ||
| /> | ||
|
|
||
| <.error :for={msg <- Enum.map(@f[:page_path].errors, &translate_error/1)}> | ||
| {msg} | ||
| </.error> | ||
|
|
||
| <.input | ||
| label="Display Name" | ||
| id="scroll_display_name_input" | ||
| field={@f[:display_name]} | ||
| type="text" | ||
| x-model="displayName" | ||
| x-data="{ firstFocus: true }" | ||
| x-on:focus="if (firstFocus) { $el.select(); firstFocus = false; }" | ||
| /> | ||
| </div> | ||
| """ | ||
|
|
@@ -372,9 +459,14 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| def tabs(assigns) do | ||
| ~H""" | ||
| <div class="text-sm mt-6 font-medium dark:text-gray-100">Goal Trigger</div> | ||
| <div class="my-2 text-sm w-full flex rounded border border-gray-300 dark:border-gray-500"> | ||
| <div class="my-2 text-sm w-full flex rounded border border-gray-300 dark:border-gray-500 overflow-hidden"> | ||
| <.custom_events_tab selected?={@selected_tab == "custom_events"} myself={@myself} /> | ||
| <.pageviews_tab selected?={@selected_tab == "pageviews"} myself={@myself} /> | ||
| <.scroll_tab | ||
| :if={Plausible.Stats.ScrollDepth.feature_visible?(@site, @current_user)} | ||
| selected?={@selected_tab == "scroll"} | ||
| myself={@myself} | ||
| /> | ||
| </div> | ||
| """ | ||
| end | ||
|
|
@@ -383,7 +475,7 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| ~H""" | ||
| <a | ||
| class={[ | ||
| "w-1/2 text-center py-2.5 border-r dark:border-gray-500", | ||
| "flex-1 text-center py-2.5 border-r dark:border-gray-500", | ||
| "cursor-pointer", | ||
| @selected? && "shadow-inner font-medium bg-indigo-600 text-white", | ||
| !@selected? && "dark:text-gray-100 text-gray-800" | ||
|
|
@@ -403,7 +495,7 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| ~H""" | ||
| <a | ||
| class={[ | ||
| "w-1/2 text-center py-2.5 cursor-pointer", | ||
| "flex-1 text-center py-2.5 cursor-pointer", | ||
| @selected? && "shadow-inner font-medium bg-indigo-600 text-white", | ||
| !@selected? && "dark:text-gray-100 text-gray-800" | ||
| ]} | ||
|
|
@@ -418,6 +510,25 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do | |
| """ | ||
| end | ||
|
|
||
| def scroll_tab(assigns) do | ||
| ~H""" | ||
| <a | ||
| class={[ | ||
| "flex-1 text-center py-2.5 cursor-pointer border-l dark:border-gray-500", | ||
| @selected? && "shadow-inner font-medium bg-indigo-600 text-white", | ||
| !@selected? && "dark:text-gray-100 text-gray-800" | ||
| ]} | ||
| id="scroll-tab" | ||
| x-on:click={!@selected? && "tabSelectionInProgress = true"} | ||
| phx-click="switch-tab" | ||
| phx-value-tab="scroll" | ||
| phx-target={@myself} | ||
| > | ||
| Scroll | ||
|
||
| </a> | ||
| """ | ||
| end | ||
|
|
||
| def handle_event("switch-tab", %{"tab" => tab}, socket) do | ||
| socket = | ||
| socket | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like there could be a useful tooltip we could add here with context what this means.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think rather than a tooltip, an info paragraph (like in the custom events tab) would perhaps be better? Anyway, not sure if we need extra info there? (cc @metmarkosaric)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's pretty clear due to the headings but it doesn't hurt to add an info paragraph. how about:
maybe then it also makes sense to add an info paragraph to Pageview goals as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense @metmarkosaric. I'll do that as part of the documentation task: https://3.basecamp.com/5308029/buckets/39034214/card_tables/cards/8195964494