diff --git a/dotcom-rendering/src/components/SlotBodyEnd.importable.tsx b/dotcom-rendering/src/components/SlotBodyEnd.importable.tsx index 7b8b476863f..3547e6246bf 100644 --- a/dotcom-rendering/src/components/SlotBodyEnd.importable.tsx +++ b/dotcom-rendering/src/components/SlotBodyEnd.importable.tsx @@ -14,11 +14,7 @@ import type { import type { EpicProps } from '@guardian/support-dotcom-components/dist/shared/types'; import { useEffect, useState } from 'react'; import { getArticleCounts } from '../lib/articleCount'; -import type { - CandidateConfig, - MaybeFC, - SlotConfig, -} from '../lib/messagePicker'; +import type { CandidateConfig, SlotConfig } from '../lib/messagePicker'; import { pickMessage } from '../lib/messagePicker'; import { useIsSignedIn } from '../lib/useAuthStatus'; import { useBraze } from '../lib/useBraze'; @@ -60,7 +56,7 @@ const buildReaderRevenueEpicConfig = ( candidate: { id: 'reader-revenue-banner', canShow: () => canShowReaderRevenueEpic(canShowData), - show: (data: ModuleData) => () => { + show: (data: ModuleData) => { return ; }, }, @@ -88,7 +84,7 @@ const buildBrazeEpicConfig = ( tags, shouldHideReaderRevenue, ), - show: (meta: any) => () => ( + show: (meta: any) => ( MaybeFC) => setSelectedEpic(PickedEpic)) + .then((result) => { + if (result.type === 'MessageSelected') { + setSelectedEpic(result.SelectedMessage); + } else { + setSelectedEpic(() => null); + } + }) .catch((e) => console.error(`SlotBodyEnd pickMessage - error: ${String(e)}`), ); diff --git a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx index f82bb658b54..e330d3c0a57 100644 --- a/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx +++ b/dotcom-rendering/src/components/StickyBottomBanner.importable.tsx @@ -11,7 +11,7 @@ import type { ArticleCounts } from '../lib/articleCount'; import { getArticleCounts } from '../lib/articleCount'; import type { CandidateConfig, - MaybeFC, + PickMessageResult, SlotConfig, } from '../lib/messagePicker'; import { pickMessage } from '../lib/messagePicker'; @@ -170,9 +170,9 @@ const buildRRBannerConfigWith = ({ ophanPageViewId, pageId, }), - show: - ({ name, props }: ModuleData) => - () => , + show: ({ name, props }: ModuleData) => ( + + ), }, timeoutMillis: DEFAULT_BANNER_TIMEOUT_MILLIS, }; @@ -188,7 +188,7 @@ const buildSignInGateConfig = ( canShow: async () => { return await canShowSignInGatePortal(canShowProps); }, - show: (meta: AuxiaGateDisplayData) => () => ( + show: (meta: AuxiaGateDisplayData) => ( () => ( + show: (meta: BrazeMeta) => ( ), }, @@ -274,9 +274,9 @@ export const StickyBottomBanner = ({ 'control', ); - const [SelectedBanner, setSelectedBanner] = useState(null); - const [hasPickMessageCompleted, setHasPickMessageCompleted] = - useState(false); + const [pickMessageResult, setPickMessageResult] = + useState(null); + const [asyncArticleCounts, setAsyncArticleCounts] = useState>(); @@ -367,9 +367,8 @@ export const StickyBottomBanner = ({ }; pickMessage(bannerConfig, renderingTarget) - .then((PickedBanner: () => MaybeFC) => { - setSelectedBanner(PickedBanner); - setHasPickMessageCompleted(true); + .then((result) => { + setPickMessageResult(result); }) .catch((e) => { // Report error to Sentry @@ -380,7 +379,6 @@ export const StickyBottomBanner = ({ new Error(msg), 'sticky-bottom-banner', ); - setHasPickMessageCompleted(true); }); }, [ isSignedIn, @@ -406,16 +404,27 @@ export const StickyBottomBanner = ({ isInAuxiaControlGroup, ]); - // Dispatches 'banner:none' event for mobile sticky ad integration (see @guardian/commercial-dev). - // Ensures ads only insert when no banner will be shown. - // hasPickMessageCompleted distinguishes between initial state (not picked yet) and final state (picked nothing). + /** + * Custom events for commercial purposes to avoid the mobile-sticky ad slot clashing with these banners + * Dispatches events when no banner is due to appear and when the sign in gate is the chosen banner + * Please talk to @guardian/commercial-dev before changing this logic + */ useEffect(() => { - if (hasPickMessageCompleted && SelectedBanner == null) { + if (pickMessageResult?.type === 'NoMessageSelected') { document.dispatchEvent(new CustomEvent('banner:none')); } - }, [SelectedBanner, hasPickMessageCompleted]); - if (SelectedBanner) { - return ; + + if ( + pickMessageResult?.type === 'MessageSelected' && + pickMessageResult.messageId === 'sign-in-gate-portal' + ) { + document.dispatchEvent(new CustomEvent('banner:sign-in-gate')); + } + }, [pickMessageResult]); + + if (pickMessageResult?.type === 'MessageSelected') { + const { SelectedMessage } = pickMessageResult; + return ; } return null; diff --git a/dotcom-rendering/src/lib/messagePicker.test.tsx b/dotcom-rendering/src/lib/messagePicker.test.tsx index 1a2b2d74fb3..d3128ae8efd 100644 --- a/dotcom-rendering/src/lib/messagePicker.test.tsx +++ b/dotcom-rendering/src/lib/messagePicker.test.tsx @@ -23,7 +23,7 @@ afterEach(async () => { describe('pickMessage', () => { it('resolves with the highest priority message which can show', async () => { const MockComponent = () =>
; - const ChosenMockComponent = () =>
; + const ChosenMockComponent = () =>
; const config: SlotConfig = { name: 'banner', candidates: [ @@ -31,7 +31,7 @@ describe('pickMessage', () => { candidate: { id: 'banner-1', canShow: () => Promise.resolve({ show: false }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, }, @@ -40,7 +40,7 @@ describe('pickMessage', () => { id: 'banner-2', canShow: () => Promise.resolve({ show: true, meta: undefined }), - show: () => ChosenMockComponent, + show: ChosenMockComponent, }, timeoutMillis: null, }, @@ -49,16 +49,20 @@ describe('pickMessage', () => { id: 'banner-3', canShow: () => Promise.resolve({ show: true, meta: undefined }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, }, ], }; - const got = await pickMessage(config, 'Web'); - - expect(got()).toEqual(ChosenMockComponent); + const pickMessageResult = await pickMessage(config, 'Web'); + expect(pickMessageResult.type).toEqual('MessageSelected'); + if (pickMessageResult.type === 'MessageSelected') { + expect(pickMessageResult.SelectedMessage()).toEqual( + ChosenMockComponent(), + ); + } }); it('resolves with null if no messages can show', async () => { @@ -71,7 +75,7 @@ describe('pickMessage', () => { candidate: { id: 'banner-1', canShow: () => Promise.resolve({ show: false }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, }, @@ -79,21 +83,21 @@ describe('pickMessage', () => { candidate: { id: 'banner-2', canShow: () => Promise.resolve({ show: false }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, }, ], }; - const got = await pickMessage(config, 'Web'); + const pickMessageResult = await pickMessage(config, 'Web'); - expect(got()).toEqual(null); + expect(pickMessageResult.type).toEqual('NoMessageSelected'); }); it('falls through to a lower priority message when a higher one times out', async () => { const MockComponent = () =>
; - const ChosenMockComponent = () =>
; + const ChosenMockComponent = () =>
; const config: SlotConfig = { name: 'banner', candidates: [ @@ -111,7 +115,7 @@ describe('pickMessage', () => { 500, ), ), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: 250, }, @@ -120,7 +124,7 @@ describe('pickMessage', () => { id: 'banner-2', canShow: () => Promise.resolve({ show: true, meta: undefined }), - show: () => ChosenMockComponent, + show: ChosenMockComponent, }, timeoutMillis: null, }, @@ -129,9 +133,14 @@ describe('pickMessage', () => { const messagePromise = pickMessage(config, 'Web'); jest.advanceTimersByTime(260); - const got = await messagePromise; - expect(got()).toEqual(ChosenMockComponent); + const pickMessageResult = await messagePromise; + expect(pickMessageResult.type).toEqual('MessageSelected'); + if (pickMessageResult.type === 'MessageSelected') { + expect(pickMessageResult.SelectedMessage()).toEqual( + ChosenMockComponent(), + ); + } }); it('resolves with null if all messages time out', async () => { @@ -155,7 +164,7 @@ describe('pickMessage', () => { 500, ); }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: 250, }, @@ -173,7 +182,7 @@ describe('pickMessage', () => { 500, ); }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: 250, }, @@ -184,14 +193,14 @@ describe('pickMessage', () => { jest.advanceTimersByTime(260); const got = await messagePromise; - expect(got()).toEqual(null); + expect(got.type).toEqual('NoMessageSelected'); clearTimeout(timer1); clearTimeout(timer2); }); it('passes metadata returned by canShow to show', async () => { - const renderComponent = jest.fn(() => () =>
); + const renderComponent = jest.fn(() =>
); const meta = { extra: 'info' }; const config: SlotConfig = { name: 'banner', @@ -212,7 +221,10 @@ describe('pickMessage', () => { }; const show = await pickMessage(config, 'Web'); - show(); + expect(show.type).toEqual('MessageSelected'); + if (show.type === 'MessageSelected') { + expect(show.SelectedMessage()).toEqual(renderComponent()); + } expect(renderComponent).toHaveBeenCalledWith(meta); }); @@ -237,7 +249,7 @@ describe('pickMessage', () => { 300, ); }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: 200, }, @@ -248,7 +260,7 @@ describe('pickMessage', () => { jest.advanceTimersByTime(250); const got = await messagePromise; - expect(got()).toEqual(null); + expect(got.type).toEqual('NoMessageSelected'); expect(ophanRecordSpy).toHaveBeenCalledWith({ component: 'banner-picker-timeout-dcr', @@ -278,7 +290,7 @@ describe('pickMessage', () => { 120, ); }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, reportTiming: true, @@ -297,7 +309,7 @@ describe('pickMessage', () => { 100, ); }), - show: () => MockComponent, + show: MockComponent, }, timeoutMillis: null, }, diff --git a/dotcom-rendering/src/lib/messagePicker.ts b/dotcom-rendering/src/lib/messagePicker.ts index f07ff1fd4a5..10b7ea9bfaa 100644 --- a/dotcom-rendering/src/lib/messagePicker.ts +++ b/dotcom-rendering/src/lib/messagePicker.ts @@ -2,7 +2,7 @@ import { isUndefined, startPerformanceMeasure } from '@guardian/libs'; import { getOphan } from '../client/ophan/ophan'; import type { RenderingTarget } from '../types/renderingTarget'; -export type MaybeFC = React.FC | null; +export type MaybeFC = React.ReactNode | null; type ShowMessage = (meta: T) => MaybeFC; interface ShouldShow { @@ -114,8 +114,6 @@ const timeoutify = ( const clearAllTimeouts = (messages: CandidateConfigWithTimeout[]) => messages.map((m) => m.cancelTimeout()); -const defaultShow = () => null; - interface PendingMessage { candidateConfig: CandidateConfigWithTimeout; canShow: Promise>; @@ -126,10 +124,21 @@ interface WinningMessage { candidate: Candidate; } +interface NoMessageSelected { + type: 'NoMessageSelected'; +} +interface MessageSelected { + type: 'MessageSelected'; + messageId: string; + // The react component is optional because sometimes we selected a message for this slot, but there is nothing to render + SelectedMessage: () => MaybeFC; +} +export type PickMessageResult = NoMessageSelected | MessageSelected; + export const pickMessage = ( { candidates, name }: SlotConfig, renderingTarget: RenderingTarget, -): Promise<() => MaybeFC> => +): Promise => new Promise((resolve) => { const candidateConfigsWithTimeout = candidates.map((c) => timeoutify(c, name, renderingTarget), @@ -165,13 +174,18 @@ export const pickMessage = ( clearAllTimeouts(candidateConfigsWithTimeout); if (winner === null) { - resolve(defaultShow); + resolve({ type: 'NoMessageSelected' }); } else { const { candidate, meta } = winner; - resolve(() => candidate.show(meta)); + resolve({ + type: 'MessageSelected', + messageId: candidate.id, + SelectedMessage: () => candidate.show(meta), + }); } }) - .catch((e) => - console.error(`pickMessage winner - error: ${String(e)}`), - ); + .catch((e) => { + console.error(`pickMessage winner - error: ${String(e)}`); + resolve({ type: 'NoMessageSelected' }); + }); });