From af6efb786dfc80e38e5ec9ebe78405065c86c93e Mon Sep 17 00:00:00 2001 From: Max Duval Date: Tue, 31 Oct 2023 10:42:10 +0000 Subject: [PATCH 1/2] test(LightboxHash): prove it works in SSR --- dotcom-rendering/src/components/Island.test.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dotcom-rendering/src/components/Island.test.tsx b/dotcom-rendering/src/components/Island.test.tsx index 6dfaed7d10e..28ccd6150c7 100644 --- a/dotcom-rendering/src/components/Island.test.tsx +++ b/dotcom-rendering/src/components/Island.test.tsx @@ -9,6 +9,7 @@ import { ConfigProvider } from './ConfigContext'; import { EnhancePinnedPost } from './EnhancePinnedPost.importable'; import { InteractiveSupportButton } from './InteractiveSupportButton.importable'; import { Island } from './Island'; +import { LightboxHash } from './LightboxHash.importable'; import { LiveBlogEpic } from './LiveBlogEpic.importable'; import { Liveness } from './Liveness.importable'; import { Metrics } from './Metrics.importable'; @@ -148,6 +149,10 @@ describe('Island: server-side rendering', () => { ).not.toThrow(); }); + test('LightboxHash', () => { + expect(() => renderToString()).not.toThrow(); + }); + test('Liveness', () => { expect(() => renderToString( From fdbab950b3ff3d5a90353bc91c5023387c5d9dbe Mon Sep 17 00:00:00 2001 From: Max Duval Date: Tue, 31 Oct 2023 10:42:21 +0000 Subject: [PATCH 2/2] refactor(LightboxJavascript): make it server safe --- .../src/components/Island.test.tsx | 15 ++++++ .../LightboxJavascript.importable.tsx | 48 +++++++++++-------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/dotcom-rendering/src/components/Island.test.tsx b/dotcom-rendering/src/components/Island.test.tsx index 28ccd6150c7..eafd451ff23 100644 --- a/dotcom-rendering/src/components/Island.test.tsx +++ b/dotcom-rendering/src/components/Island.test.tsx @@ -10,6 +10,7 @@ import { EnhancePinnedPost } from './EnhancePinnedPost.importable'; import { InteractiveSupportButton } from './InteractiveSupportButton.importable'; import { Island } from './Island'; import { LightboxHash } from './LightboxHash.importable'; +import { LightboxJavascript } from './LightboxJavascript.importable'; import { LiveBlogEpic } from './LiveBlogEpic.importable'; import { Liveness } from './Liveness.importable'; import { Metrics } from './Metrics.importable'; @@ -153,6 +154,20 @@ describe('Island: server-side rendering', () => { expect(() => renderToString()).not.toThrow(); }); + test('LightboxJavascript', () => { + expect(() => + renderToString( + , + ), + ).not.toThrow(); + }); test('Liveness', () => { expect(() => renderToString( diff --git a/dotcom-rendering/src/components/LightboxJavascript.importable.tsx b/dotcom-rendering/src/components/LightboxJavascript.importable.tsx index bc3cb672e99..8d06d877c60 100644 --- a/dotcom-rendering/src/components/LightboxJavascript.importable.tsx +++ b/dotcom-rendering/src/components/LightboxJavascript.importable.tsx @@ -1,6 +1,6 @@ -import { log, storage } from '@guardian/libs'; +import { isNonNullable, log, storage } from '@guardian/libs'; import libDebounce from 'lodash.debounce'; -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; import ReactDOM from 'react-dom'; import screenfull from 'screenfull'; import type { ImageForLightbox } from '../types/content'; @@ -490,9 +490,12 @@ export const LightboxJavascript = ({ format: ArticleFormat; images: ImageForLightbox[]; }) => { - const lightbox = document.querySelector('#gu-lightbox'); + const [root, setRoot] = useState(); + useEffect(() => { - if (!lightbox) return; + const lightbox = document.querySelector('#gu-lightbox'); + if (!isNonNullable(lightbox)) return; + /** * We only want to initialise the lightbox once so we check to see if it is already marked as ready */ @@ -501,25 +504,30 @@ export const LightboxJavascript = ({ return; } initialiseLightbox(lightbox); - }, [lightbox]); - /** - * Hydration has been requested so the first step is to render the list of images and put them into - * the DOM - * - * LightboxLayout provides a marker for where these images should go `ul#lightbox-images`. We look for - * this and then use createPortal to insert LightboxImages into this location. - * - * Why do we do this here, and not on the server? - * Because the size of the html generated by LightboxImages is very large (because the Picture element - * is so verbose) and we don't want every page view to have to download it, only those that are opening - * lightbox - */ - const imageRoot = lightbox?.querySelector('ul#lightbox-images'); - if (!imageRoot) return null; + /** + * Hydration has been requested so the first step is to render the list of images and put them into + * the DOM + * + * LightboxLayout provides a marker for where these images should go `ul#lightbox-images`. We look for + * this and then use createPortal to insert LightboxImages into this location. + * + * Why do we do this here, and not on the server? + * Because the size of the html generated by LightboxImages is very large (because the Picture element + * is so verbose) and we don't want every page view to have to download it, only those that are opening + * lightbox + */ + const ul = + lightbox.querySelector('ul#lightbox-images'); + + if (isNonNullable(ul)) setRoot(ul); + }, []); + + if (!root) return null; + log('dotcom', '💡 Generating HTML for lightbox images...'); return ReactDOM.createPortal( , - imageRoot, + root, ); };