-
Notifications
You must be signed in to change notification settings - Fork 230
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
WSTEAM1-1514: Lite Site Piano Analytics - Spike #12332
base: latest
Are you sure you want to change the base?
Changes from all commits
df6a22b
22f29fd
87a5c0c
bb028be
19f6cf1
ed6b06b
a0b70d4
0d563ca
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,8 +1,12 @@ | ||
const sendBeaconLite = (atiPageViewUrlString: string) => ` | ||
var xhr = new XMLHttpRequest(); | ||
xhr.open("GET", "${atiPageViewUrlString}", true); | ||
xhr.withCredentials = true; | ||
xhr.send(); | ||
function sendBeaconLite (atiPageViewUrlString) { | ||
var xhr = new XMLHttpRequest(); | ||
xhr.open("GET", atiPageViewUrlString, true); | ||
xhr.withCredentials = true; | ||
xhr.send(); | ||
} | ||
|
||
sendBeaconLite("${atiPageViewUrlString}"); | ||
`; | ||
|
||
export default sendBeaconLite; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,7 +1,9 @@ | ||||||
/** @jsx jsx */ | ||||||
import React, { PropsWithChildren } from 'react'; | ||||||
import React, { PropsWithChildren, useContext } from 'react'; | ||||||
Check failure on line 2 in src/app/components/MostRead/Canonical/Item/index.tsx GitHub Actions / cypress-run (18.x)
|
||||||
import { jsx } from '@emotion/react'; | ||||||
import useClickTrackerHandler from '#hooks/useClickTrackerHandler'; | ||||||
import useClickTrackerHandler, { | ||||||
constructLiteSiteURL, | ||||||
} from '#hooks/useClickTrackerHandler'; | ||||||
import styles from './index.styles'; | ||||||
import { | ||||||
mostReadListGridProps, | ||||||
|
@@ -15,6 +17,7 @@ | |||||
} from '../../types'; | ||||||
import { Direction } from '../../../../models/types/global'; | ||||||
import Grid from '../../../../legacy/components/Grid'; | ||||||
import { RequestContext } from '#app/contexts/RequestContext'; | ||||||
Check failure on line 20 in src/app/components/MostRead/Canonical/Item/index.tsx GitHub Actions / cypress-run (18.x)
|
||||||
|
||||||
export const getParentColumns = (columnLayout: ColumnLayout) => { | ||||||
return columnLayout !== 'oneColumn' | ||||||
|
@@ -47,13 +50,15 @@ | |||||
eventTrackingData, | ||||||
}: PropsWithChildren<MostReadLinkProps>) => { | ||||||
const clickTrackerHandler = useClickTrackerHandler(eventTrackingData); | ||||||
const liteUrl = constructLiteSiteURL(eventTrackingData); | ||||||
|
||||||
return ( | ||||||
<div css={getItemCss({ dir, size })} dir={dir}> | ||||||
<a | ||||||
css={[styles.link, size === 'default' && styles.defaultLink]} | ||||||
href={href} | ||||||
onClick={clickTrackerHandler} | ||||||
{...(liteUrl && {'add-lite-tracker-params': liteUrl})} | ||||||
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. Should we make 'add-lite-tracker-params' into a const somewhere which we can reuse in other places?
Suggested change
|
||||||
> | ||||||
{title} | ||||||
</a> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,7 +114,7 @@ const MostRead = ({ | |
className = '', | ||
sendOptimizelyEvents = false, | ||
}: MostReadProps) => { | ||
const { isAmp, pageType, variant } = useContext(RequestContext); | ||
const { isLite, isAmp, pageType, variant } = useContext(RequestContext); | ||
const { optimizely } = useContext(OptimizelyContext); | ||
const { | ||
service, | ||
|
@@ -145,6 +145,9 @@ const MostRead = ({ | |
optimizely, | ||
optimizelyMetricNameOverride: 'most_read', | ||
}), | ||
...(isLite && { | ||
liteSiteUrl: 'liteSiteUrl', | ||
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 know this is just a spike, but what is this value supposed to be - the .lite version of the page it's on? |
||
}) | ||
}; | ||
|
||
return isAmp ? ( | ||
|
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.
A general comment - we mentioned that we wanted to add click tracking to the LiteSiteCTA button - could we carve it out into a separate PR (so that it could be merged sooner, and we can start getting metrics), or does it need to be included in this one?