From 98604b4085fa53400b0aa225e42b57dfdc7fe63b Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 10:19:17 +0100 Subject: [PATCH 1/8] Refactor EmailSignUpWrapper to remove AB test dependencies and simplify rendering logic --- .../EmailSignUpWrapper.island.test.tsx | 189 ++---------------- .../components/EmailSignUpWrapper.island.tsx | 143 +++---------- .../components/EmailSignUpWrapper.stories.tsx | 121 +---------- dotcom-rendering/src/lib/renderElement.tsx | 3 - 4 files changed, 55 insertions(+), 401 deletions(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx index 834d1da677d..04d4ea9a2af 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx @@ -1,8 +1,7 @@ import '@testing-library/jest-dom'; import { render, screen } from '@testing-library/react'; import { submitComponentEvent } from '../client/ophan/ophan'; -import { AB_TEST_NAME } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; +import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { ConfigProvider } from './ConfigContext'; @@ -12,10 +11,6 @@ jest.mock('../client/ophan/ophan', () => ({ submitComponentEvent: jest.fn(() => Promise.resolve()), })); -jest.mock('../lib/useAB', () => ({ - useAB: jest.fn(), -})); - jest.mock('../lib/useAuthStatus', () => ({ useIsSignedIn: jest.fn(), })); @@ -35,10 +30,6 @@ jest.mock('./NewsletterSignupForm.island', () => ({ ), })); -jest.mock('./SecureSignup.island', () => ({ - SecureSignup: () =>
SecureSignup
, -})); - jest.mock('./NewsletterSignupCardContainer', () => ({ NewsletterSignupCardContainer: ({ children, @@ -51,12 +42,6 @@ jest.mock('./NewsletterSignupCardContainer', () => ({ ), })); -jest.mock('./EmailSignup', () => ({ - EmailSignup: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), -})); - const defaultProps = { index: 0, listId: 4147, @@ -64,7 +49,6 @@ const defaultProps = { name: 'The Recap', description: 'A weekly sports roundup', frequency: 'Weekly', - successDescription: "We'll send you The Recap every week", theme: 'sport', idApiUrl: 'https://idapi.theguardian.com', }; @@ -83,181 +67,62 @@ const renderWrapper = (props = {}, renderingTarget: 'Web' | 'Apps' = 'Web') => , ); -const mockAbTests = (variant: 'control' | 'variantIllustratedCard') => { - (useAB as jest.Mock).mockReturnValue({ - getParticipations: () => ({ - [AB_TEST_NAME]: variant, - }), - }); -}; - describe('EmailSignUpWrapper', () => { beforeEach(() => { jest.resetAllMocks(); - // Default: AB API not yet hydrated - (useAB as jest.Mock).mockReturnValue(undefined); (useIsSignedIn as jest.Mock).mockReturnValue(false); (useNewsletterSubscription as jest.Mock).mockReturnValue(false); }); - describe('flag off (showNewNewsletterSignupCard = false)', () => { + describe('rendering', () => { it('shows a placeholder while subscription status is loading', () => { (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); renderWrapper(); - // The Placeholder component renders a div with specific heights - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); expect( screen.queryByTestId('newsletter-signup-card-container'), ).not.toBeInTheDocument(); }); - it('renders nothing when the user is already subscribed', () => { - mockAbTests('control'); - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - const { container } = renderWrapper({ - hideNewsletterSignupComponentForSubscribers: true, - }); - expect(container).toBeEmptyDOMElement(); - }); - - it('renders the legacy EmailSignup when the user is not subscribed', () => { - mockAbTests('control'); + it('renders the NewsletterSignupCardContainer when the user is not subscribed', () => { renderWrapper(); - expect(screen.getByTestId('email-signup')).toBeInTheDocument(); - expect(screen.getByTestId('secure-signup')).toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - }); - - describe('flag on (showNewNewsletterSignupCard = true)', () => { - it('shows a placeholder when AB API has not hydrated yet', () => { - // useAB returns undefined before hydration — component shows - // Placeholder rather than committing to control or variant early. - // (default set in outer beforeEach, no override needed) - renderWrapper({ showNewNewsletterSignupCard: true }); - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('renders NewsletterSignupCardContainer for all web users regardless of AB participation', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); - expect( - screen.getByTestId('newsletter-signup-card-container'), - ).toBeInTheDocument(); - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); - }); - - it('renders NewsletterSignupCardContainer for users in the variant group', () => { - mockAbTests('variantIllustratedCard'); - renderWrapper({ showNewNewsletterSignupCard: true }); expect( screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); expect( screen.getByTestId('newsletter-signup-form'), ).toBeInTheDocument(); - expect( - screen.queryByTestId('email-signup'), - ).not.toBeInTheDocument(); }); - it('does not hide the variant for already-subscribed users (subscription handled inside the card)', () => { - mockAbTests('variantIllustratedCard'); + it('still renders the card for already-subscribed users (subscription handled inside the card)', () => { (useNewsletterSubscription as jest.Mock).mockReturnValue(true); renderWrapper({ - showNewNewsletterSignupCard: true, hideNewsletterSignupComponentForSubscribers: true, }); expect( screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); }); - }); - describe('Apps rendering target', () => { - it('renders the legacy EmailSignup without waiting for AB to resolve', () => { - // useAB remains undefined (AB framework never initialises on Apps) - // but the component should not block on it - renderWrapper({ showNewNewsletterSignupCard: true }, 'Apps'); - expect(screen.getByTestId('email-signup')).toBeInTheDocument(); + it('renders the NewsletterSignupCardContainer on Apps as well as Web', () => { + renderWrapper({}, 'Apps'); expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('fires a VIEW tracking event without waiting for AB to resolve', () => { - renderWrapper({ showNewNewsletterSignupCard: true }, 'Apps'); - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - action: 'VIEW', - abTest: { name: AB_TEST_NAME, variant: 'control' }, - }), - 'Apps', - ); + screen.getByTestId('newsletter-signup-card-container'), + ).toBeInTheDocument(); }); }); describe('VIEW tracking', () => { - it('fires a VIEW event with the variantIllustratedCard component id for all web users', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - component: { - componentType: 'NEWSLETTER_SUBSCRIPTION', - id: 'AR NewsletterSignupForm the-recap - variantIllustratedCard', - }, - action: 'VIEW', - abTest: { - name: AB_TEST_NAME, - variant: 'variantIllustratedCard', - }, - }), - 'Web', - ); - }); - - it('fires a VIEW event with the correct variant component id and ab metadata', () => { - mockAbTests('variantIllustratedCard'); - renderWrapper({ showNewNewsletterSignupCard: true }); + it('fires a VIEW event with the illustrated card component id', () => { + renderWrapper(); expect(submitComponentEvent).toHaveBeenCalledWith( expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: 'AR NewsletterSignupForm the-recap - variantIllustratedCard', - }, - action: 'VIEW', - abTest: { - name: AB_TEST_NAME, - variant: 'variantIllustratedCard', + id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + 'the-recap', + ), }, - }), - 'Web', - ); - }); - - it('fires a VIEW event with the control component id when the flag is off', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: false }); - - expect(submitComponentEvent).toHaveBeenCalledWith( - expect.objectContaining({ - component: expect.objectContaining({ - id: 'AR SecureSignup the-recap', - }), action: 'VIEW', }), 'Web', @@ -265,43 +130,21 @@ describe('EmailSignUpWrapper', () => { }); it('fires the VIEW event exactly once on mount', () => { - mockAbTests('control'); - renderWrapper({ showNewNewsletterSignupCard: true }); + renderWrapper(); expect(submitComponentEvent).toHaveBeenCalledTimes(1); }); it('does not fire a VIEW event while subscription status is loading', () => { - mockAbTests('control'); (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); - - it('does not fire a VIEW event while the AB client has not hydrated', () => { - // useAB returns undefined — default set in beforeEach - renderWrapper({ showNewNewsletterSignupCard: true }); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); - - it('does not fire a VIEW event for control users who are already subscribed', () => { - mockAbTests('control'); - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - showNewNewsletterSignupCard: true, - hideNewsletterSignupComponentForSubscribers: true, - }); + renderWrapper(); expect(submitComponentEvent).not.toHaveBeenCalled(); }); - it('does not fire a VIEW event for variant users who are already subscribed', () => { - mockAbTests('variantIllustratedCard'); + it('does not fire a VIEW event when the user is already subscribed', () => { (useNewsletterSubscription as jest.Mock).mockReturnValue(true); renderWrapper({ - showNewNewsletterSignupCard: true, hideNewsletterSignupComponentForSubscribers: true, }); diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx index c4fa4568b32..b41fd50fb9e 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx @@ -1,23 +1,21 @@ import type { Breakpoint } from '@guardian/source/foundations'; import { useEffect, useRef } from 'react'; import { - AB_TEST_NAME, NEWSLETTER_SIGNUP_COMPONENT_ID, sendNewsletterSignupEvent, } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { useConfig } from './ConfigContext'; import type { EmailSignUpProps } from './EmailSignup'; -import { EmailSignup } from './EmailSignup'; import { InlineSkipToWrapper } from './InlineSkipToWrapper'; import { Island } from './Island'; -import { NewsletterPrivacyMessage } from './NewsletterPrivacyMessage'; import { NewsletterSignupCardContainer } from './NewsletterSignupCardContainer'; import { NewsletterSignupForm } from './NewsletterSignupForm.island'; import { Placeholder } from './Placeholder'; -import { SecureSignup } from './SecureSignup.island'; +// When the next A/B experiment is added (e.g. preview-button test), import +// useAB and AB_TEST_NAME from their respective modules and thread them through +// sendNewsletterSignupEvent's `abTest` param and NewsletterSignupForm's `abTest` prop. /** * Approximate heights of the EmailSignup component at different breakpoints. @@ -33,15 +31,12 @@ interface EmailSignUpWrapperProps extends EmailSignUpProps { listId: number; identityName: string; category?: string; - successDescription: string; - /** Illustration image URL (square crop) for the NewsletterSignupCard variant */ + /** Illustration image URL (square crop) for the NewsletterSignupCard */ illustrationSquare?: string; idApiUrl: string; exampleUrl?: string; /** Feature flag to enable hiding newsletter signup for already subscribed users */ hideNewsletterSignupComponentForSubscribers?: boolean; - /** Feature flag to show the new NewsletterSignupCard design instead of EmailSignup */ - showNewNewsletterSignupCard?: boolean; } /** @@ -66,22 +61,10 @@ export const EmailSignUpWrapper = ({ illustrationSquare, frequency, theme, - successDescription, hideNewsletterSignupComponentForSubscribers = false, - showNewNewsletterSignupCard = false, }: EmailSignUpWrapperProps) => { const { renderingTarget } = useConfig(); - const abTestEnabled = - showNewNewsletterSignupCard && renderingTarget === 'Web'; - - const abTests = useAB(); - const abResolved = abTests !== undefined; - - const getVariantName = () => 'variantIllustratedCard'; - - const abVariant = abTestEnabled ? getVariantName() : 'control'; - const isSubscribed = useNewsletterSubscription( listId, idApiUrl, @@ -92,16 +75,12 @@ export const EmailSignUpWrapper = ({ const viewFiredRef = useRef(false); useEffect(() => { - if (abTestEnabled && !abResolved) { - return; - } - // Wait for subscription status in both branches — we only want to track - // a view of the actual signup form, not a loading state or success message. + // Wait for subscription status — we only want to track a view of the + // actual signup form, not a loading state or success message. if (isSubscribed === undefined) { return; } - // Don't fire if the user is already subscribed: in both branches they - // will see a success/already-subscribed message, not the signup form. + // Don't fire if the user is already subscribed. if (isSubscribed) { return; } @@ -114,109 +93,49 @@ export const EmailSignUpWrapper = ({ action: 'VIEW', identityName, componentId: - abVariant === 'variantIllustratedCard' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - identityName, - ) - : abVariant === 'variantNewField' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField( - identityName, - ) - : NEWSLETTER_SIGNUP_COMPONENT_ID.control(identityName), + NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + identityName, + ), renderingTarget, value: { eventDescription: 'newsletter-signup-viewed', }, - // Use the standard Ophan abTest field so Ophan can join events - // to the A/B test — not strings encoded inside value. - abTest: { name: AB_TEST_NAME, variant: abVariant }, }); - }, [ - abResolved, - abVariant, - identityName, - isSubscribed, - abTestEnabled, - renderingTarget, - ]); + }, [identityName, isSubscribed, renderingTarget]); - if ((abTestEnabled && !abResolved) || isSubscribed === undefined) { + if (isSubscribed === undefined) { return ; } - if (abVariant === 'variantIllustratedCard') { - return ( - - - {(previewAction) => ( - - - - )} - - - ); - } - - // Don't render control if user is already subscribed - if (isSubscribed) { - return null; - } - - const emailInputName = - abVariant === 'variantNewField' ? 'emailAddress' : 'email'; - const emailInputId = - abVariant === 'variantNewField' - ? `emailInput-${identityName}` - : undefined; - return ( - - - - - - + {(previewAction) => ( + + + + )} + ); }; diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index b3c19fe1ce6..e79d6ce74b0 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -1,31 +1,11 @@ import type { StoryObj } from '@storybook/react-webpack5'; -import { mocked, within } from 'storybook/test'; +import { mocked } from 'storybook/test'; import preview from '../../.storybook/preview'; import { lazyFetchEmailWithTimeout } from '../lib/fetchEmail'; -import { AB_TEST_NAME } from '../lib/newsletterSignupTracking'; -import { useAB } from '../lib/useAB'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; -/** Resolves `useAB` as if the AB framework has hydrated, placing the user in control or variant. */ -const mockAB = ( - variant: 'control' | 'variantNewField' | 'variantIllustratedCard', -) => { - mocked(useAB).mockReturnValue({ - isUserInTestGroup: (_testName: string, group: string) => - group === variant, - isUserInTest: () => true, - getParticipations: () => - (variant !== 'control' - ? { - [AB_TEST_NAME]: variant, - } - : {}) as Record, - trackABTests: () => ({}), - }); -}; - const meta = preview.meta({ title: 'Components/EmailSignUpWrapper', component: EmailSignUpWrapper, @@ -41,89 +21,24 @@ const defaultArgs = { "The best of our sports journalism from the past seven days and a heads-up on the weekend's action", name: 'The Recap', frequency: 'Weekly', - successDescription: "We'll send you The Recap every week", theme: 'sport', idApiUrl: 'https://idapi.theguardian.com', exampleUrl: 'https://www.theguardian.com/email/the-recap', -} satisfies Story['args']; - -const newCardArgs = { - ...defaultArgs, - showNewNewsletterSignupCard: true, hideNewsletterSignupComponentForSubscribers: true, illustrationSquare: 'https://i.guim.co.uk/img/uploads/2023/11/01/SaturdayEdition_-_5-3.jpg?width=220&dpr=2&s=none&crop=5%3A3', } satisfies Story['args']; -export const Placeholder = meta.story({ +export const LoadingPlaceholder = meta.story({ args: { ...defaultArgs }, beforeEach() { - mockAB('control'); mocked(useNewsletterSubscription).mockReturnValue(undefined); }, }); -export const DefaultStory = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(false); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve(null), - ); - }, -}); - export const SignedInNotSubscribed = meta.story({ args: { ...defaultArgs }, beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(true); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve('test@example.com'), - ); - }, -}); - -export const SignedInAlreadySubscribed = meta.story({ - args: { - ...defaultArgs, - hideNewsletterSignupComponentForSubscribers: true, - }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(true); - }, -}); - -export const FeatureFlagDisabled = meta.story({ - args: { - ...defaultArgs, - hideNewsletterSignupComponentForSubscribers: false, - }, - beforeEach() { - mockAB('control'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(true); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve('test@example.com'), - ); - }, - parameters: { - docs: { - description: { - story: 'When the hideNewsletterSignupComponentForSubscribers feature flag is disabled, the signup form is always shown regardless of subscription status.', - }, - }, - }, -}); - -export const NewsletterSignupCardSignedInNotSubscribed = meta.story({ - args: newCardArgs, - beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -132,10 +47,9 @@ export const NewsletterSignupCardSignedInNotSubscribed = meta.story({ }, }); -export const NewsletterSignupCardSignedOutNotSubscribed = meta.story({ - args: newCardArgs, +export const SignedOutNotSubscribed = meta.story({ + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -144,10 +58,9 @@ export const NewsletterSignupCardSignedOutNotSubscribed = meta.story({ }, }); -export const NewsletterSignupCardSignedInAlreadySubscribed = meta.story({ - args: newCardArgs, +export const SignedInAlreadySubscribed = meta.story({ + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -156,10 +69,9 @@ export const NewsletterSignupCardSignedInAlreadySubscribed = meta.story({ }, }); -export const NewsletterSignupCardSignedOutAlreadySubscribed = meta.story({ - args: newCardArgs, +export const SignedOutAlreadySubscribed = meta.story({ + args: { ...defaultArgs }, beforeEach() { - mockAB('variantIllustratedCard'); mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => @@ -167,20 +79,3 @@ export const NewsletterSignupCardSignedOutAlreadySubscribed = meta.story({ ); }, }); - -export const NewsletterSignupCardFocused = meta.story({ - args: newCardArgs, - beforeEach() { - mockAB('variantIllustratedCard'); - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(false); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve(null), - ); - }, - async play({ canvasElement }) { - const canvas = within(canvasElement); - const emailInput = await canvas.findByLabelText('Enter your email'); - emailInput.focus(); - }, -}); diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index 0a1593d173c..f4bc1500347 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -595,15 +595,12 @@ export const renderElement = ({ description: newsletter.description, name: newsletter.name, frequency: newsletter.frequency, - successDescription: newsletter.successDescription, theme: newsletter.theme, illustrationSquare: newsletter.illustrationSquare, exampleUrl: newsletter.exampleUrl, idApiUrl: idApiUrl ?? '', hideNewsletterSignupComponentForSubscribers: !!switches.hideNewsletterSignupComponentForSubscribers, - showNewNewsletterSignupCard: - !!switches.showNewNewsletterSignupCard, }; if (isListElement || isTimeline) { return null; From eb6939cd362acc79f9bd64f85df2a557b5ec0352 Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 11:39:49 +0100 Subject: [PATCH 2/8] Refactor newsletter signup component IDs --- .../EmailSignUpWrapper.island.test.tsx | 2 +- .../components/EmailSignUpWrapper.island.tsx | 11 ++-- .../HighlightsNewsletterSignupModal.tsx | 4 ++ .../NewsletterSignupCardContainer.test.tsx | 6 +-- .../NewsletterSignupCardContainer.tsx | 2 +- .../NewsletterSignupForm.island.stories.tsx | 1 + .../NewsletterSignupForm.island.test.tsx | 1 + .../NewsletterSignupForm.island.tsx | 5 ++ .../components/SecureSignup.island.test.tsx | 18 ++----- .../src/components/SecureSignup.island.tsx | 7 +-- .../src/lib/newsletterSignupTracking.test.ts | 44 ++++++++-------- .../src/lib/newsletterSignupTracking.ts | 24 +++------ .../src/lib/useNewsletterSignupForm.ts | 50 +++++++++---------- 13 files changed, 82 insertions(+), 93 deletions(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx index 04d4ea9a2af..b86657799bb 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx @@ -119,7 +119,7 @@ describe('EmailSignUpWrapper', () => { expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( 'the-recap', ), }, diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx index b41fd50fb9e..003ad80f3fe 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx @@ -72,6 +72,9 @@ export const EmailSignUpWrapper = ({ ); const isSignedIn = useIsSignedIn(); + const componentId = + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(identityName); + const viewFiredRef = useRef(false); useEffect(() => { @@ -92,16 +95,13 @@ export const EmailSignUpWrapper = ({ sendNewsletterSignupEvent({ action: 'VIEW', identityName, - componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - identityName, - ), + componentId, renderingTarget, value: { eventDescription: 'newsletter-signup-viewed', }, }); - }, [identityName, isSubscribed, renderingTarget]); + }, [componentId, identityName, isSubscribed, renderingTarget]); if (isSubscribed === undefined) { return ; @@ -132,6 +132,7 @@ export const EmailSignUpWrapper = ({ frequency={frequency} previewAction={previewAction} isAlreadySubscribed={isSubscribed} + componentId={componentId} /> )} diff --git a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx index b13592b4cf9..8c5e80fba1b 100644 --- a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx +++ b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx @@ -12,6 +12,7 @@ import { Pillar, } from '../../../lib/articleFormat'; import { generateImageURL } from '../../../lib/image'; +import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../../../lib/newsletterSignupTracking'; import { useNewsletterSubscription } from '../../../lib/useNewsletterSubscription'; import type { Newsletter } from '../../../types/content'; import { FormatBoundary } from '../../FormatBoundary'; @@ -143,6 +144,9 @@ const HighlightsNewsletterSignupModalContent = ({ frequency={newsletter.frequency} isModal={true} isAlreadySubscribed={isSubscribed === true} + componentId={NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard( + newsletter.identityName, + )} abTest={{ name: 'highlights-newsletter-card', variant: 'highlightsCard', diff --git a/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx b/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx index f8486e4aab4..d98347f41f9 100644 --- a/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupCardContainer.test.tsx @@ -73,7 +73,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }, @@ -102,7 +102,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }, @@ -176,7 +176,7 @@ describe('NewsletterSignupCardContainer', () => { expect.objectContaining({ action: 'EXPAND', component: expect.objectContaining({ - id: NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + id: NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( defaultProps.identityName, ), }), diff --git a/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx b/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx index 9cb2272b6ca..1f374f7c93c 100644 --- a/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupCardContainer.tsx @@ -31,7 +31,7 @@ const sendPreviewTracking = ({ action: eventDescription === 'preview-open' ? 'EXPAND' : 'CLOSE', identityName, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard(identityName), + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(identityName), renderingTarget, value: { eventDescription, renderUrl, isSignedIn }, }); diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx index 8822129fdc8..392ed120914 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.stories.tsx @@ -50,6 +50,7 @@ const defaultArgs = { newsletterName: 'Saturday Edition', frequency: 'every Saturday', previewAction: { behaviour: 'modal' as const, onClick: fn() }, + componentId: 'AR NewsletterSignupForm saturday-edition', }; /** Shared no-op handlers — stories that focus on visual state don't need real callbacks. */ diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx index e4ed1862332..a517597d21e 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.test.tsx @@ -89,6 +89,7 @@ const renderForm = () => newsletterId="morning-briefing" newsletterName="Morning Briefing" frequency="every day" + componentId="AR NewsletterSignupForm morning-briefing" /> , ); diff --git a/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx b/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx index d5518ee406d..b39b046ab90 100644 --- a/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx +++ b/dotcom-rendering/src/components/NewsletterSignupForm.island.tsx @@ -35,6 +35,9 @@ type Props = { previewAction?: NewsletterPreviewAction; /** When `true`, the success message is shown immediately (user is already subscribed). */ isAlreadySubscribed?: boolean; + /** Ophan component ID for tracking events. Derived by the caller from the active + * component/experiment so that this form is not coupled to test state. */ + componentId: string; /** Ophan A/B test metadata — forwarded to tracking events. */ abTest?: AbTest; /** @@ -290,6 +293,7 @@ const NewsletterSignupFormActive = ({ newsletterName, frequency, previewAction, + componentId, abTest, isModal = false, }: Omit) => { @@ -320,6 +324,7 @@ const NewsletterSignupFormActive = ({ } = useNewsletterSignupForm( newsletterId, renderingTarget, + componentId, abTest, hideMarketingToggle, ); diff --git a/dotcom-rendering/src/components/SecureSignup.island.test.tsx b/dotcom-rendering/src/components/SecureSignup.island.test.tsx index acac253ab99..60452076105 100644 --- a/dotcom-rendering/src/components/SecureSignup.island.test.tsx +++ b/dotcom-rendering/src/components/SecureSignup.island.test.tsx @@ -323,24 +323,12 @@ describe('SecureSignup tracking component id', () => { global.fetch = jest.fn().mockResolvedValue({ ok: true } as Response); }); - it('uses the control component id when no abTest variant is provided', async () => { - const testUser = user.setup(); - renderSecureSignup(); - await submitSignupForm(testUser); - - await waitFor(() => { - expect( - getTrackingCallForEvent('form-submission')?.componentId, - ).toBe('AR SecureSignup morning-briefing'); - }); - }); - - it('uses the variantNewField component id when variant is variantNewField', async () => { + it('always uses the secureSignup component id regardless of abTest variant', async () => { const testUser = user.setup(); renderSecureSignup({ abTest: { name: 'test-ab', - variant: 'variantNewField', + variant: 'someVariant', }, }); await submitSignupForm(testUser); @@ -348,7 +336,7 @@ describe('SecureSignup tracking component id', () => { await waitFor(() => { expect( getTrackingCallForEvent('form-submission')?.componentId, - ).toBe('AR SecureSignup morning-briefing - variantNewField'); + ).toBe('AR SecureSignup morning-briefing'); }); }); }); diff --git a/dotcom-rendering/src/components/SecureSignup.island.tsx b/dotcom-rendering/src/components/SecureSignup.island.tsx index ad2db25b31b..a4844fb8b9e 100644 --- a/dotcom-rendering/src/components/SecureSignup.island.tsx +++ b/dotcom-rendering/src/components/SecureSignup.island.tsx @@ -254,15 +254,10 @@ const sendTracking = ( abTest?: AbTest, extraDetails?: Record, ): void => { - const componentId = - abTest?.variant === 'variantNewField' - ? NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField(newsletterId) - : NEWSLETTER_SIGNUP_COMPONENT_ID.control(newsletterId); - sendNewsletterSignupEvent({ action: EVENT_DESCRIPTION_TO_ACTION[eventDescription], identityName: newsletterId, - componentId, + componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.secureSignup(newsletterId), renderingTarget, value: { ...extraDetails, diff --git a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts index 6491b0ffbe0..8c104332324 100644 --- a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts +++ b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts @@ -1,6 +1,5 @@ import { submitComponentEvent } from '../client/ophan/ophan'; import { - AB_TEST_NAME, NEWSLETTER_SIGNUP_COMPONENT_ID, sendNewsletterSignupEvent, } from './newsletterSignupTracking'; @@ -14,20 +13,16 @@ const RENDERING_TARGET = 'Web'; const MOCK_TIMESTAMP = 1_000_000_000_000; describe('NEWSLETTER_SIGNUP_COMPONENT_ID', () => { - it('returns the correct control component id', () => { - expect(NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME)).toBe( - `AR SecureSignup ${IDENTITY_NAME}`, - ); + it('returns the in article signup form component id', () => { + expect( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(IDENTITY_NAME), + ).toBe(`AR NewsletterSignupForm ${IDENTITY_NAME}`); }); - it('returns the correct variant component id', () => { + it('returns the correct highlights card component id', () => { expect( - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - IDENTITY_NAME, - ), - ).toBe( - `AR NewsletterSignupForm ${IDENTITY_NAME} - variantIllustratedCard`, - ); + NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(IDENTITY_NAME), + ).toBe(`HighlightsNewsletterCard ${IDENTITY_NAME}`); }); }); @@ -45,7 +40,10 @@ describe('sendNewsletterSignupEvent', () => { sendNewsletterSignupEvent({ action: 'VIEW', identityName: IDENTITY_NAME, - componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME), + componentId: + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( + IDENTITY_NAME, + ), renderingTarget: RENDERING_TARGET, value: { eventDescription: 'newsletter-signup-viewed' }, }); @@ -54,7 +52,7 @@ describe('sendNewsletterSignupEvent', () => { { component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: `AR SecureSignup ${IDENTITY_NAME}`, + id: `AR NewsletterSignupForm ${IDENTITY_NAME}`, }, action: 'VIEW', value: JSON.stringify({ @@ -75,7 +73,7 @@ describe('sendNewsletterSignupEvent', () => { action: 'EXPAND', identityName: IDENTITY_NAME, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( IDENTITY_NAME, ), renderingTarget: RENDERING_TARGET, @@ -86,7 +84,7 @@ describe('sendNewsletterSignupEvent', () => { { component: { componentType: 'NEWSLETTER_SUBSCRIPTION', - id: `AR NewsletterSignupForm ${IDENTITY_NAME} - variantIllustratedCard`, + id: `AR NewsletterSignupForm ${IDENTITY_NAME}`, }, action: 'EXPAND', value: JSON.stringify({ @@ -106,7 +104,7 @@ describe('sendNewsletterSignupEvent', () => { action: 'CLOSE', identityName: IDENTITY_NAME, componentId: - NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( + NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm( IDENTITY_NAME, ), renderingTarget: RENDERING_TARGET, @@ -130,21 +128,27 @@ describe('sendNewsletterSignupEvent', () => { }); it('passes the abTest object to the top-level abTest field (not encoded in value)', () => { + const abTest = { + name: 'highlights-newsletter-card', + variant: 'highlightsCard', + }; + sendNewsletterSignupEvent({ action: 'VIEW', identityName: IDENTITY_NAME, - componentId: NEWSLETTER_SIGNUP_COMPONENT_ID.control(IDENTITY_NAME), + componentId: + NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(IDENTITY_NAME), renderingTarget: RENDERING_TARGET, value: { eventDescription: 'newsletter-signup-viewed', isAlreadySubscribed: false, }, - abTest: { name: AB_TEST_NAME, variant: 'control' }, + abTest, }); expect(submitComponentEvent).toHaveBeenCalledWith( expect.objectContaining({ - abTest: { name: AB_TEST_NAME, variant: 'control' }, + abTest, value: JSON.stringify({ eventDescription: 'newsletter-signup-viewed', isAlreadySubscribed: false, diff --git a/dotcom-rendering/src/lib/newsletterSignupTracking.ts b/dotcom-rendering/src/lib/newsletterSignupTracking.ts index 2c67124dbb5..623377943bf 100644 --- a/dotcom-rendering/src/lib/newsletterSignupTracking.ts +++ b/dotcom-rendering/src/lib/newsletterSignupTracking.ts @@ -2,8 +2,6 @@ import type { AbTest, TAction } from '@guardian/ophan-tracker-js'; import { submitComponentEvent } from '../client/ophan/ophan'; import type { RenderingTarget } from '../types/renderingTarget'; -export const AB_TEST_NAME = 'newsletters-signup-card-country-illustration'; - export type NewsletterEventDescription = | 'click-button' | 'email-input-focused' @@ -36,20 +34,15 @@ export const EVENT_DESCRIPTION_TO_ACTION = { } as const satisfies Record; /** - * The component ids used by each branch of the newsletter signup A/B test. + * Stable component IDs for newsletter signup tracking. * Keeping them here ensures the VIEW event (fired on mount in EmailSignUpWrapper) - * and subsequent events (EXPAND/CLOSE in NewsletterSignupCardContainer, SUBSCRIBE - * in SecureSignup) all share the same id, so they can be joined in Ophan. - * - * Note: both ids use the `AR` prefix as a shared convention, keeping them - * consistent with the pre-existing `SecureSignup` component id. + * and subsequent interaction events all share the same id so they can be + * joined in Ophan. */ export const NEWSLETTER_SIGNUP_COMPONENT_ID = { - control: (identityName: string) => `AR SecureSignup ${identityName}`, - variantNewField: (identityName: string) => - `AR SecureSignup ${identityName} - variantNewField`, - variantIllustratedCard: (identityName: string) => - `AR NewsletterSignupForm ${identityName} - variantIllustratedCard`, + secureSignup: (identityName: string) => `AR SecureSignup ${identityName}`, + inArticleSignupForm: (identityName: string) => + `AR NewsletterSignupForm ${identityName}`, highlightsCard: (identityName: string) => `HighlightsNewsletterCard ${identityName}`, } as const; @@ -57,9 +50,8 @@ export const NEWSLETTER_SIGNUP_COMPONENT_ID = { /** * Sends a newsletter signup tracking event to Ophan. * - * Used by both branches of the A/B test so that all events share a consistent - * component shape. Import this wherever you need to track an interaction with - * the email signup — e.g. VIEW on mount, EXPAND/CLOSE on preview open/close, + * Import this wherever you need to track an interaction with the email signup — + * e.g. VIEW on mount, EXPAND/CLOSE on preview open/close, * CLICK/ANSWER/SUBSCRIBE on form interactions. */ export const sendNewsletterSignupEvent = ({ diff --git a/dotcom-rendering/src/lib/useNewsletterSignupForm.ts b/dotcom-rendering/src/lib/useNewsletterSignupForm.ts index 6cfdccc54ac..81d830c2931 100644 --- a/dotcom-rendering/src/lib/useNewsletterSignupForm.ts +++ b/dotcom-rendering/src/lib/useNewsletterSignupForm.ts @@ -12,7 +12,6 @@ import { } from './newsletter-marketing-opt-in'; import { EVENT_DESCRIPTION_TO_ACTION, - NEWSLETTER_SIGNUP_COMPONENT_ID, type NewsletterEventDescription, sendNewsletterSignupEvent, } from './newsletterSignupTracking'; @@ -110,34 +109,15 @@ const postFormData = async ( }); }; -// TODO: when the in-article newsletter signup AB test (newsletters-signup-card-country-illustration) -// is cleaned up, refactor getComponentId (and useNewsletterSignupForm) to accept an explicit -// componentId rather than deriving it from abTest.variant, so tracking is not coupled to test state. -const getComponentId = (newsletterId: string, abTest?: AbTest): string => { - switch (abTest?.variant) { - case 'variantIllustratedCard': - return NEWSLETTER_SIGNUP_COMPONENT_ID.variantIllustratedCard( - newsletterId, - ); - case 'variantNewField': - return NEWSLETTER_SIGNUP_COMPONENT_ID.variantNewField(newsletterId); - case 'highlightsCard': - return NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard(newsletterId); - case undefined: - default: - return NEWSLETTER_SIGNUP_COMPONENT_ID.control(newsletterId); - } -}; - const sendTracking = ( newsletterId: string, + componentId: string, eventDescription: NewsletterEventDescription, renderingTarget: RenderingTarget, isSignedIn: boolean | 'Pending', abTest?: AbTest, extraDetails?: Record, ): void => { - const componentId = getComponentId(newsletterId, abTest); sendNewsletterSignupEvent({ action: EVENT_DESCRIPTION_TO_ACTION[eventDescription], identityName: newsletterId, @@ -233,6 +213,7 @@ export type NewsletterSignupFormState = { export const useNewsletterSignupForm = ( newsletterId: string, renderingTarget: RenderingTarget, + componentId: string, abTest?: AbTest, hideMarketingToggle = false, ): NewsletterSignupFormState => { @@ -351,6 +332,7 @@ export const useNewsletterSignupForm = ( sendTracking( newsletterId, + componentId, 'form-submission', renderingTarget, isSignedIn, @@ -388,6 +370,7 @@ export const useNewsletterSignupForm = ( sendTracking( newsletterId, + componentId, response.ok ? 'submission-confirmed' : 'submission-failed', renderingTarget, isSignedIn, @@ -395,7 +378,7 @@ export const useNewsletterSignupForm = ( marketingOptInType ? { marketingOptInType } : undefined, ); }, - [abTest, isSignedIn, newsletterId, renderingTarget], + [abTest, componentId, isSignedIn, newsletterId, renderingTarget], ); const handleCaptchaComplete = useCallback( @@ -403,6 +386,7 @@ export const useNewsletterSignupForm = ( if (!token) { sendTracking( newsletterId, + componentId, 'captcha-not-passed', renderingTarget, isSignedIn, @@ -416,6 +400,7 @@ export const useNewsletterSignupForm = ( } sendTracking( newsletterId, + componentId, 'captcha-passed', renderingTarget, isSignedIn, @@ -430,6 +415,7 @@ export const useNewsletterSignupForm = ( console.error(error); sendTracking( newsletterId, + componentId, 'form-submit-error', renderingTarget, isSignedIn, @@ -445,12 +431,20 @@ export const useNewsletterSignupForm = ( setIsWaitingForResponse(false); }); }, - [abTest, isSignedIn, newsletterId, renderingTarget, submitForm], + [ + abTest, + componentId, + isSignedIn, + newsletterId, + renderingTarget, + submitForm, + ], ); const handleCaptchaLoadError = useCallback((): void => { sendTracking( newsletterId, + componentId, 'captcha-load-error', renderingTarget, isSignedIn, @@ -460,7 +454,7 @@ export const useNewsletterSignupForm = ( setErrorMessage('Sorry, the reCAPTCHA failed to load.'); setIsWaitingForResponse(false); recaptchaRef.current?.reset(); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleSubmit = useCallback( (event: FormEvent): void => { @@ -480,6 +474,7 @@ export const useNewsletterSignupForm = ( setIsWaitingForResponse(true); sendTracking( newsletterId, + componentId, 'open-captcha', renderingTarget, isSignedIn, @@ -489,6 +484,7 @@ export const useNewsletterSignupForm = ( }, [ abTest, + componentId, isSignedIn, isWaitingForResponse, newsletterId, @@ -508,12 +504,13 @@ export const useNewsletterSignupForm = ( setIsInteracted(true); sendTracking( newsletterId, + componentId, 'email-input-focused', renderingTarget, isSignedIn, abTest, ); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleEmailInvalid = useCallback< React.FormEventHandler @@ -540,12 +537,13 @@ export const useNewsletterSignupForm = ( hasAttemptedSubmitRef.current = true; sendTracking( newsletterId, + componentId, 'click-button', renderingTarget, isSignedIn, abTest, ); - }, [abTest, isSignedIn, newsletterId, renderingTarget]); + }, [abTest, componentId, isSignedIn, newsletterId, renderingTarget]); const handleReset = useCallback< ReactEventHandler From b85fb78d44712611f213646891f99d8d4cbaae9c Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 12:10:06 +0100 Subject: [PATCH 3/8] Refactor EmailSignUpWrapper to remove newsletter subscription logic and simplify component structure --- .../EmailSignUpWrapper.island.test.tsx | 46 +----------------- .../components/EmailSignUpWrapper.island.tsx | 48 +------------------ .../components/EmailSignUpWrapper.stories.tsx | 39 +-------------- .../HighlightsNewsletterSignupModal.tsx | 4 -- dotcom-rendering/src/lib/renderElement.tsx | 8 ---- 5 files changed, 6 insertions(+), 139 deletions(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx index b86657799bb..2d262c83858 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx @@ -3,7 +3,6 @@ import { render, screen } from '@testing-library/react'; import { submitComponentEvent } from '../client/ophan/ophan'; import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; -import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { ConfigProvider } from './ConfigContext'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; @@ -15,10 +14,6 @@ jest.mock('../lib/useAuthStatus', () => ({ useIsSignedIn: jest.fn(), })); -jest.mock('../lib/useNewsletterSubscription', () => ({ - useNewsletterSubscription: jest.fn(), -})); - // Avoid rendering real island children in unit tests jest.mock('./Island', () => ({ Island: ({ children }: { children: React.ReactNode }) => <>{children}, @@ -44,13 +39,11 @@ jest.mock('./NewsletterSignupCardContainer', () => ({ const defaultProps = { index: 0, - listId: 4147, identityName: 'the-recap', name: 'The Recap', description: 'A weekly sports roundup', frequency: 'Weekly', theme: 'sport', - idApiUrl: 'https://idapi.theguardian.com', }; const renderWrapper = (props = {}, renderingTarget: 'Web' | 'Apps' = 'Web') => @@ -71,19 +64,10 @@ describe('EmailSignUpWrapper', () => { beforeEach(() => { jest.resetAllMocks(); (useIsSignedIn as jest.Mock).mockReturnValue(false); - (useNewsletterSubscription as jest.Mock).mockReturnValue(false); }); describe('rendering', () => { - it('shows a placeholder while subscription status is loading', () => { - (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); - renderWrapper(); - expect( - screen.queryByTestId('newsletter-signup-card-container'), - ).not.toBeInTheDocument(); - }); - - it('renders the NewsletterSignupCardContainer when the user is not subscribed', () => { + it('renders the NewsletterSignupCardContainer', () => { renderWrapper(); expect( screen.getByTestId('newsletter-signup-card-container'), @@ -93,16 +77,6 @@ describe('EmailSignUpWrapper', () => { ).toBeInTheDocument(); }); - it('still renders the card for already-subscribed users (subscription handled inside the card)', () => { - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - hideNewsletterSignupComponentForSubscribers: true, - }); - expect( - screen.getByTestId('newsletter-signup-card-container'), - ).toBeInTheDocument(); - }); - it('renders the NewsletterSignupCardContainer on Apps as well as Web', () => { renderWrapper({}, 'Apps'); expect( @@ -112,7 +86,7 @@ describe('EmailSignUpWrapper', () => { }); describe('VIEW tracking', () => { - it('fires a VIEW event with the illustrated card component id', () => { + it('fires a VIEW event with the in-article signup form component id', () => { renderWrapper(); expect(submitComponentEvent).toHaveBeenCalledWith( @@ -134,21 +108,5 @@ describe('EmailSignUpWrapper', () => { expect(submitComponentEvent).toHaveBeenCalledTimes(1); }); - - it('does not fire a VIEW event while subscription status is loading', () => { - (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); - renderWrapper(); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); - - it('does not fire a VIEW event when the user is already subscribed', () => { - (useNewsletterSubscription as jest.Mock).mockReturnValue(true); - renderWrapper({ - hideNewsletterSignupComponentForSubscribers: true, - }); - - expect(submitComponentEvent).not.toHaveBeenCalled(); - }); }); }); diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx index 003ad80f3fe..b57794683e4 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx @@ -1,75 +1,46 @@ -import type { Breakpoint } from '@guardian/source/foundations'; import { useEffect, useRef } from 'react'; import { NEWSLETTER_SIGNUP_COMPONENT_ID, sendNewsletterSignupEvent, } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; -import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { useConfig } from './ConfigContext'; import type { EmailSignUpProps } from './EmailSignup'; import { InlineSkipToWrapper } from './InlineSkipToWrapper'; import { Island } from './Island'; import { NewsletterSignupCardContainer } from './NewsletterSignupCardContainer'; import { NewsletterSignupForm } from './NewsletterSignupForm.island'; -import { Placeholder } from './Placeholder'; // When the next A/B experiment is added (e.g. preview-button test), import // useAB and AB_TEST_NAME from their respective modules and thread them through // sendNewsletterSignupEvent's `abTest` param and NewsletterSignupForm's `abTest` prop. -/** - * Approximate heights of the EmailSignup component at different breakpoints. - */ -const PLACEHOLDER_HEIGHTS = new Map([ - ['mobile', 220], - ['tablet', 180], - ['desktop', 180], -]); - interface EmailSignUpWrapperProps extends EmailSignUpProps { index: number; - listId: number; identityName: string; category?: string; /** Illustration image URL (square crop) for the NewsletterSignupCard */ illustrationSquare?: string; - idApiUrl: string; exampleUrl?: string; - /** Feature flag to enable hiding newsletter signup for already subscribed users */ - hideNewsletterSignupComponentForSubscribers?: boolean; } /** * EmailSignUpWrapper as an island component. * * This component needs to be hydrated client-side because it uses - * the useNewsletterSubscription hook which depends on auth status - * to determine if the user is already subscribed to the newsletter. - * - * If the user is signed in and already subscribed, this component - * will return null (hide the signup form). + * client-side hooks for auth status and tracking. */ export const EmailSignUpWrapper = ({ index, - listId, identityName, category, - idApiUrl, exampleUrl, name, description, illustrationSquare, frequency, theme, - hideNewsletterSignupComponentForSubscribers = false, }: EmailSignUpWrapperProps) => { const { renderingTarget } = useConfig(); - - const isSubscribed = useNewsletterSubscription( - listId, - idApiUrl, - hideNewsletterSignupComponentForSubscribers, - ); const isSignedIn = useIsSignedIn(); const componentId = @@ -78,16 +49,6 @@ export const EmailSignUpWrapper = ({ const viewFiredRef = useRef(false); useEffect(() => { - // Wait for subscription status — we only want to track a view of the - // actual signup form, not a loading state or success message. - if (isSubscribed === undefined) { - return; - } - // Don't fire if the user is already subscribed. - if (isSubscribed) { - return; - } - // Guard against double-firing (e.g. if deps change after the first fire) if (viewFiredRef.current) { return; } @@ -101,11 +62,7 @@ export const EmailSignUpWrapper = ({ eventDescription: 'newsletter-signup-viewed', }, }); - }, [componentId, identityName, isSubscribed, renderingTarget]); - - if (isSubscribed === undefined) { - return ; - } + }, [componentId, identityName, renderingTarget]); return ( diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index e79d6ce74b0..3164794ac1a 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -3,7 +3,6 @@ import { mocked } from 'storybook/test'; import preview from '../../.storybook/preview'; import { lazyFetchEmailWithTimeout } from '../lib/fetchEmail'; import { useIsSignedIn } from '../lib/useAuthStatus'; -import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; const meta = preview.meta({ @@ -15,53 +14,20 @@ type Story = StoryObj; const defaultArgs = { index: 10, - listId: 4147, identityName: 'the-recap', description: "The best of our sports journalism from the past seven days and a heads-up on the weekend's action", name: 'The Recap', frequency: 'Weekly', theme: 'sport', - idApiUrl: 'https://idapi.theguardian.com', exampleUrl: 'https://www.theguardian.com/email/the-recap', - hideNewsletterSignupComponentForSubscribers: true, illustrationSquare: 'https://i.guim.co.uk/img/uploads/2023/11/01/SaturdayEdition_-_5-3.jpg?width=220&dpr=2&s=none&crop=5%3A3', } satisfies Story['args']; -export const LoadingPlaceholder = meta.story({ +export const SignedIn = meta.story({ args: { ...defaultArgs }, beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(undefined); - }, -}); - -export const SignedInNotSubscribed = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(true); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve('test@example.com'), - ); - }, -}); - -export const SignedOutNotSubscribed = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(false); - mocked(useIsSignedIn).mockReturnValue(false); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve(null), - ); - }, -}); - -export const SignedInAlreadySubscribed = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => Promise.resolve('test@example.com'), @@ -69,10 +35,9 @@ export const SignedInAlreadySubscribed = meta.story({ }, }); -export const SignedOutAlreadySubscribed = meta.story({ +export const SignedOut = meta.story({ args: { ...defaultArgs }, beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => Promise.resolve(null), diff --git a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx index 8c5e80fba1b..cb5bdb091b1 100644 --- a/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx +++ b/dotcom-rendering/src/components/Masthead/Newsletter/HighlightsNewsletterSignupModal.tsx @@ -147,10 +147,6 @@ const HighlightsNewsletterSignupModalContent = ({ componentId={NEWSLETTER_SIGNUP_COMPONENT_ID.highlightsCard( newsletter.identityName, )} - abTest={{ - name: 'highlights-newsletter-card', - variant: 'highlightsCard', - }} /> diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index f4bc1500347..bc48af9764c 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -107,7 +107,6 @@ type Props = { shouldHideAds: boolean; contentType?: string; contentLayout?: string; - idApiUrl?: string; }; // updateRole modifies the role of an element in a way appropriate for most @@ -179,7 +178,6 @@ export const renderElement = ({ shouldHideAds, contentType, contentLayout, - idApiUrl, }: Props) => { const isBlog = format.design === ArticleDesign.LiveBlog || @@ -589,7 +587,6 @@ export const renderElement = ({ const newsletter = element.newsletter; const emailSignUpProps = { index, - listId: newsletter.listId, identityName: newsletter.identityName, category: newsletter.category, description: newsletter.description, @@ -598,9 +595,6 @@ export const renderElement = ({ theme: newsletter.theme, illustrationSquare: newsletter.illustrationSquare, exampleUrl: newsletter.exampleUrl, - idApiUrl: idApiUrl ?? '', - hideNewsletterSignupComponentForSubscribers: - !!switches.hideNewsletterSignupComponentForSubscribers, }; if (isListElement || isTimeline) { return null; @@ -1052,7 +1046,6 @@ export const RenderArticleElement = ({ shouldHideAds, contentType, contentLayout, - idApiUrl, }: Props) => { const withUpdatedRole = updateRole(element, format); @@ -1081,7 +1074,6 @@ export const RenderArticleElement = ({ shouldHideAds, contentType, contentLayout, - idApiUrl, }); const needsFigure = !bareElements.has(element._type); From 61f87aebcd87ed5b0225a1ef239234bc6364e3b7 Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 12:46:54 +0100 Subject: [PATCH 4/8] Continue to pass through is already subscribed --- .../components/EmailSignUpWrapper.island.tsx | 18 +++++++++++++++++- dotcom-rendering/src/lib/renderElement.tsx | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx index b57794683e4..84b203d8dbf 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.tsx @@ -4,6 +4,7 @@ import { sendNewsletterSignupEvent, } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; +import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { useConfig } from './ConfigContext'; import type { EmailSignUpProps } from './EmailSignup'; import { InlineSkipToWrapper } from './InlineSkipToWrapper'; @@ -16,10 +17,12 @@ import { NewsletterSignupForm } from './NewsletterSignupForm.island'; interface EmailSignUpWrapperProps extends EmailSignUpProps { index: number; + listId: number; identityName: string; category?: string; /** Illustration image URL (square crop) for the NewsletterSignupCard */ illustrationSquare?: string; + idApiUrl: string; exampleUrl?: string; } @@ -31,8 +34,10 @@ interface EmailSignUpWrapperProps extends EmailSignUpProps { */ export const EmailSignUpWrapper = ({ index, + listId, identityName, category, + idApiUrl, exampleUrl, name, description, @@ -42,6 +47,7 @@ export const EmailSignUpWrapper = ({ }: EmailSignUpWrapperProps) => { const { renderingTarget } = useConfig(); const isSignedIn = useIsSignedIn(); + const isSubscribed = useNewsletterSubscription(listId, idApiUrl); const componentId = NEWSLETTER_SIGNUP_COMPONENT_ID.inArticleSignupForm(identityName); @@ -49,6 +55,15 @@ export const EmailSignUpWrapper = ({ const viewFiredRef = useRef(false); useEffect(() => { + // Wait for subscription status — we only want to track a view of the + // actual signup form, not a loading state or success message. + if (isSubscribed === undefined) { + return; + } + // Don't fire if the user is already subscribed. + if (isSubscribed) { + return; + } if (viewFiredRef.current) { return; } @@ -62,7 +77,7 @@ export const EmailSignUpWrapper = ({ eventDescription: 'newsletter-signup-viewed', }, }); - }, [componentId, identityName, renderingTarget]); + }, [componentId, identityName, isSubscribed, renderingTarget]); return ( )} diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index bc48af9764c..322d36f4596 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -107,6 +107,8 @@ type Props = { shouldHideAds: boolean; contentType?: string; contentLayout?: string; + /** @deprecated No longer forwarded to renderElement — retained only so call sites do not need updating. */ + idApiUrl?: string; }; // updateRole modifies the role of an element in a way appropriate for most @@ -1046,6 +1048,7 @@ export const RenderArticleElement = ({ shouldHideAds, contentType, contentLayout, + idApiUrl: _idApiUrl, // accepted for backwards compatibility but no longer used }: Props) => { const withUpdatedRole = updateRole(element, format); From 1ced401f45304b4c5bcc2585e3cfb2c9cf4af881 Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 12:51:44 +0100 Subject: [PATCH 5/8] Revert signup wrapper stories --- .../EmailSignUpWrapper.island.test.tsx | 30 ++++++++++ .../components/EmailSignUpWrapper.stories.tsx | 59 ++++++++++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx index 2d262c83858..6dfed7691ea 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.island.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from '@testing-library/react'; import { submitComponentEvent } from '../client/ophan/ophan'; import { NEWSLETTER_SIGNUP_COMPONENT_ID } from '../lib/newsletterSignupTracking'; import { useIsSignedIn } from '../lib/useAuthStatus'; +import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { ConfigProvider } from './ConfigContext'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; @@ -14,6 +15,10 @@ jest.mock('../lib/useAuthStatus', () => ({ useIsSignedIn: jest.fn(), })); +jest.mock('../lib/useNewsletterSubscription', () => ({ + useNewsletterSubscription: jest.fn(), +})); + // Avoid rendering real island children in unit tests jest.mock('./Island', () => ({ Island: ({ children }: { children: React.ReactNode }) => <>{children}, @@ -39,11 +44,13 @@ jest.mock('./NewsletterSignupCardContainer', () => ({ const defaultProps = { index: 0, + listId: 4147, identityName: 'the-recap', name: 'The Recap', description: 'A weekly sports roundup', frequency: 'Weekly', theme: 'sport', + idApiUrl: 'https://idapi.theguardian.com', }; const renderWrapper = (props = {}, renderingTarget: 'Web' | 'Apps' = 'Web') => @@ -64,6 +71,7 @@ describe('EmailSignUpWrapper', () => { beforeEach(() => { jest.resetAllMocks(); (useIsSignedIn as jest.Mock).mockReturnValue(false); + (useNewsletterSubscription as jest.Mock).mockReturnValue(false); }); describe('rendering', () => { @@ -83,6 +91,14 @@ describe('EmailSignUpWrapper', () => { screen.getByTestId('newsletter-signup-card-container'), ).toBeInTheDocument(); }); + + it('still renders the card when the user is already subscribed', () => { + (useNewsletterSubscription as jest.Mock).mockReturnValue(true); + renderWrapper(); + expect( + screen.getByTestId('newsletter-signup-card-container'), + ).toBeInTheDocument(); + }); }); describe('VIEW tracking', () => { @@ -108,5 +124,19 @@ describe('EmailSignUpWrapper', () => { expect(submitComponentEvent).toHaveBeenCalledTimes(1); }); + + it('does not fire a VIEW event while subscription status is loading', () => { + (useNewsletterSubscription as jest.Mock).mockReturnValue(undefined); + renderWrapper(); + + expect(submitComponentEvent).not.toHaveBeenCalled(); + }); + + it('does not fire a VIEW event when the user is already subscribed', () => { + (useNewsletterSubscription as jest.Mock).mockReturnValue(true); + renderWrapper(); + + expect(submitComponentEvent).not.toHaveBeenCalled(); + }); }); }); diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index 3164794ac1a..45cf9e76ecd 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -1,8 +1,9 @@ import type { StoryObj } from '@storybook/react-webpack5'; -import { mocked } from 'storybook/test'; +import { mocked, within } from 'storybook/test'; import preview from '../../.storybook/preview'; import { lazyFetchEmailWithTimeout } from '../lib/fetchEmail'; import { useIsSignedIn } from '../lib/useAuthStatus'; +import { useNewsletterSubscription } from '../lib/useNewsletterSubscription'; import { EmailSignUpWrapper } from './EmailSignUpWrapper.island'; const meta = preview.meta({ @@ -14,20 +15,46 @@ type Story = StoryObj; const defaultArgs = { index: 10, + listId: 4147, identityName: 'the-recap', description: "The best of our sports journalism from the past seven days and a heads-up on the weekend's action", name: 'The Recap', frequency: 'Weekly', theme: 'sport', + idApiUrl: 'https://idapi.theguardian.com', exampleUrl: 'https://www.theguardian.com/email/the-recap', illustrationSquare: 'https://i.guim.co.uk/img/uploads/2023/11/01/SaturdayEdition_-_5-3.jpg?width=220&dpr=2&s=none&crop=5%3A3', } satisfies Story['args']; +/** Subscription status is still resolving — form renders without subscribed state yet. */ +export const Loading = meta.story({ + args: { ...defaultArgs }, + beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(undefined); + mocked(useIsSignedIn).mockReturnValue(false); + mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => + Promise.resolve(null), + ); + }, +}); + +export const SignedOut = meta.story({ + args: { ...defaultArgs }, + beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(false); + mocked(useIsSignedIn).mockReturnValue(false); + mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => + Promise.resolve(null), + ); + }, +}); + export const SignedIn = meta.story({ args: { ...defaultArgs }, beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(false); mocked(useIsSignedIn).mockReturnValue(true); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => Promise.resolve('test@example.com'), @@ -35,12 +62,40 @@ export const SignedIn = meta.story({ }, }); -export const SignedOut = meta.story({ +export const SignedInAlreadySubscribed = meta.story({ args: { ...defaultArgs }, beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(true); + mocked(useIsSignedIn).mockReturnValue(true); + mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => + Promise.resolve('test@example.com'), + ); + }, +}); + +export const SignedOutAlreadySubscribed = meta.story({ + args: { ...defaultArgs }, + beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(true); mocked(useIsSignedIn).mockReturnValue(false); mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => Promise.resolve(null), ); }, }); + +export const EmailInputFocused = meta.story({ + args: { ...defaultArgs }, + beforeEach() { + mocked(useNewsletterSubscription).mockReturnValue(false); + mocked(useIsSignedIn).mockReturnValue(false); + mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => + Promise.resolve(null), + ); + }, + async play({ canvasElement }) { + const canvas = within(canvasElement); + const emailInput = await canvas.findByLabelText('Enter your email'); + emailInput.focus(); + }, +}); From 447050a96c6ed5f923a3c6a3e4045ade32b0932a Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 13:03:54 +0100 Subject: [PATCH 6/8] fix renderElement being missing --- dotcom-rendering/src/lib/renderElement.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index 322d36f4596..f877210f059 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -107,7 +107,6 @@ type Props = { shouldHideAds: boolean; contentType?: string; contentLayout?: string; - /** @deprecated No longer forwarded to renderElement — retained only so call sites do not need updating. */ idApiUrl?: string; }; @@ -180,6 +179,7 @@ export const renderElement = ({ shouldHideAds, contentType, contentLayout, + idApiUrl, }: Props) => { const isBlog = format.design === ArticleDesign.LiveBlog || @@ -589,6 +589,7 @@ export const renderElement = ({ const newsletter = element.newsletter; const emailSignUpProps = { index, + listId: newsletter.listId, identityName: newsletter.identityName, category: newsletter.category, description: newsletter.description, @@ -597,6 +598,7 @@ export const renderElement = ({ theme: newsletter.theme, illustrationSquare: newsletter.illustrationSquare, exampleUrl: newsletter.exampleUrl, + idApiUrl: idApiUrl ?? '', }; if (isListElement || isTimeline) { return null; @@ -1048,7 +1050,7 @@ export const RenderArticleElement = ({ shouldHideAds, contentType, contentLayout, - idApiUrl: _idApiUrl, // accepted for backwards compatibility but no longer used + idApiUrl, }: Props) => { const withUpdatedRole = updateRole(element, format); @@ -1077,6 +1079,7 @@ export const RenderArticleElement = ({ shouldHideAds, contentType, contentLayout, + idApiUrl, }); const needsFigure = !bareElements.has(element._type); From 22ffc3352ecae32ad2331efc33fb8ff8d5725f66 Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 14:11:46 +0100 Subject: [PATCH 7/8] correct test setup data to be clearer --- dotcom-rendering/src/lib/newsletterSignupTracking.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts index 8c104332324..f21cc47bf84 100644 --- a/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts +++ b/dotcom-rendering/src/lib/newsletterSignupTracking.test.ts @@ -129,8 +129,8 @@ describe('sendNewsletterSignupEvent', () => { it('passes the abTest object to the top-level abTest field (not encoded in value)', () => { const abTest = { - name: 'highlights-newsletter-card', - variant: 'highlightsCard', + name: 'ab-test-name', + variant: 'ab-test-variant', }; sendNewsletterSignupEvent({ From 92b8e98c63e609a085bd5eacf55e0c15d56a1eee Mon Sep 17 00:00:00 2001 From: George Richmond Date: Mon, 22 Jun 2026 16:17:43 +0100 Subject: [PATCH 8/8] remove loading story as not required --- .../src/components/EmailSignUpWrapper.stories.tsx | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx index 45cf9e76ecd..a3e92e981dd 100644 --- a/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx +++ b/dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx @@ -28,18 +28,6 @@ const defaultArgs = { 'https://i.guim.co.uk/img/uploads/2023/11/01/SaturdayEdition_-_5-3.jpg?width=220&dpr=2&s=none&crop=5%3A3', } satisfies Story['args']; -/** Subscription status is still resolving — form renders without subscribed state yet. */ -export const Loading = meta.story({ - args: { ...defaultArgs }, - beforeEach() { - mocked(useNewsletterSubscription).mockReturnValue(undefined); - mocked(useIsSignedIn).mockReturnValue(false); - mocked(lazyFetchEmailWithTimeout).mockReturnValue(() => - Promise.resolve(null), - ); - }, -}); - export const SignedOut = meta.story({ args: { ...defaultArgs }, beforeEach() {