Dashboard filtering, automatic tracking, chart rebuild & settings modal #40
Reference in New Issue
Block a user
No description provided.
Delete Branch "staging"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Changes
Dashboard Filtering
Automatic Tracking (Script Features)
Chart Rebuild
Chart.tsxfrom scratch — removed sparklines, wider Y-axis with integer-only ticks, lighter grid, streamlined toolbar with icon-only exportgetComputedStyle)tabular-numscausing font fallback on KPI numbersDashboard UI
Settings Modal
SettingsModalOther
@ciphera-net/uito ^0.0.80Test Plan
Greptile Summary
This is a large, well-structured PR that adds dimension filtering, automatic event tracking (404, scroll depth, outbound links, file downloads), a rebuilt chart, and a settings modal. The SWR layer and API functions are consistently updated to carry
filtersthrough every data-fetching path, and the newAddFilterDropdownandFilterBarcomponents handle deduplication and URL state cleanly.Key issues to address before merging:
lib/filters.ts) —|and,are used as raw separators without encoding, so any filter value containing those characters (referrer URLs, page paths with query strings, city names like "Portland, OR") will be silently truncated or split incorrectly duringparseFiltersFromURL. This affects a core user-facing feature.app/sites/[id]/page.tsxline 232) —prevStatsis fetched withoutfiltersParam, meaning the KPI percentage deltas compare filtered current data against unfiltered historical data, producing misleading numbers whenever a filter is active.EventPropertiesvalues fetch has no error handling or loading state — if the API call for property values fails, stale values from the previously selected key remain visible without any indication of failure.propsparameter could trivially carry the destinationurl, making these events actionable instead of just counts.'404'event name is unconventional — the validation regex allows it, but'page_not_found'would be more consistent with the documented naming convention and render more cleanly in the dashboard UI.Confidence Score: 3/5
|, city names with,), and the previous-period KPI comparison will show misleading deltas whenever a filter is active. Both issues are in the critical filtering feature that is the centerpiece of this PR.lib/filters.ts(delimiter encoding) andapp/sites/[id]/page.tsxline 232 (prevStatsmissing filters) need attention before this is safe to ship.Important Files Changed
prevStatsfetched without active filters causes misleading KPI period-over-period comparisons when filters are applied.'404'event name is unconventional.filtersinto both the cache key and fetcher — cache invalidation on filter change is handled properly.filtersparameter added to all private dashboard API functions; new event-property endpoints are well-typed and follow existing patterns.Flowchart
Last reviewed commit:
8ebd8baPrevious-period stats comparison ignores active filters
prevStatsis fetched without the activefiltersParam, so when a user filters the dashboard (e.g. "country is US"), the KPI cards compare filtered current-period data against unfiltered previous-period data. This produces misleading percentage deltas.useStatsalready accepts afiltersparameter — pass it here:Prompt To Fix With AI
@@ -0,0 +31,4 @@if (!selectedKey) returngetEventPropertyValues(siteId, eventName, selectedKey, dateRange.start, dateRange.end).then(setValues)}, [siteId, eventName, selectedKey, dateRange.start, dateRange.end])Missing loading state and error handling for property values fetch
When
selectedKeychanges (user taps a different property pill),valuesis updated asynchronously but there is no loading indicator and no.catch(). If the API request fails,valuessilently retains the data from the previous key selection, showing stale data without any indication of the error.Consider adding a
valuesLoadingflag and a.catch()similar to how the keys fetch is handled:Prompt To Fix With AI
@@ -0,0 +48,4 @@operator: operator as DimensionFilter['operator'],values: valuesRaw?.split(';') ?? [],}}).filter(f => f.dimension && f.operator && f.values.length > 0)Delimiter collision breaks filter parsing for URLs and city names
The serialization format uses
|as an intra-filter separator and,as an inter-filter separator, but neither character is escaped. This causesparseFiltersFromURLto silently drop data for any filter value containing these characters:https://t.co/abc|xyz) — afterpart.split('|'), destructuring assignsvaluesRaw = 'https://t.co/abc'and thexyzportion becomes an ignored 4th element.Portland, Oregonor/shop?a=1,b=2) —raw.split(',')splits the whole filter string first, soPortland, Oregonbecomes two separate, unparseable fragments.Consider URL-encoding the individual dimension/value parts before joining, or using a safer delimiter (e.g.
encodeURIComponenton values):Prompt To Fix With AI
@@ -349,0 +434,4 @@document.addEventListener('click', function(e) {var el = e.target;// * Walk up from clicked element to find nearest <a> tag'404'event name is an invalid identifier per the script's own validationtrackCustomEventvalidates event names withEVENT_NAME_REGEX = /^[a-zA-Z0-9_]+$/— digits are technically allowed, so'404'passes. However, a name that starts with a digit can be ambiguous and is inconsistent with the documented convention ("letters, numbers, underscores"). More importantly, when the event name is displayed in the dashboard it is converted with.replace(/_/g, ' '), yielding the display string"404"— which is usable, but a name like'page_not_found'would display more clearly as "page not found" and remain consistent with the naming convention shown in the docs embed (pulse.track('event_name')).Prompt To Fix With AI
Outbound link and file download events omit the destination URL
trackCustomEvent('outbound_link')andtrackCustomEvent('file_download')fire without anyprops, so the backend only knows that a click happened on the current page — not where the user went. The newpropsparameter added in this PR was specifically designed for this purpose.Consider passing the destination as a property:
This gives users actionable data ("which external links are clicked most") rather than a raw event count.
Prompt To Fix With AI