Skip to content

Commit 10fa906

Browse files
authored
Revert "Personal Highlight Order Test (#14757)" (#14832)
This reverts commit 63c3564.
1 parent ccb082d commit 10fa906

File tree

8 files changed

+5
-605
lines changed

8 files changed

+5
-605
lines changed

dotcom-rendering/src/components/Card/components/CardLink.tsx

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,16 @@ type Props = {
2121
headlineText: string;
2222
dataLinkName?: string;
2323
isExternalLink: boolean;
24-
trackCardClick?: () => void;
2524
};
2625

2726
const InternalLink = ({
2827
linkTo,
2928
headlineText,
3029
dataLinkName,
31-
trackCardClick,
3230
}: {
3331
linkTo: string;
3432
headlineText: string;
3533
dataLinkName?: string;
36-
trackCardClick?: () => void;
3734
}) => {
3835
return (
3936
// eslint-disable-next-line jsx-a11y/anchor-has-content -- we have an aria-label attribute describing the content
@@ -42,7 +39,6 @@ const InternalLink = ({
4239
css={fauxLinkStyles}
4340
data-link-name={dataLinkName}
4441
aria-label={headlineText}
45-
onClick={trackCardClick}
4642
/>
4743
);
4844
};
@@ -51,12 +47,10 @@ const ExternalLink = ({
5147
linkTo,
5248
headlineText,
5349
dataLinkName,
54-
trackCardClick,
5550
}: {
5651
linkTo: string;
5752
headlineText: string;
5853
dataLinkName?: string;
59-
trackCardClick?: () => void;
6054
}) => {
6155
return (
6256
// eslint-disable-next-line jsx-a11y/anchor-has-content -- we have an aria-label attribute describing the content
@@ -67,7 +61,6 @@ const ExternalLink = ({
6761
aria-label={headlineText + ' (opens in new tab)'}
6862
target="_blank"
6963
rel="noreferrer"
70-
onClick={trackCardClick}
7164
/>
7265
);
7366
};
@@ -77,7 +70,6 @@ export const CardLink = ({
7770
headlineText,
7871
dataLinkName = 'article', //this makes sense if the link is to an article, but should this say something like "external" if it's an external link? are there any other uses/alternatives?
7972
isExternalLink,
80-
trackCardClick,
8173
}: Props) => {
8274
return (
8375
<>
@@ -86,15 +78,13 @@ export const CardLink = ({
8678
linkTo={linkTo}
8779
headlineText={headlineText}
8880
dataLinkName={dataLinkName}
89-
trackCardClick={trackCardClick}
9081
/>
9182
)}
9283
{!isExternalLink && (
9384
<InternalLink
9485
linkTo={linkTo}
9586
headlineText={headlineText}
9687
dataLinkName={dataLinkName}
97-
trackCardClick={trackCardClick}
9888
/>
9989
)}
10090
</>

dotcom-rendering/src/components/DecideContainer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ export const DecideContainer = ({
233233
return <NavList trails={trails} showImage={true} />;
234234
case 'scrollable/highlights':
235235
return (
236-
<Island priority="critical">
236+
<Island priority="feature" defer={{ until: 'visible' }}>
237237
<ScrollableHighlights trails={trails} frontId={frontId} />
238238
</Island>
239239
);

dotcom-rendering/src/components/Masthead/HighlightsCard.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ export type HighlightsCardProps = {
3232
byline?: string;
3333
isExternalLink: boolean;
3434
starRating?: Rating;
35-
trackCardClick: () => void;
3635
};
3736

3837
const container = css`
@@ -134,7 +133,6 @@ export const HighlightsCard = ({
134133
byline,
135134
isExternalLink,
136135
starRating,
137-
trackCardClick,
138136
}: HighlightsCardProps) => {
139137
const isMediaCard = isMedia(format);
140138

@@ -146,7 +144,6 @@ export const HighlightsCard = ({
146144
headlineText={headlineText}
147145
dataLinkName={dataLinkName}
148146
isExternalLink={isExternalLink}
149-
trackCardClick={trackCardClick}
150147
/>
151148

152149
<div css={content}>

dotcom-rendering/src/components/ScrollableHighlights.importable.tsx

Lines changed: 4 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,8 @@ import {
77
SvgChevronRightSingle,
88
} from '@guardian/source/react-components';
99
import { useEffect, useRef, useState } from 'react';
10-
import { submitComponentEvent } from '../client/ophan/ophan';
1110
import { getZIndex } from '../lib/getZIndex';
1211
import { ophanComponentId } from '../lib/ophan-helpers';
13-
import {
14-
getOrderedHighlights,
15-
onHighlightEvent,
16-
resetHighlightsState,
17-
} from '../lib/personaliseHighlights';
18-
import { useAB } from '../lib/useAB';
1912
import { palette } from '../palette';
2013
import type { DCRFrontCard } from '../types/front';
2114
import { HighlightsCard } from './Masthead/HighlightsCard';
@@ -48,7 +41,8 @@ const carouselStyles = css`
4841
grid-auto-flow: column;
4942
overflow-x: auto;
5043
overflow-y: hidden;
51-
scroll-behavior: auto;
44+
scroll-snap-type: x mandatory;
45+
scroll-behavior: smooth;
5246
overscroll-behavior-x: contain;
5347
overscroll-behavior-y: auto;
5448
@@ -84,10 +78,6 @@ const carouselStyles = css`
8478
position: relative;
8579
`;
8680

87-
const scrollSnapStyles = css`
88-
scroll-snap-type: x mandatory;
89-
`;
90-
9181
const itemStyles = css`
9282
scroll-snap-align: start;
9383
grid-area: span 1;
@@ -217,45 +207,12 @@ const getOphanInfo = (frontId?: string) => {
217207
};
218208
};
219209

220-
const isEqual = (
221-
historicalHighlights: DCRFrontCard[],
222-
currentHighlights: DCRFrontCard[],
223-
) => {
224-
const c = currentHighlights.map((v) => v.url);
225-
const h = historicalHighlights.map((v) => v.url);
226-
return c.every((v) => {
227-
return h.includes(v);
228-
});
229-
};
230210
export const ScrollableHighlights = ({ trails, frontId }: Props) => {
231211
const carouselRef = useRef<HTMLOListElement | null>(null);
232212
const carouselLength = trails.length;
233213
const imageLoading = 'eager';
234214
const [showPreviousButton, setShowPreviousButton] = useState(false);
235215
const [showNextButton, setShowNextButton] = useState(true);
236-
const [shouldShowHighlights, setShouldShowHighlights] = useState(false);
237-
const [orderedTrails, setOrderedTrails] = useState<DCRFrontCard[]>(trails);
238-
239-
const ABTestAPI = useAB()?.api;
240-
let abTestPersonalisedHighlightAttr = 'not-in-test';
241-
if (
242-
ABTestAPI?.isUserInVariant('PersonalisedHighlights', 'click-tracking')
243-
) {
244-
abTestPersonalisedHighlightAttr = 'click-tracking';
245-
}
246-
247-
if (ABTestAPI?.isUserInVariant('PersonalisedHighlights', 'view-tracking')) {
248-
abTestPersonalisedHighlightAttr = 'view-tracking';
249-
}
250-
251-
if (
252-
ABTestAPI?.isUserInVariant(
253-
'PersonalisedHighlights',
254-
'click-and-view-tracking',
255-
)
256-
) {
257-
abTestPersonalisedHighlightAttr = 'click-and-view-tracking';
258-
}
259216

260217
const scrollTo = (direction: 'left' | 'right') => {
261218
if (!carouselRef.current) return;
@@ -290,15 +247,6 @@ export const ScrollableHighlights = ({ trails, frontId }: Props) => {
290247
setShowNextButton(scrollLeft < maxScrollLeft);
291248
};
292249

293-
useEffect(() => {
294-
if (
295-
abTestPersonalisedHighlightAttr === 'view-tracking' ||
296-
abTestPersonalisedHighlightAttr === 'click-and-view-tracking'
297-
) {
298-
onHighlightEvent('VIEW');
299-
}
300-
}, [abTestPersonalisedHighlightAttr]);
301-
302250
useEffect(() => {
303251
const carouselElement = carouselRef.current;
304252
if (!carouselElement) return;
@@ -316,50 +264,11 @@ export const ScrollableHighlights = ({ trails, frontId }: Props) => {
316264
};
317265
}, []);
318266

319-
useEffect(() => {
320-
const personalisedHighlights = getOrderedHighlights();
321-
if (
322-
personalisedHighlights.length === 0 ||
323-
personalisedHighlights.length !== trails.length ||
324-
!isEqual(personalisedHighlights, trails)
325-
) {
326-
/* Reset to editorial order */
327-
resetHighlightsState(trails);
328-
setOrderedTrails(trails);
329-
return;
330-
}
331-
/* Otherwise, use personalised order from local storage */
332-
setOrderedTrails(personalisedHighlights);
333-
334-
/* Fire a view event only if the container has been personalised */
335-
void submitComponentEvent(
336-
{
337-
component: {
338-
componentType: 'CONTAINER',
339-
id: `reordered-highlights-container`,
340-
},
341-
action: 'VIEW',
342-
},
343-
'Web',
344-
);
345-
}, [trails]);
346-
347-
/* Update the visibility of highlights once we have the correct trail order set in state. */
348-
useEffect(() => {
349-
setShouldShowHighlights(true);
350-
}, [orderedTrails]);
351-
352267
const { ophanComponentLink, ophanComponentName, ophanFrontName } =
353268
getOphanInfo(frontId);
354269

355270
return (
356-
<div
357-
css={[
358-
containerStyles,
359-
{ visibility: shouldShowHighlights ? 'visible' : 'hidden' },
360-
]}
361-
data-link-name={ophanFrontName}
362-
>
271+
<div css={containerStyles} data-link-name={ophanFrontName}>
363272
<ol
364273
data-link-name={ophanComponentLink}
365274
data-component={ophanComponentName}
@@ -368,15 +277,10 @@ export const ScrollableHighlights = ({ trails, frontId }: Props) => {
368277
css={[
369278
carouselStyles,
370279
generateCarouselColumnStyles(carouselLength),
371-
/*
372-
* Enable scroll-snap only when visible to prevent browser scroll anchoring.
373-
* When items reorder, browsers try to keep previously visible items in view,
374-
* causing unwanted scrolling. Enabling snap after reordering forces position 0.
375-
*/ shouldShowHighlights && scrollSnapStyles,
376280
]}
377281
data-heatphan-type="carousel"
378282
>
379-
{orderedTrails.map((trail) => {
283+
{trails.map((trail) => {
380284
return (
381285
<li
382286
key={trail.url}
@@ -396,16 +300,6 @@ export const ScrollableHighlights = ({ trails, frontId }: Props) => {
396300
showQuotedHeadline={trail.showQuotedHeadline}
397301
mainMedia={trail.mainMedia}
398302
starRating={trail.starRating}
399-
trackCardClick={() => {
400-
if (
401-
abTestPersonalisedHighlightAttr ===
402-
'click-tracking' ||
403-
abTestPersonalisedHighlightAttr ===
404-
'click-and-view-tracking'
405-
) {
406-
onHighlightEvent('CLICK', trail);
407-
}
408-
}}
409303
/>
410304
</li>
411305
);

dotcom-rendering/src/experiments/ab-tests.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import type { ABTest } from '@guardian/ab-core';
22
import { abTestTest } from './tests/ab-test-test';
33
import { auxiaSignInGate } from './tests/auxia-sign-in-gate';
44
import { noAuxiaSignInGate } from './tests/no-auxia-sign-in-gate';
5-
import { personalisedHighlights } from './tests/personalised-highlights';
65
import { signInGateMainControl } from './tests/sign-in-gate-main-control';
76
import { signInGateMainVariant } from './tests/sign-in-gate-main-variant';
87
import { userBenefitsApi } from './tests/user-benefits-api';
@@ -15,6 +14,5 @@ export const tests: ABTest[] = [
1514
signInGateMainControl,
1615
userBenefitsApi,
1716
auxiaSignInGate,
18-
personalisedHighlights,
1917
noAuxiaSignInGate,
2018
];

dotcom-rendering/src/experiments/tests/personalised-highlights.ts

Lines changed: 0 additions & 43 deletions
This file was deleted.

0 commit comments

Comments
 (0)