Skip to content

Commit

Permalink
fix: a bunch of heatmaps bugs (#25743)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Oct 22, 2024
1 parent b210407 commit 68d2c10
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 12 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 8 additions & 9 deletions frontend/src/scenes/heatmaps/HeatmapsBrowser.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IconCollapse, IconGear } from '@posthog/icons'
import { LemonBanner, LemonButton, LemonInputSelect, LemonSkeleton, Spinner, Tooltip } from '@posthog/lemon-ui'
import clsx from 'clsx'
import { BindLogic, useActions, useValues } from 'kea'
import { AuthorizedUrlList } from 'lib/components/AuthorizedUrlList/AuthorizedUrlList'
import { appEditorUrl, AuthorizedUrlListType } from 'lib/components/AuthorizedUrlList/authorizedUrlListLogic'
Expand Down Expand Up @@ -152,7 +153,7 @@ function FilterPanel(): JSX.Element {
} = useActions(logic)

return (
<div className="flex flex-col gap-y-2 px-2 py-1 border-r w-100">
<div className={clsx('flex flex-col gap-y-2 px-2 py-1 border-r', !filterPanelCollapsed && 'w-100')}>
{filterPanelCollapsed ? (
<Tooltip title="Expand heatmap settings">
<LemonButton
Expand Down Expand Up @@ -209,14 +210,12 @@ function IframeErrorOverlay(): JSX.Element | null {
) : null
}

function LoadingOverlay(): JSX.Element | null {
const logic = heatmapsBrowserLogic()
const { loading } = useValues(logic)
return loading ? (
function LoadingOverlay(): JSX.Element {
return (
<div className="absolute flex flex-col w-full h-full items-center justify-center pointer-events-none">
<Spinner className="text-5xl" textColored={true} />
</div>
) : null
)
}

function EmbeddedHeatmapBrowser({
Expand All @@ -226,7 +225,7 @@ function EmbeddedHeatmapBrowser({
}): JSX.Element | null {
const logic = heatmapsBrowserLogic()

const { browserUrl } = useValues(logic)
const { browserUrl, loading, iframeBanner } = useValues(logic)
const { onIframeLoad, setIframeWidth } = useActions(logic)

const { width: iframeWidth } = useResizeObserver<HTMLIFrameElement>({ ref: iframeRef })
Expand All @@ -238,8 +237,8 @@ function EmbeddedHeatmapBrowser({
<div className="flex flex-row gap-x-2 w-full">
<FilterPanel />
<div className="relative flex-1 w-full h-full">
<IframeErrorOverlay />
<LoadingOverlay />
{loading ? <LoadingOverlay /> : null}
{!loading && iframeBanner ? <IframeErrorOverlay /> : null}
<iframe
ref={iframeRef}
className="w-full h-full bg-white"
Expand Down
6 changes: 5 additions & 1 deletion frontend/src/toolbar/elements/heatmapLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,11 @@ export const heatmapLogic = kea<heatmapLogicType>([

scrollDepthPosthogJsError: [
(s) => [s.posthog],
(posthog: PostHog): 'version' | 'disabled' | null => {
(posthog: PostHog | null): 'version' | 'disabled' | null => {
if (!posthog) {
return null
}

const posthogVersion =
posthog?.version ??
posthog?._calculate_event_properties('test', {}, new Date())?.['$lib_version'] ??
Expand Down
31 changes: 31 additions & 0 deletions frontend/src/toolbar/stats/currentPageLogic.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { withoutPostHogInit } from '~/toolbar/stats/currentPageLogic'

const posthogInitHashParam =
'__posthog={%22action%22:%20%22ph_authorize%22,%20%22token%22:%20%the-ph-token%22,%20%22temporaryToken%22:%20%the-posthog-token%22,%20%22actionId%22:%20null,%20%22userIntent%22:%20%22heatmaps%22,%20%22toolbarVersion%22:%20%22toolbar%22,%20%22apiURL%22:%20%22https://eu.posthog.com%22,%20%22dataAttributes%22:%20[%22data-attr%22],%20%22jsURL%22:%20%22https://app-static.eu.posthog.com%22,%20%22instrument%22:%20true,%20%22userEmail%22:%20%[email protected]%22,%20%22distinctId%22:%20%the-distinct-id%22}'

describe('current page logic', () => {
describe('cleaning href', () => {
it('can ignore posthog init hash param when other hash params present', () => {
// not technically a valid URL but :shrug:
expect(withoutPostHogInit(`https://wat.io?something=a#${posthogInitHashParam}#myfragment`)).toBe(
'https://wat.io/?something=a#myfragment'
)
})
it('can handle multiple curly braces in the init', () => {
// not technically a valid URL but :shrug:
expect(
withoutPostHogInit(
`https://wat.io?something=a#__posthog={something}and something}#myfragment={something}`
)
).toBe('https://wat.io/?something=a#myfragment={something}')
})
it('can ignore posthog init hash param when no other hash params present', () => {
expect(withoutPostHogInit(`https://wat.io?something=a#${posthogInitHashParam}`)).toBe(
'https://wat.io/?something=a'
)
})
it('gives nonsense back if it receives it', () => {
expect(withoutPostHogInit('i am not a url')).toBe('i am not a url')
})
})
})
23 changes: 21 additions & 2 deletions frontend/src/toolbar/stats/currentPageLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,22 @@ const replaceWithWildcard = (part: string): string => {
return part
}

/**
* Sometimes we are able to set the href before the posthog init fragment is removed
* we never want to store it as it will mean the heatmap URL is too specific and doesn't match
* this ensures we never store it
*/
export function withoutPostHogInit(href: string): string {
try {
const url = new URL(href)
url.hash = url.hash.replace(/__posthog=\{[^}]*}[^#]*/, '').replace(/^##/, '#')
url.hash = url.hash === '#' ? '' : url.hash
return url.toString()
} catch {
return href
}
}

export const currentPageLogic = kea<currentPageLogicType>([
path(['toolbar', 'stats', 'currentPageLogic']),
actions(() => ({
Expand All @@ -29,10 +45,13 @@ export const currentPageLogic = kea<currentPageLogicType>([
autoWildcardHref: true,
})),
reducers(() => ({
href: [window.location.href, { setHref: (_, { href }) => href }],
href: [window.location.href, { setHref: (_, { href }) => withoutPostHogInit(href) }],
wildcardHref: [
window.location.href,
{ setHref: (_, { href }) => href, setWildcardHref: (_, { href }) => href },
{
setHref: (_, { href }) => withoutPostHogInit(href),
setWildcardHref: (_, { href }) => withoutPostHogInit(href),
},
],
})),

Expand Down

0 comments on commit 68d2c10

Please sign in to comment.