Skip to content
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
74f916e
Hide signUp card for signed in users in newsletter
Marianaguardian Nov 26, 2025
6c6ffc1
removed unused imports
Marianaguardian Nov 26, 2025
177412b
fixed lint issues
Marianaguardian Nov 26, 2025
b999d89
Merge branch 'main' of https://github.com/guardian/dotcom-rendering i…
Marianaguardian Nov 26, 2025
da6e644
Merge branch 'main' of https://github.com/guardian/dotcom-rendering i…
Marianaguardian Nov 26, 2025
a5b9c4b
remove console
Marianaguardian Nov 26, 2025
7ad7a22
Merge branch 'main' of https://github.com/guardian/dotcom-rendering i…
Marianaguardian Nov 26, 2025
6f12d91
updated from main branch
Marianaguardian Nov 26, 2025
bb54b31
updated idApiUrl type
Marianaguardian Nov 26, 2025
4927841
Merge branch 'main' of https://github.com/guardian/dotcom-rendering i…
Marianaguardian Nov 26, 2025
8582503
Minor changes for testing
Marianaguardian Nov 28, 2025
b56c69f
Merge branch 'main' into 14890-show-or-hide-the-sign-up-newsletter-co…
Marianaguardian Nov 28, 2025
618f558
reverted test changes and correct response structure for user newslet…
Marianaguardian Dec 1, 2025
08ba7cc
Merge branch '14890-show-or-hide-the-sign-up-newsletter-component' of…
Marianaguardian Dec 1, 2025
6e620a5
Merge branch 'main' into 14890-show-or-hide-the-sign-up-newsletter-co…
Marianaguardian Dec 1, 2025
8eed71c
render newsletter list id of current news letter for testing and upda…
Marianaguardian Dec 1, 2025
97e519d
added console logs for debuging
Marianaguardian Dec 1, 2025
f0de441
render auth status and api reponse
Marianaguardian Dec 1, 2025
d5dbdaa
fix lint error
Marianaguardian Dec 1, 2025
6161c89
Removed debugging logs and cleaned up code
Marianaguardian Dec 1, 2025
67ce48a
Wrapped email signup wrapper in Island for runtime render
Marianaguardian Dec 1, 2025
c190059
Addressed PR comments
Marianaguardian Dec 2, 2025
0a9eb75
Merge branch 'main' of https://github.com/guardian/dotcom-rendering i…
Marianaguardian Dec 2, 2025
c2f8825
resolved lint issue
Marianaguardian Dec 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { useState } from 'react';
import { useNewsletterSubscription } from '../lib/useNewsletterSubscription';
import type { EmailSignUpProps } from './EmailSignup';
import { EmailSignup } from './EmailSignup';
import { InlineSkipToWrapper } from './InlineSkipToWrapper';
Expand All @@ -7,16 +9,39 @@ import { SecureSignup } from './SecureSignup.importable';

interface EmailSignUpWrapperProps extends EmailSignUpProps {
index: number;
listId: number;
identityName: string;
successDescription: string;
/** You should only set this to true if the privacy message will be shown elsewhere on the page */
hidePrivacyMessage?: boolean;
}

/**
* EmailSignUpWrapper as an importable 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).
*/
export const EmailSignUpWrapper = ({
index,
listId,
...emailSignUpProps
}: EmailSignUpWrapperProps) => {
const [idApiUrl] = useState(() => {
if (typeof window === 'undefined') return undefined;
return window.guardian?.config?.page?.idApiUrl ?? undefined;
});
const isSubscribed = useNewsletterSubscription(listId, idApiUrl);

// Don't render if user is signed in and already subscribed
if (isSubscribed === true) {
return null;
}
Comment on lines +41 to +43
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we initially render the the newsletter component and then remove it if we determine there is a subscription?

I assume we have considered this approach will cause the least layout shift vs not showing and then inserting.

There is also the possibility of showing a newsletter related placeholder and replacing with the aim of zero layout shift.


return (
<InlineSkipToWrapper
id={`EmailSignup-skip-link-${index}`}
Expand Down
61 changes: 58 additions & 3 deletions dotcom-rendering/src/components/EmailSignUpWrapper.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
import type { Meta, StoryObj } from '@storybook/react-webpack5';
import { EmailSignUpWrapper } from './EmailSignUpWrapper';
import { useEffect } from 'react';
import type { EmailSignUpProps } from './EmailSignup';
import { EmailSignUpWrapper } from './EmailSignUpWrapper.importable';

const meta: Meta<typeof EmailSignUpWrapper> = {
title: 'Components/EmailSignUpWrapper',
component: EmailSignUpWrapper,
};

type Story = StoryObj<typeof EmailSignUpWrapper>;

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 weekends action',
"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',
} satisfies Story['args'];
type Story = StoryObj<typeof EmailSignUpWrapper>;

export const DefaultStory: Story = {
args: {
Expand All @@ -32,4 +36,55 @@ export const DefaultStoryWithPrivacy: Story = {
},
};

// Simulates when user is NOT subscribed - signup form is visible
export const NotSubscribedState: Story = {
args: {
hidePrivacyMessage: false,
...defaultArgs,
},
decorators: [(Story) => <Story />],
};

// Simulates when user IS subscribed - component returns null
interface EmailSignUpWrapperProps extends EmailSignUpProps {
index: number;
listId: number;
identityName: string;
successDescription: string;
hidePrivacyMessage?: boolean;
}

const AlreadySubscribedWrapper = (props: EmailSignUpWrapperProps) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use the hook in the react story and just mock out what the service call returns?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either by mocking fetch or if there is a standard way it's mocked out in the codebase so that we can see the hook being used in the story.

Otherwise it's just repeating logic from the hook for the point of the story - may as well just show the component as it looks with and without a subscription

// Directly use the hook logic - in a real scenario it would return true
// For storybook, we simulate by just returning null
const isSubscribed = true; // Simulating hook return value

useEffect(() => {
const idApiUrl =
typeof window !== 'undefined'
? window.guardian?.config?.page?.idApiUrl
: undefined;
console.log(
'AlreadySubscribedState: useNewsletterSubscription would return true',
);
console.log('Props:', { listId: props.listId, idApiUrl });
}, [props.listId]);

if (isSubscribed) {
return null;
}

return <EmailSignUpWrapper {...props} />;
};

export const AlreadySubscribedState = {
render: (args: EmailSignUpWrapperProps) => (
<AlreadySubscribedWrapper {...args} />
),
args: {
...defaultArgs,
hidePrivacyMessage: false,
},
} satisfies Story;

export default meta;
9 changes: 7 additions & 2 deletions dotcom-rendering/src/lib/renderElement.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CommentBlockComponent } from '../components/CommentBlockComponent';
import { CrosswordComponent } from '../components/CrosswordComponent.importable';
import { DividerBlockComponent } from '../components/DividerBlockComponent';
import { DocumentBlockComponent } from '../components/DocumentBlockComponent.importable';
import { EmailSignUpWrapper } from '../components/EmailSignUpWrapper';
import { EmailSignUpWrapper } from '../components/EmailSignUpWrapper.importable';
import { EmbedBlockComponent } from '../components/EmbedBlockComponent.importable';
import { ExplainerAtom } from '../components/ExplainerAtom';
import { Figure } from '../components/Figure';
Expand Down Expand Up @@ -546,6 +546,7 @@ export const renderElement = ({
case 'model.dotcomrendering.pageElements.NewsletterSignupBlockElement':
const emailSignUpProps = {
index,
listId: element.newsletter.listId,
identityName: element.newsletter.identityName,
description: element.newsletter.description,
name: element.newsletter.name,
Expand All @@ -554,7 +555,11 @@ export const renderElement = ({
theme: element.newsletter.theme,
};
if (isListElement || isTimeline) return null;
return <EmailSignUpWrapper {...emailSignUpProps} />;
return (
<Island priority="feature" defer={{ until: 'visible' }}>
<EmailSignUpWrapper {...emailSignUpProps} />
</Island>
);
case 'model.dotcomrendering.pageElements.AdPlaceholderBlockElement':
return renderAds && <AdPlaceholder />;
case 'model.dotcomrendering.pageElements.NumberedTitleBlockElement':
Expand Down
93 changes: 93 additions & 0 deletions dotcom-rendering/src/lib/useNewsletterSubscription.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import { useEffect, useState } from 'react';
import { getOptionsHeaders } from './identity';
import { useAuthStatus } from './useAuthStatus';

interface NewsletterSubscriptionResponse {
result: {
subscriptions: Array<{
listId: string;
}>;
};
}

/**
* A hook to check if a user is subscribed to a specific newsletter.
*/
export const useNewsletterSubscription = (

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth having a test for this?

Copy link

@georgerichmond georgerichmond Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be tested as part of testing the component by mocking the fetch call possibly

newsletterId: number,
idApiUrl: string | undefined,
): boolean | undefined => {
const [isSubscribed, setIsSubscribed] = useState<boolean | undefined>(
undefined,
);

const authStatus = useAuthStatus();

useEffect(() => {
// Wait for auth to be determined
if (authStatus.kind === 'Pending') {
setIsSubscribed(undefined);
return;
}

// User is not signed in
if (authStatus.kind === 'SignedOut') {
setIsSubscribed(false);
return;
}

// No API URL available
if (idApiUrl === undefined || idApiUrl === '') {
setIsSubscribed(false);
return;
}

// At this point: authStatus.kind === 'SignedIn'

// User is signed in, fetch their newsletters
const fetchNewsletters = async () => {
try {
const options = getOptionsHeaders(authStatus);
const origin = window.location.origin;
const response = await fetch(
`${idApiUrl}/users/me/newsletters`,
{
method: 'GET',
credentials: 'include',
...options,
headers: {
...options.headers,
Origin: origin,
Referer: origin,
},
},
);

if (!response.ok) {
console.error('Failed to fetch user newsletters');
setIsSubscribed(false);
return;
}

const data: NewsletterSubscriptionResponse =
await response.json();

const newsletters = data.result?.subscriptions ?? [];

// If newsletter exists in the subscriptions array, user is subscribed
const isUserSubscribed = newsletters.some(
(n) => Number(n.listId) === newsletterId,
);

setIsSubscribed(isUserSubscribed);
} catch (error) {
console.error('Error fetching newsletters:', error);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than console.error, we have a Sentry reportError function on the window which will allow you to report errors to Sentry. Here's an example e.g.:

window.guardian.modules.sentry.reportError(
new Error(errorMessage),
'rr-banner',
);

setIsSubscribed(false);
}
};

void fetchNewsletters();
}, [authStatus, newsletterId, idApiUrl]);

return isSubscribed;
};
Loading