From 1497feb107b67065ad4144cdf9614b315d822f1d Mon Sep 17 00:00:00 2001 From: andresilva-guardian Date: Wed, 16 Sep 2026 22:38:47 +0100 Subject: [PATCH 01/25] Refactor PuzzlePageLayout to mirror CrosswordLayout Restructure PuzzlePageLayout to use the same Article-domain components (ArticleTitle, ArticleHeadline, ArticleMeta, SubMeta, DiscussionLayout, StickyBottomBanner) and grid structure as CrosswordLayout, while keeping Puzzle-specific content. The puzzleGroup label is now rendered as a real section link through ArticleTitle instead of plain text. Update design format from Standard to Crossword to reuse existing width-handling logic. Add proper ad slot handling (header, survey, merchandising) and comments section (hardcoded off). Extensive comments document hardcoded fallback values for fields not present in FEPuzzlePageType. --- .../src/layouts/PuzzlePageLayout.test.tsx | 7 +- .../src/layouts/PuzzlePageLayout.tsx | 592 +++++++++++++----- 2 files changed, 442 insertions(+), 157 deletions(-) diff --git a/dotcom-rendering/src/layouts/PuzzlePageLayout.test.tsx b/dotcom-rendering/src/layouts/PuzzlePageLayout.test.tsx index c3b45e13ac8..1bef3872987 100644 --- a/dotcom-rendering/src/layouts/PuzzlePageLayout.test.tsx +++ b/dotcom-rendering/src/layouts/PuzzlePageLayout.test.tsx @@ -76,13 +76,12 @@ describe('PuzzlePageLayout', () => { expect(screen.queryByText('11 September 2026')).not.toBeInTheDocument(); }); - it('renders the puzzleGroup label as plain, non-linked text', () => { + it('renders the puzzleGroup label as a real section link, exactly like CrosswordLayout/ArticleTitle', () => { renderPuzzlePageLayout('sudoku-easy'); expect( - screen.queryByRole('link', { name: 'Logic puzzles' }), - ).not.toBeInTheDocument(); - expect(screen.getByText('Logic puzzles')).toBeInTheDocument(); + screen.getByRole('link', { name: 'Logic puzzles' }), + ).toBeInTheDocument(); }); describe('print button (Sudoku-only, per PR #16700 review)', () => { diff --git a/dotcom-rendering/src/layouts/PuzzlePageLayout.tsx b/dotcom-rendering/src/layouts/PuzzlePageLayout.tsx index b501aae027e..107933ac40c 100644 --- a/dotcom-rendering/src/layouts/PuzzlePageLayout.tsx +++ b/dotcom-rendering/src/layouts/PuzzlePageLayout.tsx @@ -1,43 +1,71 @@ import { css } from '@emotion/react'; import { from, + remSpace, palette as sourcePalette, - until, } from '@guardian/source/foundations'; +import { Hide } from '@guardian/source/react-components'; import { StraightLines } from '@guardian/source-development-kitchen/react-components'; import { AdSlot, MobileStickyContainer } from '../components/AdSlot.web'; +import { ArticleContainer } from '../components/ArticleContainer'; +import { ArticleHeadline } from '../components/ArticleHeadline'; +import { ArticleMeta } from '../components/ArticleMeta.web'; +import { ArticleTitle } from '../components/ArticleTitle'; +import { DecideLines } from '../components/DecideLines'; +import { DiscussionLayout } from '../components/DiscussionLayout'; import { Footer } from '../components/Footer'; import { GridItem } from '../components/GridItem'; import { HeaderAdSlot } from '../components/HeaderAdSlot'; import { Island } from '../components/Island'; import { Masthead } from '../components/Masthead/Masthead'; import { PuzzleIframe } from '../components/PuzzleIframe.island'; +import { RightColumn } from '../components/RightColumn'; import { Section } from '../components/Section'; -import { ShareButton } from '../components/ShareButton.island'; +import { Standfirst } from '../components/Standfirst'; +import { StickyBottomBanner } from '../components/StickyBottomBanner.island'; +import { SubMeta } from '../components/SubMeta'; import { SubNav } from '../components/SubNav.island'; import { ArticleDesign, ArticleDisplay, Pillar } from '../lib/articleFormat'; +import { shouldShowMobileAboveNavSlot } from '../lib/commercialMobileAboveNavTest'; import { formatPuzzleDate } from '../lib/puzzleDate'; import { isPuzzlesHubV1Enabled } from '../lib/puzzlesHubVersionExperiment'; import type { NavType } from '../model/extract-nav'; import type { PuzzleConfig } from '../model/puzzles/puzzleConfigs'; import { palette as themePalette } from '../palette'; import type { FEPuzzlePageType } from '../types/puzzlePage'; +import { BannerWrapper, Stuck } from './lib/stickiness'; /** - * A fresh, self-contained layout for generic Puzzle Pages. It intentionally - * does not reuse Article-domain composite components (`ArticleMeta`, - * `ArticleTitle`, `ArticleBody`) as those require a full `ArticleFormat` + - * `TagType[]` + branding/podcast/avatar machinery that doesn't apply to a - * generic puzzle page. It does directly reuse existing generic building - * blocks (Masthead, Section, Footer, AdSlot, ShareButton.island) rather than - * duplicating them. + * A deliberate, near-verbatim structural replica of `CrosswordLayout.tsx` + * (sticky header ad, Masthead, survey ad, an `
` grid with + * title/headline/standfirst/meta rows and a `right-column` ad on wide + * viewports, a straight-line separator, SubMeta, merchandising ads, + * comments, sub-nav, footer, the contributions banner and the mobile + * sticky container), reusing the exact same generic + Article-domain + * components (`ArticleTitle`, `ArticleHeadline`, `ArticleMeta`, `SubMeta`, + * `DiscussionLayout`, `StickyBottomBanner`) that page uses, not + * Puzzle-Page-specific rewrites of them - see `CrosswordLayout.tsx` for the + * original this mirrors. * - * Puzzle Page is scoped to iframe-based puzzles only, crosswords remain on - * their existing, separate `/crosswords/*` flow - * (`ArticleDesign.Crossword` / `src/layouts/CrosswordLayout.tsx`), which is - * unrelated to this layout. There is accordingly no setter byline, PDF - * link, or comments rendering here, none of the current `PuzzleConfig` - * registry entries have any equivalent concept. + * The **one deliberate content difference**, per explicit product/design + * direction: the crossword game itself (`ArticleBody` rendering the + * `crossword` block) is replaced by `PuzzleIframe.island` in the `body` + * grid area. Puzzle Page is scoped to iframe-based puzzles only; the + * crossword-only game/data model has no equivalent here. + * + * Everywhere else, `FEPuzzlePageType` (`src/types/puzzlePage.ts`) simply + * doesn't carry the same fields `ArticleDeprecated` does (no `tags`, + * `byline`, `crossword`, `blocks`, `isCommentable`, `subMetaKeywordLinks`/ + * `subMetaSectionLinks`, `pageType`, `isAdFreeUser`, `shouldHideAds`, + * `guardianBaseURL`, etc. - see `docs/puzzle-page.md`'s "The + * `FEPuzzlePageType` request contract"). Rather than dropping those + * components, each is still rendered with the closest real data Puzzle + * Page actually has, and a **hardcoded, explicitly-commented fallback** + * (an empty array, `undefined`, or a `false` gate) everywhere a genuine + * Puzzle-Page equivalent doesn't exist - e.g. comments are permanently + * disabled below via `const showComments = false`, mirroring exactly how + * `CrosswordLayout` itself gates its own `showComments` Section, just with + * a fixed value instead of a derived one. */ const puzzleGroupLabels: Record = { @@ -46,58 +74,102 @@ const puzzleGroupLabels: Record = { }; /** - * `ShareButton.island` only needs an `ArticleFormat` to branch a handful of - * minor style decisions (e.g. LiveBlog-specific spacing). Puzzle pages have - * no equivalent concept, so a minimal, fixed format value is used to satisfy - * its prop contract without fabricating article-specific data (tags, - * branding, etc.). This is read-only reuse of existing exported enum - * values, it does not modify `articleFormat.ts` or any crossword decision - * logic. + * Puzzle Page has no real equivalent of `ArticleDeprecated.guardianBaseURL` + * (see `docs/puzzle-page.md`'s contract table - it isn't part of + * `FEPuzzlePageType`). `ArticleTitle` only uses it to build the tag/section + * link's absolute href, so the real, stable production base URL is + * hardcoded here rather than leaving it blank. + */ +const GUARDIAN_BASE_URL = 'https://www.theguardian.com'; + +/** + * `ArticleTitle`/`ArticleHeadline`/`ArticleMeta`/`SubMeta`/`DiscussionLayout`/ + * `StickyBottomBanner` only need an `ArticleFormat` to branch a handful of + * style/behaviour decisions (e.g. LiveBlog-specific spacing, headline + * weight). Puzzle pages have no equivalent concept, so a minimal, fixed + * format value is used to satisfy those prop contracts without fabricating + * article-specific data (tags, branding, etc.). + * + * `design: ArticleDesign.Crossword` is deliberately reused (not + * `Standard`): `ArticleContainer`'s width switch already special-cases + * `Crossword` as "the player manages its own width" (no fixed `620px` + * desktop max-width) - exactly the behaviour `PuzzleIframe`'s own + * `width: 100%` styling needs too, so this reuses that existing, + * documented case rather than inventing a new one. */ const puzzlePageFormat = { display: ArticleDisplay.Standard, - design: ArticleDesign.Standard, + design: ArticleDesign.Crossword, theme: Pillar.News, } as const; -const headerGrid = css` - display: grid; - grid-template-columns: minmax(0, 1fr); - grid-template-areas: - 'label' - 'title' - 'meta' - 'body'; - row-gap: 8px; +/** + * Identical to `CrosswordLayout.tsx`'s own `CrosswordGrid`: the same grid + * areas (including `instructions`, which Puzzle Page never populates, see + * the `instructions` `GridItem` below), the same breakpoints, the same + * print stylesheet. + */ +const PuzzleGrid = ({ children }: { children: React.ReactNode }) => ( +
+ {children} +
+); -const puzzleDateStyles = css` - display: block; - color: ${themePalette('--sub-meta-text')}; - font-weight: 400; +const maxWidth = css` + ${from.desktop} { + max-width: 620px; + } `; -const metaRow = css` - display: flex; - align-items: center; - gap: 16px; - flex-wrap: wrap; +const stretchLines = css` + ${from.leftCol} { + margin-left: 0; + } `; const printButtonStyles = css` @@ -105,6 +177,7 @@ const printButtonStyles = css` border: 1px solid currentColor; border-radius: 100px; padding: 4px 12px; + margin: ${remSpace[2]}px 0; cursor: pointer; font-size: inherit; color: inherit; @@ -114,6 +187,14 @@ const printButtonStyles = css` } `; +/** + * Occupies the exact same `standfirst`-area slot `CrosswordLinks`' "PDF + * version" link uses in `CrosswordLayout`, but a plain print button + * instead: iframe-based puzzles have no `crossword.pdf` concept, but + * `PuzzleConfig.printEnabled` (Sudoku only, per explicit product decision - + * PR #16700 review, see `puzzleConfigs.ts`) is the real Puzzle Page + * equivalent of "give the reader an offline/printable copy". + */ const PrintButton = () => (