Skip to content
Merged
Show file tree
Hide file tree
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
47 changes: 38 additions & 9 deletions assets/js/dashboard/components/liveview-portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,58 @@

import React from 'react'
import classNames from 'classnames'
import { ReportHeader } from '../stats/reports/report-header'
import { ReportLayout } from '../stats/reports/report-layout'
import { TabButton, TabWrapper } from './tabs'
import { MoreLinkState } from '../stats/more-link-state'
import MoreLink from '../stats/more-link'

const MIN_HEIGHT = 380
const MIN_HEIGHT = 356

type LiveViewPortalProps = {
id: string
tabs: { value: string; label: string }[]
storageKey: string
className?: string
}

export const LiveViewPortal = React.memo(
function ({ id, className }: LiveViewPortalProps) {
function ({ id, tabs, storageKey, className }: LiveViewPortalProps) {
const activeTab = localStorage.getItem(storageKey) || 'pages'

return (
<div
id={id}
className={classNames('group', className)}
style={{ width: '100%', border: '0', minHeight: MIN_HEIGHT }}
>
<div
className="w-full flex flex-col justify-center group-has-[[data-phx-teleported]]:hidden"
style={{ minHeight: MIN_HEIGHT }}
>
<div className="mx-auto loading">
<div />
</div>
<div className={'group-has-[[data-phx-teleported]]:hidden'}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice improvement in UX ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this gets a bit tricky when we want to replace "Top Pages" -> "Conversion Pages" when filtering by a goal. As soon as the navigation event (let's say applying the goal filter) happens, the component instantly starts to load (optimistically, relying on the phx-navigation-loading class). During that optimistic loading we don't know whether the report title should be Conversion/Top pages. It requires a round-trip to the server to know whether a goal filter is applied or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the green light to revert the dynamic report title change and keep it static as it was before a couple of days ago. Before we do that though, let's make sure there's a plan to overcome the issue with hooks in a deadview. Not a blocker to this PR anyways.

<ReportLayout>
<ReportHeader>
<div className="flex gap-x-3">
<TabWrapper>
{tabs.map(({ value, label }) => (
<TabButton
key={value}
active={activeTab === value}
onClick={() => {}}
>
{label}
</TabButton>
))}
</TabWrapper>
</div>
<MoreLink state={MoreLinkState.LOADING} linkProps={undefined} />
</ReportHeader>
<div
className="w-full flex flex-col justify-center"
style={{ minHeight: `${MIN_HEIGHT}px` }}
>
<div className="mx-auto loading">
<div></div>
</div>
</div>
</ReportLayout>
</div>
</div>
)
Expand Down
15 changes: 14 additions & 1 deletion assets/js/dashboard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { TopBar } from './nav-menu/top-bar'
import Behaviours from './stats/behaviours'
import { useQueryContext } from './query-context'
import { useSiteContext } from './site-context'
import { isRealTimeDashboard } from './util/filters'
import { hasConversionGoalFilter, isRealTimeDashboard } from './util/filters'
import { useAppNavigate } from './navigation/use-app-navigate'
import { parseSearch } from './util/url-search-params'
import { getDomainScopedStorageKey } from './util/storage'

function DashboardStats({
importedDataInView,
Expand All @@ -22,6 +23,7 @@ function DashboardStats({
}) {
const navigate = useAppNavigate()
const site = useSiteContext()
const { query } = useQueryContext()

// Handler for navigation events delegated from LiveView dashboard.
// Necessary to emulate navigation events in LiveView with pushState
Expand Down Expand Up @@ -57,6 +59,17 @@ function DashboardStats({
{site.flags.live_dashboard ? (
<LiveViewPortal
id="pages-breakdown-live"
tabs={[
{
label: hasConversionGoalFilter(query)
? 'Conversion pages'
: 'Top pages',
value: 'pages'
},
{ label: 'Entry pages', value: 'entry-pages' },
{ label: 'Exit pages', value: 'exit-pages' }
]}
storageKey={getDomainScopedStorageKey('pageTab', site.domain)}
className="w-full h-full border-0 overflow-hidden"
/>
) : (
Expand Down
32 changes: 17 additions & 15 deletions assets/js/liveview/dashboard_tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,29 @@ export default buildHook({
this.addListener('click', this.el, (e) => {
const button = e.target.closest('button')
const tabKey = button && button.dataset.tabKey
const span = button && button.querySelector('span')

if (span && span.dataset.active === 'false') {
const reportLabel = button.dataset.reportLabel
if (button && button.closest('div').dataset.active === 'false') {
const storageKey = button.dataset.storageKey
const target = button.dataset.target
const tile = this.el.closest('[data-tile]')
const title = tile.querySelector('[data-title]')

title.innerText = reportLabel

this.el.querySelectorAll(`button[data-tab-key] span`).forEach((s) => {
this.js().setAttribute(s, 'data-active', 'false')
this.el.querySelectorAll(`button[data-tab-key]`).forEach((b) => {
if (b.dataset.tabKey === tabKey) {
this.js().setAttribute(b.closest('div'), 'data-active', 'true')
this.js().setAttribute(
b.querySelector('span'),
'data-active',
'true'
)
} else {
this.js().setAttribute(b.closest('div'), 'data-active', 'false')
this.js().setAttribute(
b.querySelector('span'),
'data-active',
'false'
)
}
})

this.js().setAttribute(
button.querySelector('span'),
'data-active',
'true'
)

if (storageKey) {
localStorage.setItem(`${storageKey}__${domain}`, tabKey)
}
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency's sake, we could adjust the module name as well. Or even change that to dashboard/query_parser.ex and Plausible.Stats.Dashboard.QueryParser, respectively. When aliasing, we can always alias Plausible.Stats.Dashboard and use Dashboard.QueryParser to maintain the context, without redundant repetition of Dashboard.DashboardQueryParser. This applies to other renaming instances as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! 3d5ed50

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
defmodule Plausible.Stats.DashboardQueryParser do
defmodule Plausible.Stats.Dashboard.QueryParser do
@moduledoc """
Parses a dashboard query string into `%ParsedQueryParams{}`. Note that
`metrics` and `dimensions` do not exist at this step yet, and are expected
Expand All @@ -13,7 +13,7 @@ defmodule Plausible.Stats.DashboardQueryParser do
# is false. Even if we don't want to include imported data, we
# might still want to know whether imported data can be toggled
# on/off on the dashboard.
imports_meta: true,
imports_meta: false,
time_labels: false,
total_rows: false,
trim_relative_date_range: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
defmodule Plausible.Stats.DashboardQuerySerializer do
defmodule Plausible.Stats.Dashboard.QuerySerializer do
@moduledoc """
Takes a `%ParsedQueryParams{}` struct and turns it into a query
string.
"""

alias Plausible.Stats.{ParsedQueryParams, DashboardQueryParser, QueryInclude}
alias Plausible.Stats.{ParsedQueryParams, Dashboard, QueryInclude}

def serialize(%ParsedQueryParams{} = params, segments \\ []) do
params
Expand Down Expand Up @@ -62,7 +62,7 @@ defmodule Plausible.Stats.DashboardQuerySerializer do
defp get_serialized_fields(_, _), do: []

defp get_serialized_fields_from_include(:imports, %QueryInclude{} = include) do
if include.imports == DashboardQueryParser.default_include().imports do
if include.imports == Dashboard.QueryParser.default_include().imports do
[]
else
[{"with_imported", to_string(include.imports)}]
Expand All @@ -88,7 +88,7 @@ defmodule Plausible.Stats.DashboardQuerySerializer do

defp get_serialized_fields_from_include(:compare_match_day_of_week, include) do
if include.compare_match_day_of_week ==
DashboardQueryParser.default_include().compare_match_day_of_week do
Dashboard.QueryParser.default_include().compare_match_day_of_week do
[]
else
[{"match_day_of_week", to_string(include.compare_match_day_of_week)}]
Expand Down
47 changes: 47 additions & 0 deletions lib/plausible/stats/dashboard/utils.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
defmodule Plausible.Stats.Dashboard.Utils do
@moduledoc """
Shared utilities by different dashboard reports.
"""

alias Plausible.Site
alias Plausible.Stats.{Dashboard, ParsedQueryParams}

def page_external_link_fn_for(site) do
with true <- Plausible.Sites.regular?(site),
[domain | _] <- String.split(site.domain, "/"),
{:ok, domain} <- idna_encode(domain),
{:ok, uri} <- URI.new("https://#{domain}/") do
fn item ->
"https://#{uri.host}#{hd(item.dimensions)}"
end
else
_ -> nil
end
end

def dashboard_route(%Site{} = site, %ParsedQueryParams{} = params, opts) do
path = Keyword.get(opts, :path, "")

params =
case Keyword.get(opts, :filter) do
nil -> params
filter -> ParsedQueryParams.add_or_replace_filter(params, filter)
end

query_string =
case Dashboard.QuerySerializer.serialize(params) do
"" -> ""
query_string -> "?" <> query_string
end

"/" <> site.domain <> path <> query_string
end

defp idna_encode(domain) do
try do
{:ok, domain |> String.to_charlist() |> :idna.encode() |> IO.iodata_to_binary()}
catch
_ -> {:error, :invalid_domain}
end
end
end
1 change: 1 addition & 0 deletions lib/plausible/stats/metrics.ex
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ defmodule Plausible.Stats.Metrics do
def dashboard_metric_label(:visitors, _context), do: "Visitors"

def dashboard_metric_label(:conversion_rate, _context), do: "CR"
def dashboard_metric_label(:group_conversion_rate, _context), do: "CR"

def dashboard_metric_label(metric, _context), do: "#{metric}"
end
41 changes: 2 additions & 39 deletions lib/plausible_web/live/components/dashboard/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,17 @@ defmodule PlausibleWeb.Components.Dashboard.Base do

use PlausibleWeb, :component

alias Plausible.Stats.DashboardQuerySerializer
alias Plausible.Stats.ParsedQueryParams

attr :site, Plausible.Site, required: true
attr :params, :map, required: true
attr :path, :string, default: ""
attr :to, :string, required: true
attr :class, :string, default: ""
attr :rest, :global

slot :inner_block, required: true

def dashboard_link(assigns) do
query_string = DashboardQuerySerializer.serialize(assigns.params)
url = "/" <> assigns.site.domain <> assigns.path

url =
if query_string != "" do
url <> "?" <> query_string
else
url
end

assigns = assign(assigns, :url, url)

~H"""
<.link
data-type="dashboard-link"
patch={@url}
patch={@to}
class={@class}
{@rest}
>
Expand All @@ -41,26 +24,6 @@ defmodule PlausibleWeb.Components.Dashboard.Base do
"""
end

attr :site, Plausible.Site, required: true
attr :params, :map, required: true
attr :filter, :list, required: true
attr :class, :string, default: ""
attr :rest, :global

slot :inner_block, required: true

def filter_link(assigns) do
params = ParsedQueryParams.add_or_replace_filter(assigns.params, assigns.filter)

assigns = assign(assigns, :params, params)

~H"""
<.dashboard_link site={@site} params={@params} class={@class} {@rest}>
{render_slot(@inner_block)}
</.dashboard_link>
"""
end

attr :style, :string, default: ""
attr :background_class, :string, default: ""
attr :width, :integer, required: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
defmodule PlausibleWeb.Components.Dashboard.ImportedDataWarnings do
@moduledoc false

use PlausibleWeb, :component
alias Phoenix.LiveView.AsyncResult
alias Plausible.Stats.QueryResult

def unsupported_filters(assigns) do
show? =
case assigns.query_result do
%AsyncResult{result: %QueryResult{meta: meta}} ->
meta[:imports_skip_reason] == :unsupported_query

_ ->
false
end

assigns = assign(assigns, :show?, show?)

~H"""
<div :if={@show?} data-test-id="unsupported-filters-warning">
<span class="hidden">
Imported data is excluded due to the applied filters
</span>
<Heroicons.exclamation_circle class="mb-1 size-4.5 text-gray-500 dark:text-gray-400" />
</div>
"""
end
end
3 changes: 2 additions & 1 deletion lib/plausible_web/live/components/dashboard/metric.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ defmodule PlausibleWeb.Components.Dashboard.Metric do

@formatters %{
visitors: :number_short,
conversion_rate: :percentage
conversion_rate: :percentage,
group_conversion_rate: :percentage
}

attr :name, :atom, required: true
Expand Down
Loading
Loading