-
Notifications
You must be signed in to change notification settings - Fork 13
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
Initial GTM Migrations #456
base: main
Are you sure you want to change the base?
Changes from all commits
4599bec
0711d31
5353bab
ccf405b
8f273a4
dfb0aae
1cc0cd4
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 |
---|---|---|
@@ -1,9 +1,9 @@ | ||
import { useEffect } from "react"; | ||
import { useEffect, useState } from "react"; | ||
import { useRouter } from "next/router"; | ||
import Script from "next/script"; | ||
import type { AppProps } from "next/app"; | ||
import { DocsContextProvider } from "layouts/DocsPage/context"; | ||
import { posthog, sendPageview } from "utils/posthog"; | ||
import { posthog, sendEngagedView, sendPageview } from "utils/posthog"; | ||
import { TabContextProvider } from "components/Tabs"; | ||
|
||
// https://larsmagnus.co/blog/how-to-optimize-custom-fonts-with-next-font | ||
|
@@ -68,7 +68,32 @@ interface dataLayerItem { | |
declare global { | ||
var dataLayer: dataLayerItem[]; // eslint-disable-line no-var | ||
} | ||
const useIsEngaged = () => { | ||
const router = useRouter(); | ||
const [timerReached, setTimerReached] = useState(false); | ||
const [secondPageReached, setSecondPageReached] = useState(false); | ||
const [isEngaged, setIsEngaged] = useState(false); | ||
|
||
useEffect(() => { | ||
setTimeout(() => { | ||
setTimerReached(true); | ||
}, 30000); | ||
const routeChanged = () => { | ||
setSecondPageReached(true); | ||
}; | ||
|
||
router.events.on("routeChangeComplete", routeChanged); | ||
return () => { | ||
router.events.off("routeChangeComplete", routeChanged); | ||
}; | ||
}, [router.events]); | ||
|
||
useEffect(() => { | ||
setIsEngaged(secondPageReached && timerReached); | ||
}, [secondPageReached, timerReached]); | ||
|
||
return isEngaged; | ||
}; | ||
const Analytics = () => { | ||
return ( | ||
<> | ||
|
@@ -163,6 +188,18 @@ const Analytics = () => { | |
{/* End Google Tag Manager (noscript) */} | ||
</> | ||
)} | ||
{/* Quailified Script */} | ||
<Script id="script_qualified"> | ||
{`(function (w, q) { | ||
w["QualifiedObject"] = q; | ||
w[q] = | ||
w[q] || | ||
function () { | ||
(w[q].q = w[q].q || []).push(arguments); | ||
}; | ||
})(window, "qualified")`} | ||
</Script> | ||
<Script src="https://js.qualified.com/qualified.js?token=GWPbwWJLtjykim4W" /> | ||
|
||
{NEXT_PUBLIC_REDDIT_ID && ( | ||
<> | ||
|
@@ -180,14 +217,29 @@ const Analytics = () => { | |
|
||
const MyApp = ({ Component, pageProps }: AppProps) => { | ||
const router = useRouter(); | ||
const isEngaged = useIsEngaged(); | ||
|
||
useEffect(() => { | ||
posthog(); // init posthog | ||
if (!isEngaged) return; | ||
// Trigger engagement view events here | ||
sendEngagedView(); | ||
}, [isEngaged]); | ||
|
||
Comment on lines
+223
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have to return an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this mainly comes down to which solution you perceive as being more readable. I think the current solution keeps the hook readable by isolating the tracking logic from the tracking events. |
||
router.events.on("routeChangeComplete", sendPageview); | ||
const Pageviews = () => { | ||
// Trigger page views here | ||
|
||
// Qualified page view | ||
if (!!window["qualified"]) window["qualified"]("page"); | ||
// Posthog page view | ||
sendPageview(); | ||
}; | ||
useEffect(() => { | ||
posthog(); // init posthog | ||
// Trigger initial load page views | ||
Pageviews(); | ||
router.events.on("routeChangeComplete", Pageviews); | ||
return () => { | ||
router.events.off("routeChangeComplete", sendPageview); | ||
router.events.off("routeChangeComplete", Pageviews); | ||
}; | ||
}, [router.events]); | ||
|
||
|
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.
does this have to be two effects? From what I can tell from the logic, we are checking
Since the timer isn't being reset anywhere, we can just put
setIsEngaged
into the timeout instead right? this removes extra state and an extra useEffect. something like this pseudo code below. what do you think?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.
Doesn't quite match the criteria for tracking the engagement metrics.
With the current logic the timeout continues through page navigation and will trigger the tracking events when
With this proposed implementation the timeout resets on page navigation and the user would have to spend the full 30 seconds on the second (or any subsequent) page visited to be marked as an engaged viewer. This would cause a rapidly navigating user to not be marked as an engaged viewer.