From 6a0624f236a169f33011a46ce5366f6e0db781b8 Mon Sep 17 00:00:00 2001 From: Rhys Mills Date: Wed, 25 Sep 2024 09:04:01 +0100 Subject: [PATCH 01/22] Add foundation for multi-byline element, based on mini-profiles --- .../src/components/MultiByline.tsx | 88 ++++++++++++++ .../src/components/MultiBylineItem.tsx | 112 ++++++++++++++++++ dotcom-rendering/src/types/content.ts | 9 ++ 3 files changed, 209 insertions(+) create mode 100644 dotcom-rendering/src/components/MultiByline.tsx create mode 100644 dotcom-rendering/src/components/MultiBylineItem.tsx diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx new file mode 100644 index 00000000000..4b56ae8b68d --- /dev/null +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -0,0 +1,88 @@ +import { css } from '@emotion/react'; +import type { ArticleFormat } from '@guardian/libs'; +import { palette } from '@guardian/source/foundations'; +import type { EditionId } from '../lib/edition'; +import type { ArticleElementRenderer } from '../lib/renderElement'; +import type { ServerSideTests, Switches } from '../types/config'; +import type { MultiBylineItem, StarRating } from '../types/content'; +import { MultiBylineItem as MultiBylineItemComponent } from './MultiBylineItem'; + +interface MiniProfilesProps { + format: ArticleFormat; + ajaxUrl: string; + host?: string; + pageId: string; + isAdFreeUser: boolean; + isSensitive: boolean; + abTests: ServerSideTests; + switches: Switches; + editionId: EditionId; + hideCaption?: boolean; + starRating?: StarRating; + multiBylineItems: MultiBylineItem[]; + RenderArticleElement: ArticleElementRenderer; + /** + * Whether this is the last element in the article. If true, no separator will be rendered. + */ + isLastElement: boolean; +} + +const separatorStyles = css` + width: 140px; + margin: 8px 0 2px 0; + border-top: 1px solid ${palette.neutral[86]}; +`; + +export const MultiByline = ({ + multiBylineItems, + format, + ajaxUrl, + host, + pageId, + isAdFreeUser, + isSensitive, + switches, + abTests, + editionId, + hideCaption, + starRating, + RenderArticleElement, + isLastElement, +}: MiniProfilesProps) => { + return ( +
    + {multiBylineItems.map((multiBylineItem, index) => ( + // eslint-disable-next-line react/no-array-index-key -- Title should usually be identical, but in case it isn't, also use array index + + {multiBylineItem.body.map((element) => ( + // eslint-disable-next-line react/jsx-key -- The element array should remain consistent as it's derived from the order of elements in CAPI + + ))} + + ))} + {!isLastElement &&
    } +
+ ); +}; diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx new file mode 100644 index 00000000000..a3daf3dd7e2 --- /dev/null +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -0,0 +1,112 @@ +import { css } from '@emotion/react'; +import { type ArticleFormat } from '@guardian/libs'; +import { neutral, space, textSans14 } from '@guardian/source/foundations'; +import sanitise from 'sanitize-html'; +import { slugify } from '../model/enhance-H2s'; +import { palette } from '../palette'; +import type { MultiBylineItem as MultiBylineItemModel } from '../types/content'; +import { headingLineStyles } from './KeyTakeaway'; +import { subheadingStyles } from './Subheading'; + +const multiBylineItemStyles = css` + padding-top: 8px; +`; + +/** Nesting is necessary in the bio styles because we receive a string of html from the + * field. This can contain the following tags: + * Blocks: p, ul, li + * Inline: strong, em, a + */ +const bioStyles = css` + ${textSans14}; + padding: ${space[1]}px 0; + color: ${palette('--mini-profiles-text-subdued')}; + p { + margin-bottom: ${space[2]}px; + } + a { + color: ${palette('--link-kicker-text')}; + text-underline-offset: 3px; + } + a:not(:hover) { + text-decoration-color: ${neutral[86]}; + } + a:hover { + text-decoration: underline; + } + ul { + list-style: none; + margin: 0 0 ${space[2]}px; + padding: 0; + } + ul li { + padding-left: ${space[5]}px; + } + ul li p { + display: inline-block; + margin-bottom: 0; + } + ul li:before { + display: inline-block; + content: ''; + border-radius: 0.375rem; + height: 10px; + width: 10px; + margin: 0 ${space[2]}px 0 -${space[5]}px; + background-color: ${palette('--bullet-fill')}; + } + strong { + font-weight: bold; + } +`; + +const bottomBorderStyles = css` + border-top: 1px solid ${palette('--article-border')}; + margin-bottom: ${space[2]}px; +`; + +const headingMarginStyle = css` + margin-bottom: ${space[2]}px; +`; + +interface MultiBylineItemProps { + multiBylineItem: MultiBylineItemModel; + format: ArticleFormat; + children: React.ReactNode; +} + +export const MultiBylineItem = ({ + multiBylineItem, + format, + children, +}: MultiBylineItemProps) => { + return ( + <> +
  • +
    +

    + {multiBylineItem.title} +

    + + {children} +
    • + + ); +}; + +const Bio = ({ html }: { html?: string }) => { + if (!html) return null; + const sanitizedHtml = sanitise(html, {}); + return ( + <> +
    +
    + + ); +}; diff --git a/dotcom-rendering/src/types/content.ts b/dotcom-rendering/src/types/content.ts index d8305d55ba8..ddb25e0548f 100644 --- a/dotcom-rendering/src/types/content.ts +++ b/dotcom-rendering/src/types/content.ts @@ -333,6 +333,15 @@ export interface MiniProfile { endNote?: string; } +export interface MultiBylineItem { + title: string; + body: FEElement[]; + bio?: string; + endNote?: string; + imageOverrideUrl?: string; + contributors?: BlockContributor[]; +} + export interface KeyTakeawaysBlockElement { _type: 'model.dotcomrendering.pageElements.KeyTakeawaysBlockElement'; keyTakeaways: KeyTakeaway[]; From 2d23d41dcee31b923dc50341fa139014e133b049 Mon Sep 17 00:00:00 2001 From: Rhys Mills Date: Wed, 25 Sep 2024 09:16:42 +0100 Subject: [PATCH 02/22] Add Story for multi byline element and update model to include byline and bylineHtml --- .../src/components/MultiByline.stories.tsx | 233 ++++++++++++++++++ dotcom-rendering/src/types/content.ts | 2 + 2 files changed, 235 insertions(+) create mode 100644 dotcom-rendering/src/components/MultiByline.stories.tsx diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx new file mode 100644 index 00000000000..239d7f2e88f --- /dev/null +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -0,0 +1,233 @@ +import { + ArticleDesign, + ArticleDisplay, + ArticleSpecial, + Pillar, +} from '@guardian/libs'; +import type { Meta, StoryObj } from '@storybook/react'; +import { centreColumnDecorator } from '../../.storybook/decorators/gridDecorators'; +import { allModes } from '../../.storybook/modes'; +import { images } from '../../fixtures/generated/images'; +import { getAllDesigns, getAllThemes } from '../lib/format'; +import { RenderArticleElement } from '../lib/renderElement'; +import type { TextBlockElement } from '../types/content'; +import { MultiByline } from './MultiByline'; + +const meta = { + component: MultiByline, + title: 'Components/MultiByline', +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +const testTextElement: TextBlockElement = { + _type: 'model.dotcomrendering.pageElements.TextBlockElement', + elementId: 'test-text-element-id-1', + dropCap: 'on', // this should be overruled by multi byline which always sets forceDropCap="off" + html: '

    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesquepharetra libero nec varius feugiat. Nulla commodo sagittis erat amalesuada. Ut iaculis interdum eros, et tristique ex. In veldignissim arcu. Nulla nisi urna, laoreet a aliquam at, viverra eueros. Proin imperdiet pellentesque turpis sed luctus. Donecdignissim lacus in risus fermentum maximus eu vel justo. Duis nontortor ac elit dapibus imperdiet ut at risus. Etiam pretium, odioeget accumsan venenatis, tortor mi aliquet nisl, vel ullamcorperneque nulla vel elit. Etiam porta mauris nec sagittis luctus.

    ', +}; + +const testParagraph = + '

    Proin imperdiet pellentesque adipiscing turpis sed luctus. Donecdignissim lacus in risus fermentum maximus eu vel justo. Duis nontortor ac elit dapibus imperdiet ut at risus.

    '; +const testListHtml = + '

    • This is the first item in the list

    • The second item has a hyperlink.

    '; +const testBioText = testParagraph + testListHtml; + +export const ThemeVariations = { + args: { + multiBylineItems: [ + { + title: 'The first byline', + bio: testBioText, + body: [testTextElement], + byline: 'Richard Hillgrove Guardian Contributor', + bylineHtml: + "Richard Hillgrove", + contributors: [ + { + name: 'Richard Hillgrove', + imageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', + }, + ], + }, + { + title: 'The first byline', + bio: testBioText, + body: [testTextElement], + byline: 'Richard Hillgrove Guardian Contributor', + bylineHtml: + "Richard Hillgrove", + contributors: [ + { + name: 'Richard Hillgrove', + imageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', + }, + ], + imageOverrideUrl: + 'https://i.guim.co.uk/img/uploads/2024/09/17/Maurice_Casey.png?width=180&dpr=1&s=none', + }, + { + title: 'The second byline', + body: [testTextElement], + }, + ], + isLastElement: true, + /** + * This will be replaced by the `formats` parameter, but it's + * required by the type. + */ + format: { + design: ArticleDesign.Standard, + display: ArticleDisplay.Standard, + theme: Pillar.News, + }, + abTests: {}, + /** + * This is used for rich links. An empty string isn't technically valid, + * but there are no rich links in this example. + */ + ajaxUrl: '', + editionId: 'UK', + isAdFreeUser: false, + isSensitive: false, + pageId: 'testID', + switches: {}, + RenderArticleElement, + }, + decorators: [centreColumnDecorator], + parameters: { + formats: getAllThemes({ + design: ArticleDesign.Standard, + display: ArticleDisplay.Standard, + }), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const DesignVariations = { + args: ThemeVariations.args, + decorators: [centreColumnDecorator], + parameters: { + formats: getAllDesigns({ + theme: Pillar.News, + display: ArticleDisplay.Standard, + }), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const OtherVariations = { + args: ThemeVariations.args, + decorators: [centreColumnDecorator], + parameters: { + formats: [ + { + design: ArticleDesign.Obituary, + display: ArticleDisplay.Standard, + theme: Pillar.Lifestyle, + }, + { + design: ArticleDesign.Review, + display: ArticleDisplay.Standard, + theme: Pillar.Sport, + }, + { + design: ArticleDesign.Recipe, + display: ArticleDisplay.Immersive, + theme: Pillar.Lifestyle, + }, + { + design: ArticleDesign.Feature, + display: ArticleDisplay.Immersive, + theme: ArticleSpecial.SpecialReport, + }, + { + design: ArticleDesign.Feature, + display: ArticleDisplay.Immersive, + theme: ArticleSpecial.SpecialReportAlt, + }, + ], + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const Images = { + args: { + ...ThemeVariations.args, + format: { + design: ArticleDesign.Standard, + display: ArticleDisplay.Standard, + theme: Pillar.Culture, + }, + multiBylineItems: [ + { + title: 'The first byline', + bio: testBioText, + body: [ + testTextElement, + { ...images[0], displayCredit: true, role: 'inline' }, + { ...images[1], displayCredit: true, role: 'thumbnail' }, + testTextElement, + ], + byline: 'Richard Hillgrove Guardian Contributor', + bylineHtml: + "Richard Hillgrove", + contributors: [ + { + name: 'Richard Hillgrove', + imageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', + }, + ], + }, + { + title: 'The second byline', + body: [ + testTextElement, + { + ...images[2], + displayCredit: true, + data: { ...images[2].data, caption: 'Sunset' }, + }, + ], + }, + ], + }, + decorators: [centreColumnDecorator], + parameters: { + chromatic: { + modes: { + vertical: allModes.splitVertical, + }, + }, + }, +} satisfies Story; + +export const WithSeparatorLine = { + args: { + ...ThemeVariations.args, + isLastElement: false, + format: { + design: ArticleDesign.Standard, + display: ArticleDisplay.Standard, + theme: Pillar.Culture, + }, + }, + decorators: [centreColumnDecorator], +} satisfies Story; diff --git a/dotcom-rendering/src/types/content.ts b/dotcom-rendering/src/types/content.ts index ddb25e0548f..6098635c93a 100644 --- a/dotcom-rendering/src/types/content.ts +++ b/dotcom-rendering/src/types/content.ts @@ -340,6 +340,8 @@ export interface MultiBylineItem { endNote?: string; imageOverrideUrl?: string; contributors?: BlockContributor[]; + byline?: string; + bylineHtml?: string; } export interface KeyTakeawaysBlockElement { From 88667b1b9e2fb8df23f7c1b96010043f3bb99490 Mon Sep 17 00:00:00 2001 From: Rhys Mills Date: Wed, 25 Sep 2024 10:09:02 +0100 Subject: [PATCH 03/22] Show a byline in the multi-byline element --- .../src/components/MultiByline.tsx | 1 - .../src/components/MultiBylineItem.tsx | 58 +++++++++++++++++-- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 4b56ae8b68d..10fe5080ccd 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -52,7 +52,6 @@ export const MultiByline = ({ return (
      {multiBylineItems.map((multiBylineItem, index) => ( - // eslint-disable-next-line react/no-array-index-key -- Title should usually be identical, but in case it isn't, also use array index
    1. -

      - {multiBylineItem.title} -

      + {children}
      2. @@ -97,6 +98,51 @@ export const MultiBylineItem = ({ ); }; +type BylineProps = { + title: string; + bylineHtml: string; + byline: string; + imageOverrideUrl?: string; + contributors: BlockContributor[]; + format: ArticleFormat; +}; + +const Byline = ({ + title, + bylineHtml, + byline, + imageOverrideUrl, + contributors, + format, +}: BylineProps) => { + const sanitizedHtml = sanitise(bylineHtml, {}); + + return ( +
      +
      +

      + {title} +

      + {bylineHtml ? ( +

      + ) : null} +

      + {imageOverrideUrl ?? contributors[0]?.imageUrl ? ( + {byline} + ) : null} +
      + ); +}; + const Bio = ({ html }: { html?: string }) => { if (!html) return null; const sanitizedHtml = sanitise(html, {}); From 3f2589314ab9f2ddc42c7ba898cd6849c610a9da Mon Sep 17 00:00:00 2001 From: Rhys Mills Date: Wed, 25 Sep 2024 16:28:42 +0100 Subject: [PATCH 04/22] Improve styles on Multi byline element --- .../src/components/MultiByline.stories.tsx | 4 +- .../src/components/MultiBylineItem.tsx | 186 +++++++++++++++++- 2 files changed, 182 insertions(+), 8 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx index 239d7f2e88f..9da434f385e 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -42,9 +42,9 @@ export const ThemeVariations = { title: 'The first byline', bio: testBioText, body: [testTextElement], - byline: 'Richard Hillgrove Guardian Contributor', + byline: 'Richard Hillgrove Political Editor', bylineHtml: - "Richard Hillgrove", + "Richard Hillgrove Political Editor", contributors: [ { name: 'Richard Hillgrove', diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index c3f51cb52e6..2c06e9d5c37 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -1,17 +1,47 @@ import { css } from '@emotion/react'; -import { type ArticleFormat } from '@guardian/libs'; -import { neutral, space, textSans14 } from '@guardian/source/foundations'; +import { + ArticleDesign, + ArticleDisplay, + type ArticleFormat, + ArticleSpecial, +} from '@guardian/libs'; +import { + from, + headlineLightItalic24, + headlineLightItalic28, + headlineLightItalic34, + headlineMediumItalic24, + headlineMediumItalic28, + neutral, + space, + textSans14, + textSans24, + textSans28, + textSans34, + textSansItalic17, +} from '@guardian/source/foundations'; import sanitise from 'sanitize-html'; import { slugify } from '../model/enhance-H2s'; import { palette } from '../palette'; import type { MultiBylineItem as MultiBylineItemModel } from '../types/content'; -import { headingLineStyles } from './KeyTakeaway'; import { subheadingStyles } from './Subheading'; const multiBylineItemStyles = css` padding-top: 8px; `; +const labsBylineStyles = css` + ${textSansItalic17}; + line-height: 1.4; +`; + +const headingLineStyles = css` + width: 140px; + margin: 8px 0 2px 0; + border: none; + border-top: 4px solid ${palette('--heading-line')}; +`; + /** Nesting is necessary in the bio styles because we receive a string of html from the * field. This can contain the following tags: * Blocks: p, ul, li @@ -69,6 +99,144 @@ const headingMarginStyle = css` margin-bottom: ${space[2]}px; `; +export const nonAnchorHeadlineStyles = ({ + format, + fontWeight, +}: { + format: ArticleFormat; + fontWeight: 'light' | 'medium' | 'bold'; +}) => css` + ${format.display === ArticleDisplay.Immersive + ? headlineLightItalic28 + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${fontWeight === 'light' && headlineLightItalic24}; + ${fontWeight === 'medium' && headlineMediumItalic24}; + ${fontWeight === 'bold' && headlineLightItalic24}; + `}; + + ${from.tablet} { + ${format.display === ArticleDisplay.Immersive + ? headlineLightItalic34 + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${fontWeight === 'light' && headlineLightItalic28}; + ${fontWeight === 'medium' && headlineMediumItalic28}; + ${fontWeight === 'bold' && headlineLightItalic28}; + `}; + } + + /** Labs uses sans text */ + ${format.theme === ArticleSpecial.Labs && + css` + ${format.display === ArticleDisplay.Immersive + ? ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans28}; + font-weight: 300; + line-height: 1.15; + ` + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans24}; + ${fontWeight === 'light' && 'font-weight: 300;'}; + ${fontWeight === 'medium' && 'font-weight: 500;'}; + ${fontWeight === 'bold' && 'font-weight: 700;'}; + line-height: 1.15; + `}; + + ${from.tablet} { + ${format.display === ArticleDisplay.Immersive + ? ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans34}; + font-weight: 300; + line-height: 1.15; + ` + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans28}; + ${fontWeight === 'light' && 'font-weight: 300;'}; + ${fontWeight === 'medium' && 'font-weight: 500;'}; + ${fontWeight === 'bold' && 'font-weight: 700;'}; + line-height: 1.15; + `}; + } + `} + + color: ${palette('--subheading-text')}; + + /* We don't allow additional font weight inside h2 tags except for immersive articles */ + strong { + font-weight: ${format.display === ArticleDisplay.Immersive + ? 'bold' + : 'inherit'}; + } +`; + +export const multiBylineBylineStyles = (format: ArticleFormat) => css` + ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} + padding-bottom: 8px; + font-style: italic !important; + margin-top: -4px; + font-weight: 300 !important; + color: ${neutral[46]}; + a { + ${subheadingStyles(format)} + color: ${palette('--byline-anchor')}; + text-decoration: none; + font-style: normal; + :hover { + text-decoration: underline; + } + } +`; + +const bylineWrapperStyles = css` + display: flex; + width: 100%; + overflow: hidden; + border-bottom: 1px solid ${palette('--article-border')}; + margin-bottom: ${space[2]}px; +`; + +const bylineTextStyles = css` + flex-grow: 1; +`; + +const bylineImageStyles = css` + width: 140px; + border-radius: 50%; + margin-bottom: -16px; + height: 140px; + overflow: hidden; + align-self: flex-end; +`; + interface MultiBylineItemProps { multiBylineItem: MultiBylineItemModel; format: ArticleFormat; @@ -118,8 +286,8 @@ const Byline = ({ const sanitizedHtml = sanitise(bylineHtml, {}); return ( -
      -
      +
      +

      {bylineHtml ? (

      ) : null} @@ -137,6 +310,7 @@ const Byline = ({ {byline} ) : null}

      From 48839afd3fccd321f4e8824971bdcdab964ede1c Mon Sep 17 00:00:00 2001 From: Rhys Mills Date: Thu, 3 Oct 2024 14:40:30 +0100 Subject: [PATCH 05/22] Push pending changes --- .../src/components/MultiByline.stories.tsx | 8 ++++---- .../src/components/MultiBylineItem.tsx | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx index 9da434f385e..d088b8a768a 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -39,7 +39,7 @@ export const ThemeVariations = { args: { multiBylineItems: [ { - title: 'The first byline', + title: 'This subheading is quite long so is likely to run on to multiple lines', bio: testBioText, body: [testTextElement], byline: 'Richard Hillgrove Political Editor', @@ -54,10 +54,10 @@ export const ThemeVariations = { ], }, { - title: 'The first byline', + title: 'My hot take', bio: testBioText, body: [testTextElement], - byline: 'Richard Hillgrove Guardian Contributor', + byline: 'Guardian Contributor', bylineHtml: "Richard Hillgrove", contributors: [ @@ -71,7 +71,7 @@ export const ThemeVariations = { 'https://i.guim.co.uk/img/uploads/2024/09/17/Maurice_Casey.png?width=180&dpr=1&s=none', }, { - title: 'The second byline', + title: 'A further subheading', body: [testTextElement], }, ], diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index 2c06e9d5c37..d99d01e3e4f 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -207,7 +207,7 @@ export const multiBylineBylineStyles = (format: ArticleFormat) => css` color: ${neutral[46]}; a { ${subheadingStyles(format)} - color: ${palette('--byline-anchor')}; + color: ${palette('--link-kicker-text')}; text-decoration: none; font-style: normal; :hover { @@ -229,12 +229,20 @@ const bylineTextStyles = css` `; const bylineImageStyles = css` - width: 140px; + width: 80px; border-radius: 50%; - margin-bottom: -16px; - height: 140px; + margin-bottom: -8px; + height: 80px; + min-width: 80px; + // TODO: make this different size based on screen size overflow: hidden; align-self: flex-end; + ${from.tablet} { + height: 120px; + min-width: 120px; + width: 120px; + margin-bottom: -12px; + } `; interface MultiBylineItemProps { @@ -251,7 +259,6 @@ export const MultiBylineItem = ({ return ( <>
    2. -
      +

      Date: Mon, 28 Oct 2024 11:42:54 +0000 Subject: [PATCH 06/22] fix import errors following https://github.com/guardian/dotcom-rendering/pull/12461 and https://github.com/guardian/dotcom-rendering/pull/12566 --- .../src/components/MultiByline.stories.tsx | 13 ++++++------- dotcom-rendering/src/components/MultiByline.tsx | 2 +- dotcom-rendering/src/components/MultiBylineItem.tsx | 12 ++++++------ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx index d088b8a768a..eca2dd43217 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -1,14 +1,13 @@ -import { - ArticleDesign, - ArticleDisplay, - ArticleSpecial, - Pillar, -} from '@guardian/libs'; +import { ArticleDisplay, ArticleSpecial, Pillar } from '@guardian/libs'; import type { Meta, StoryObj } from '@storybook/react'; import { centreColumnDecorator } from '../../.storybook/decorators/gridDecorators'; import { allModes } from '../../.storybook/modes'; import { images } from '../../fixtures/generated/images'; -import { getAllDesigns, getAllThemes } from '../lib/format'; +import { + ArticleDesign, + getAllDesigns, + getAllThemes, +} from '../lib/articleFormat'; import { RenderArticleElement } from '../lib/renderElement'; import type { TextBlockElement } from '../types/content'; import { MultiByline } from './MultiByline'; diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 10fe5080ccd..fe544a757cb 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/react'; -import type { ArticleFormat } from '@guardian/libs'; import { palette } from '@guardian/source/foundations'; +import type { ArticleFormat } from '../lib/articleFormat'; import type { EditionId } from '../lib/edition'; import type { ArticleElementRenderer } from '../lib/renderElement'; import type { ServerSideTests, Switches } from '../types/config'; diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index d99d01e3e4f..6c21b22a213 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -1,10 +1,4 @@ import { css } from '@emotion/react'; -import { - ArticleDesign, - ArticleDisplay, - type ArticleFormat, - ArticleSpecial, -} from '@guardian/libs'; import { from, headlineLightItalic24, @@ -21,6 +15,12 @@ import { textSansItalic17, } from '@guardian/source/foundations'; import sanitise from 'sanitize-html'; +import { + ArticleDesign, + ArticleDisplay, + type ArticleFormat, + ArticleSpecial, +} from '../lib/articleFormat'; import { slugify } from '../model/enhance-H2s'; import { palette } from '../palette'; import type { MultiBylineItem as MultiBylineItemModel } from '../types/content'; From 2c8c91b46c4b74881b53ba2066b02195da7da0e5 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Mon, 28 Oct 2024 12:26:07 +0000 Subject: [PATCH 07/22] fix eslint errors --- .../src/components/MultiBylineItem.tsx | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index 6c21b22a213..75c244d2591 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -114,9 +114,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${fontWeight === 'light' && headlineLightItalic24}; - ${fontWeight === 'medium' && headlineMediumItalic24}; - ${fontWeight === 'bold' && headlineLightItalic24}; + ${fontWeight === 'medium' ? headlineMediumItalic24 : headlineLightItalic24}; `}; ${from.tablet} { @@ -128,9 +126,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${fontWeight === 'light' && headlineLightItalic28}; - ${fontWeight === 'medium' && headlineMediumItalic28}; - ${fontWeight === 'bold' && headlineLightItalic28}; + ${fontWeight === 'medium' ? headlineMediumItalic28 : headlineLightItalic28}; `}; } @@ -155,9 +151,13 @@ export const nonAnchorHeadlineStyles = ({ * Please speak to your team's designer and update this to use a more appropriate preset. */ ${textSans24}; - ${fontWeight === 'light' && 'font-weight: 300;'}; - ${fontWeight === 'medium' && 'font-weight: 500;'}; - ${fontWeight === 'bold' && 'font-weight: 700;'}; + ${ + fontWeight === 'light' + ? 'font-weight: 300;' + : fontWeight === 'medium' + ? 'font-weight: 500;' + : 'font-weight: 700;' + }; line-height: 1.15; `}; @@ -180,9 +180,13 @@ export const nonAnchorHeadlineStyles = ({ * Please speak to your team's designer and update this to use a more appropriate preset. */ ${textSans28}; - ${fontWeight === 'light' && 'font-weight: 300;'}; - ${fontWeight === 'medium' && 'font-weight: 500;'}; - ${fontWeight === 'bold' && 'font-weight: 700;'}; + ${ + fontWeight === 'light' + ? 'font-weight: 300;' + : fontWeight === 'medium' + ? 'font-weight: 500;' + : 'font-weight: 700;' + }; line-height: 1.15; `}; } From 2ec4ce4b1af0d0478cc85370b956c419919c2f7e Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Tue, 29 Oct 2024 09:54:07 +0000 Subject: [PATCH 08/22] fix stylelint errors --- dotcom-rendering/src/components/MultiBylineItem.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index 75c244d2591..462af4c9809 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -205,8 +205,10 @@ export const nonAnchorHeadlineStyles = ({ export const multiBylineBylineStyles = (format: ArticleFormat) => css` ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} padding-bottom: 8px; + /* stylelint-disable-next-line declaration-no-important */ font-style: italic !important; margin-top: -4px; + /* stylelint-disable-next-line declaration-no-important */ font-weight: 300 !important; color: ${neutral[46]}; a { @@ -232,13 +234,13 @@ const bylineTextStyles = css` flex-grow: 1; `; +// TODO: make this different size based on screen size const bylineImageStyles = css` width: 80px; border-radius: 50%; margin-bottom: -8px; height: 80px; min-width: 80px; - // TODO: make this different size based on screen size overflow: hidden; align-self: flex-end; ${from.tablet} { From dd6ce908ad25da0ad7e615b0ca278a804e96b839 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Wed, 30 Oct 2024 15:59:31 +0000 Subject: [PATCH 09/22] style contributor names that don't have a corresponding tag --- .../src/components/MultiByline.stories.tsx | 8 ++++++++ dotcom-rendering/src/components/MultiByline.tsx | 6 +++--- .../src/components/MultiBylineItem.tsx | 14 ++++++++++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx index eca2dd43217..ef90667d47b 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -73,6 +73,14 @@ export const ThemeVariations = { title: 'A further subheading', body: [testTextElement], }, + { + title: 'This byline has a contributor with no link', + bio: testBioText, + body: [testTextElement], + byline: 'Steve McQueen on Paul Gilroy', + bylineHtml: + "Steve McQueen on Paul Gilroy", + }, ], isLastElement: true, /** diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index fe544a757cb..e6a3bf449da 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -7,7 +7,7 @@ import type { ServerSideTests, Switches } from '../types/config'; import type { MultiBylineItem, StarRating } from '../types/content'; import { MultiBylineItem as MultiBylineItemComponent } from './MultiBylineItem'; -interface MiniProfilesProps { +interface MultiBylineProps { format: ArticleFormat; ajaxUrl: string; host?: string; @@ -48,14 +48,14 @@ export const MultiByline = ({ starRating, RenderArticleElement, isLastElement, -}: MiniProfilesProps) => { +}: MultiBylineProps) => { return (
        {multiBylineItems.map((multiBylineItem, index) => ( {multiBylineItem.body.map((element) => ( // eslint-disable-next-line react/jsx-key -- The element array should remain consistent as it's derived from the order of elements in CAPI diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index 462af4c9809..0d6243f054c 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -14,7 +14,7 @@ import { textSans34, textSansItalic17, } from '@guardian/source/foundations'; -import sanitise from 'sanitize-html'; +import sanitise, { defaults } from 'sanitize-html'; import { ArticleDesign, ArticleDisplay, @@ -220,6 +220,11 @@ export const multiBylineBylineStyles = (format: ArticleFormat) => css` text-decoration: underline; } } + span[data-contributor-rel='author'] { + ${subheadingStyles(format)} + color: ${neutral[46]}; + font-style: normal; + } `; const bylineWrapperStyles = css` @@ -296,7 +301,12 @@ const Byline = ({ contributors, format, }: BylineProps) => { - const sanitizedHtml = sanitise(bylineHtml, {}); + const sanitizedHtml = sanitise(bylineHtml, { + allowedAttributes: { + ...defaults.allowedAttributes, + span: ['data-contributor-rel'], + }, + }); return (
        From 0363e3e41125788e44a0b90c1a7f84ea02d89cc7 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:30:26 +0000 Subject: [PATCH 10/22] add stories for opinion and culture pillars --- .../src/components/MultiByline.stories.tsx | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiByline.stories.tsx index ef90667d47b..59094817160 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiByline.stories.tsx @@ -119,7 +119,7 @@ export const ThemeVariations = { }, } satisfies Story; -export const DesignVariations = { +export const DesignVariationsNews = { args: ThemeVariations.args, decorators: [centreColumnDecorator], parameters: { @@ -135,6 +135,38 @@ export const DesignVariations = { }, } satisfies Story; +export const DesignVariationsCulture = { + args: ThemeVariations.args, + decorators: [centreColumnDecorator], + parameters: { + formats: getAllDesigns({ + theme: Pillar.Culture, + display: ArticleDisplay.Standard, + }), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const DesignVariationsOpinion = { + args: ThemeVariations.args, + decorators: [centreColumnDecorator], + parameters: { + formats: getAllDesigns({ + theme: Pillar.Opinion, + display: ArticleDisplay.Standard, + }), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + export const OtherVariations = { args: ThemeVariations.args, decorators: [centreColumnDecorator], From b4e2a11920f7b8fe4a9d734abbf3466d98960fa1 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Thu, 31 Oct 2024 09:53:22 +0000 Subject: [PATCH 11/22] byline image styling --- dotcom-rendering/src/components/MultiBylineItem.tsx | 3 +++ dotcom-rendering/src/palette.ts | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx index 0d6243f054c..698d3b59a87 100644 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ b/dotcom-rendering/src/components/MultiBylineItem.tsx @@ -243,11 +243,13 @@ const bylineTextStyles = css` const bylineImageStyles = css` width: 80px; border-radius: 50%; + margin-left: 10px; margin-bottom: -8px; height: 80px; min-width: 80px; overflow: hidden; align-self: flex-end; + background-color: ${palette('--multi-byline-avatar-background')}; ${from.tablet} { height: 120px; min-width: 120px; @@ -275,6 +277,7 @@ export const MultiBylineItem = ({ byline={multiBylineItem.byline ?? ''} bylineHtml={multiBylineItem.bylineHtml ?? ''} contributors={multiBylineItem.contributors ?? []} + imageOverrideUrl={multiBylineItem.imageOverrideUrl} format={format} /> diff --git a/dotcom-rendering/src/palette.ts b/dotcom-rendering/src/palette.ts index d5b190c5510..9f36e979e20 100644 --- a/dotcom-rendering/src/palette.ts +++ b/dotcom-rendering/src/palette.ts @@ -4417,6 +4417,11 @@ const mostViewedTabBorderDark: PaletteFunction = ({ theme }) => { } }; +const multiBylineAvatarBackgroundLight: PaletteFunction = () => + sourcePalette.neutral[86]; +const multiBylineAvatarBackgroundDark: PaletteFunction = () => + sourcePalette.neutral[38]; + const mostViewedTabBorderLight: PaletteFunction = ({ theme }) => { switch (theme) { case Pillar.News: @@ -6695,6 +6700,10 @@ const paletteColours = { light: mostViewedTabBorderLight, dark: mostViewedTabBorderDark, }, + '--multi-byline-avatar-background': { + light: multiBylineAvatarBackgroundLight, + dark: multiBylineAvatarBackgroundDark, + }, '--nav-reader-revenue-link-text': { light: navReaderRevenueLinkText, dark: navReaderRevenueLinkText, From e14cc5c96f41996f5a0201fe0c824cad99100281 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Thu, 31 Oct 2024 12:08:11 +0000 Subject: [PATCH 12/22] add multi-bylines to DCR model --- .../src/components/MultiByline.tsx | 421 +++++++++++++++--- .../src/components/MultiBylineItem.tsx | 359 --------------- ...e.stories.tsx => MultiBylines.stories.tsx} | 36 +- .../src/components/MultiBylines.tsx | 90 ++++ dotcom-rendering/src/lib/renderElement.tsx | 19 +- .../src/model/article-schema.json | 81 +++- dotcom-rendering/src/model/block-schema.json | 81 +++- dotcom-rendering/src/model/enhanceLists.ts | 41 +- .../src/model/insertPromotedNewsletter.ts | 1 + dotcom-rendering/src/types/content.ts | 22 +- 10 files changed, 681 insertions(+), 470 deletions(-) delete mode 100644 dotcom-rendering/src/components/MultiBylineItem.tsx rename dotcom-rendering/src/components/{MultiByline.stories.tsx => MultiBylines.stories.tsx} (88%) create mode 100644 dotcom-rendering/src/components/MultiBylines.tsx diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index e6a3bf449da..15fda626c1d 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -1,87 +1,360 @@ import { css } from '@emotion/react'; -import { palette } from '@guardian/source/foundations'; -import type { ArticleFormat } from '../lib/articleFormat'; -import type { EditionId } from '../lib/edition'; -import type { ArticleElementRenderer } from '../lib/renderElement'; -import type { ServerSideTests, Switches } from '../types/config'; -import type { MultiBylineItem, StarRating } from '../types/content'; -import { MultiBylineItem as MultiBylineItemComponent } from './MultiBylineItem'; - -interface MultiBylineProps { - format: ArticleFormat; - ajaxUrl: string; - host?: string; - pageId: string; - isAdFreeUser: boolean; - isSensitive: boolean; - abTests: ServerSideTests; - switches: Switches; - editionId: EditionId; - hideCaption?: boolean; - starRating?: StarRating; - multiBylineItems: MultiBylineItem[]; - RenderArticleElement: ArticleElementRenderer; - /** - * Whether this is the last element in the article. If true, no separator will be rendered. - */ - isLastElement: boolean; -} +import { + from, + headlineLightItalic24, + headlineLightItalic28, + headlineLightItalic34, + headlineMediumItalic24, + headlineMediumItalic28, + neutral, + space, + textSans14, + textSans24, + textSans28, + textSans34, + textSansItalic17, +} from '@guardian/source/foundations'; +import sanitise, { defaults } from 'sanitize-html'; +import { + ArticleDesign, + ArticleDisplay, + type ArticleFormat, + ArticleSpecial, +} from '../lib/articleFormat'; +import { slugify } from '../model/enhance-H2s'; +import { palette } from '../palette'; +import type { MultiByline as MultiBylineModel } from '../types/content'; +import { subheadingStyles } from './Subheading'; + +const multiBylineItemStyles = css` + padding-top: 8px; +`; + +const labsBylineStyles = css` + ${textSansItalic17}; + line-height: 1.4; +`; -const separatorStyles = css` +const headingLineStyles = css` width: 140px; margin: 8px 0 2px 0; - border-top: 1px solid ${palette.neutral[86]}; + border: none; + border-top: 4px solid ${palette('--heading-line')}; +`; + +/** Nesting is necessary in the bio styles because we receive a string of html from the + * field. This can contain the following tags: + * Blocks: p, ul, li + * Inline: strong, em, a + */ +const bioStyles = css` + ${textSans14}; + padding: ${space[1]}px 0; + color: ${palette('--mini-profiles-text-subdued')}; + p { + margin-bottom: ${space[2]}px; + } + a { + color: ${palette('--link-kicker-text')}; + text-underline-offset: 3px; + } + a:not(:hover) { + text-decoration-color: ${neutral[86]}; + } + a:hover { + text-decoration: underline; + } + ul { + list-style: none; + margin: 0 0 ${space[2]}px; + padding: 0; + } + ul li { + padding-left: ${space[5]}px; + } + ul li p { + display: inline-block; + margin-bottom: 0; + } + ul li:before { + display: inline-block; + content: ''; + border-radius: 0.375rem; + height: 10px; + width: 10px; + margin: 0 ${space[2]}px 0 -${space[5]}px; + background-color: ${palette('--bullet-fill')}; + } + strong { + font-weight: bold; + } +`; + +const bottomBorderStyles = css` + border-top: 1px solid ${palette('--article-border')}; + margin-bottom: ${space[2]}px; +`; + +const headingMarginStyle = css` + margin-bottom: ${space[2]}px; +`; + +export const nonAnchorHeadlineStyles = ({ + format, + fontWeight, +}: { + format: ArticleFormat; + fontWeight: 'light' | 'medium' | 'bold'; +}) => css` + ${format.display === ArticleDisplay.Immersive + ? headlineLightItalic28 + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${fontWeight === 'medium' ? headlineMediumItalic24 : headlineLightItalic24}; + `}; + + ${from.tablet} { + ${format.display === ArticleDisplay.Immersive + ? headlineLightItalic34 + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${fontWeight === 'medium' ? headlineMediumItalic28 : headlineLightItalic28}; + `}; + } + + /** Labs uses sans text */ + ${format.theme === ArticleSpecial.Labs && + css` + ${format.display === ArticleDisplay.Immersive + ? ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans28}; + font-weight: 300; + line-height: 1.15; + ` + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans24}; + ${ + fontWeight === 'light' + ? 'font-weight: 300;' + : fontWeight === 'medium' + ? 'font-weight: 500;' + : 'font-weight: 700;' + }; + line-height: 1.15; + `}; + + ${from.tablet} { + ${format.display === ArticleDisplay.Immersive + ? ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans34}; + font-weight: 300; + line-height: 1.15; + ` + : ` + /** + * Typography preset styles should not be overridden. + * This has been done because the styles do not directly map to the new presets. + * Please speak to your team's designer and update this to use a more appropriate preset. + */ + ${textSans28}; + ${ + fontWeight === 'light' + ? 'font-weight: 300;' + : fontWeight === 'medium' + ? 'font-weight: 500;' + : 'font-weight: 700;' + }; + line-height: 1.15; + `}; + } + `} + + color: ${palette('--subheading-text')}; + + /* We don't allow additional font weight inside h2 tags except for immersive articles */ + strong { + font-weight: ${format.display === ArticleDisplay.Immersive + ? 'bold' + : 'inherit'}; + } +`; + +export const multiBylineBylineStyles = (format: ArticleFormat) => css` + ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} + padding-bottom: 8px; + /* stylelint-disable-next-line declaration-no-important */ + font-style: italic !important; + margin-top: -4px; + /* stylelint-disable-next-line declaration-no-important */ + font-weight: 300 !important; + color: ${neutral[46]}; + a { + ${subheadingStyles(format)} + color: ${palette('--link-kicker-text')}; + text-decoration: none; + font-style: normal; + :hover { + text-decoration: underline; + } + } + span[data-contributor-rel='author'] { + ${subheadingStyles(format)} + color: ${neutral[46]}; + font-style: normal; + } `; +const bylineWrapperStyles = css` + display: flex; + width: 100%; + overflow: hidden; + border-bottom: 1px solid ${palette('--article-border')}; + margin-bottom: ${space[2]}px; +`; + +const bylineTextStyles = css` + flex-grow: 1; +`; + +// TODO: make this different size based on screen size +const bylineImageStyles = css` + width: 80px; + border-radius: 50%; + margin-left: 10px; + margin-bottom: -8px; + height: 80px; + min-width: 80px; + overflow: hidden; + align-self: flex-end; + background-color: ${palette('--multi-byline-avatar-background')}; + ${from.tablet} { + height: 120px; + min-width: 120px; + width: 120px; + margin-bottom: -12px; + } +`; + +interface MultiBylineItemProps { + multiByline: MultiBylineModel; + format: ArticleFormat; + children: React.ReactNode; +} + export const MultiByline = ({ - multiBylineItems, + multiByline, format, - ajaxUrl, - host, - pageId, - isAdFreeUser, - isSensitive, - switches, - abTests, - editionId, - hideCaption, - starRating, - RenderArticleElement, - isLastElement, -}: MultiBylineProps) => { + children, +}: MultiBylineItemProps) => { return ( -
          - {multiBylineItems.map((multiBylineItem, index) => ( - +
        1. + + + {children} +
          2. + + ); +}; + +type BylineProps = { + title: string; + bylineHtml: string; + byline: string; + imageOverrideUrl?: string; + contributorIds: string[]; + format: ArticleFormat; +}; + +const Byline = ({ + title, + bylineHtml, + byline, + imageOverrideUrl, + contributorIds, + format, +}: BylineProps) => { + const sanitizedHtml = sanitise(bylineHtml, { + allowedAttributes: { + ...defaults.allowedAttributes, + span: ['data-contributor-rel'], + }, + }); + + return ( +
          +
          +
          +

          - {multiBylineItem.body.map((element) => ( - // eslint-disable-next-line react/jsx-key -- The element array should remain consistent as it's derived from the order of elements in CAPI - - ))} - - ))} - {!isLastElement &&
          } -

        + {title} +

      + {bylineHtml ? ( +

      + ) : null} +

      + {/* TODO: fetch image url from contributor tag */} + {imageOverrideUrl ?? contributorIds[0] ? ( + {byline} + ) : null} +
      3. + ); +}; + +const Bio = ({ html }: { html?: string }) => { + if (!html) return null; + const sanitizedHtml = sanitise(html, {}); + return ( + <> +
      +
      + ); }; diff --git a/dotcom-rendering/src/components/MultiBylineItem.tsx b/dotcom-rendering/src/components/MultiBylineItem.tsx deleted file mode 100644 index 698d3b59a87..00000000000 --- a/dotcom-rendering/src/components/MultiBylineItem.tsx +++ /dev/null @@ -1,359 +0,0 @@ -import { css } from '@emotion/react'; -import { - from, - headlineLightItalic24, - headlineLightItalic28, - headlineLightItalic34, - headlineMediumItalic24, - headlineMediumItalic28, - neutral, - space, - textSans14, - textSans24, - textSans28, - textSans34, - textSansItalic17, -} from '@guardian/source/foundations'; -import sanitise, { defaults } from 'sanitize-html'; -import { - ArticleDesign, - ArticleDisplay, - type ArticleFormat, - ArticleSpecial, -} from '../lib/articleFormat'; -import { slugify } from '../model/enhance-H2s'; -import { palette } from '../palette'; -import type { MultiBylineItem as MultiBylineItemModel } from '../types/content'; -import { subheadingStyles } from './Subheading'; - -const multiBylineItemStyles = css` - padding-top: 8px; -`; - -const labsBylineStyles = css` - ${textSansItalic17}; - line-height: 1.4; -`; - -const headingLineStyles = css` - width: 140px; - margin: 8px 0 2px 0; - border: none; - border-top: 4px solid ${palette('--heading-line')}; -`; - -/** Nesting is necessary in the bio styles because we receive a string of html from the - * field. This can contain the following tags: - * Blocks: p, ul, li - * Inline: strong, em, a - */ -const bioStyles = css` - ${textSans14}; - padding: ${space[1]}px 0; - color: ${palette('--mini-profiles-text-subdued')}; - p { - margin-bottom: ${space[2]}px; - } - a { - color: ${palette('--link-kicker-text')}; - text-underline-offset: 3px; - } - a:not(:hover) { - text-decoration-color: ${neutral[86]}; - } - a:hover { - text-decoration: underline; - } - ul { - list-style: none; - margin: 0 0 ${space[2]}px; - padding: 0; - } - ul li { - padding-left: ${space[5]}px; - } - ul li p { - display: inline-block; - margin-bottom: 0; - } - ul li:before { - display: inline-block; - content: ''; - border-radius: 0.375rem; - height: 10px; - width: 10px; - margin: 0 ${space[2]}px 0 -${space[5]}px; - background-color: ${palette('--bullet-fill')}; - } - strong { - font-weight: bold; - } -`; - -const bottomBorderStyles = css` - border-top: 1px solid ${palette('--article-border')}; - margin-bottom: ${space[2]}px; -`; - -const headingMarginStyle = css` - margin-bottom: ${space[2]}px; -`; - -export const nonAnchorHeadlineStyles = ({ - format, - fontWeight, -}: { - format: ArticleFormat; - fontWeight: 'light' | 'medium' | 'bold'; -}) => css` - ${format.display === ArticleDisplay.Immersive - ? headlineLightItalic28 - : ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${fontWeight === 'medium' ? headlineMediumItalic24 : headlineLightItalic24}; - `}; - - ${from.tablet} { - ${format.display === ArticleDisplay.Immersive - ? headlineLightItalic34 - : ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${fontWeight === 'medium' ? headlineMediumItalic28 : headlineLightItalic28}; - `}; - } - - /** Labs uses sans text */ - ${format.theme === ArticleSpecial.Labs && - css` - ${format.display === ArticleDisplay.Immersive - ? ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${textSans28}; - font-weight: 300; - line-height: 1.15; - ` - : ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${textSans24}; - ${ - fontWeight === 'light' - ? 'font-weight: 300;' - : fontWeight === 'medium' - ? 'font-weight: 500;' - : 'font-weight: 700;' - }; - line-height: 1.15; - `}; - - ${from.tablet} { - ${format.display === ArticleDisplay.Immersive - ? ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${textSans34}; - font-weight: 300; - line-height: 1.15; - ` - : ` - /** - * Typography preset styles should not be overridden. - * This has been done because the styles do not directly map to the new presets. - * Please speak to your team's designer and update this to use a more appropriate preset. - */ - ${textSans28}; - ${ - fontWeight === 'light' - ? 'font-weight: 300;' - : fontWeight === 'medium' - ? 'font-weight: 500;' - : 'font-weight: 700;' - }; - line-height: 1.15; - `}; - } - `} - - color: ${palette('--subheading-text')}; - - /* We don't allow additional font weight inside h2 tags except for immersive articles */ - strong { - font-weight: ${format.display === ArticleDisplay.Immersive - ? 'bold' - : 'inherit'}; - } -`; - -export const multiBylineBylineStyles = (format: ArticleFormat) => css` - ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} - padding-bottom: 8px; - /* stylelint-disable-next-line declaration-no-important */ - font-style: italic !important; - margin-top: -4px; - /* stylelint-disable-next-line declaration-no-important */ - font-weight: 300 !important; - color: ${neutral[46]}; - a { - ${subheadingStyles(format)} - color: ${palette('--link-kicker-text')}; - text-decoration: none; - font-style: normal; - :hover { - text-decoration: underline; - } - } - span[data-contributor-rel='author'] { - ${subheadingStyles(format)} - color: ${neutral[46]}; - font-style: normal; - } -`; - -const bylineWrapperStyles = css` - display: flex; - width: 100%; - overflow: hidden; - border-bottom: 1px solid ${palette('--article-border')}; - margin-bottom: ${space[2]}px; -`; - -const bylineTextStyles = css` - flex-grow: 1; -`; - -// TODO: make this different size based on screen size -const bylineImageStyles = css` - width: 80px; - border-radius: 50%; - margin-left: 10px; - margin-bottom: -8px; - height: 80px; - min-width: 80px; - overflow: hidden; - align-self: flex-end; - background-color: ${palette('--multi-byline-avatar-background')}; - ${from.tablet} { - height: 120px; - min-width: 120px; - width: 120px; - margin-bottom: -12px; - } -`; - -interface MultiBylineItemProps { - multiBylineItem: MultiBylineItemModel; - format: ArticleFormat; - children: React.ReactNode; -} - -export const MultiBylineItem = ({ - multiBylineItem, - format, - children, -}: MultiBylineItemProps) => { - return ( - <> -
    3. - - - {children} -
      4. - - ); -}; - -type BylineProps = { - title: string; - bylineHtml: string; - byline: string; - imageOverrideUrl?: string; - contributors: BlockContributor[]; - format: ArticleFormat; -}; - -const Byline = ({ - title, - bylineHtml, - byline, - imageOverrideUrl, - contributors, - format, -}: BylineProps) => { - const sanitizedHtml = sanitise(bylineHtml, { - allowedAttributes: { - ...defaults.allowedAttributes, - span: ['data-contributor-rel'], - }, - }); - - return ( -
      -
      -
      -

      - {title} -

      - {bylineHtml ? ( -

      - ) : null} -

      - {imageOverrideUrl ?? contributors[0]?.imageUrl ? ( - {byline} - ) : null} -
      - ); -}; - -const Bio = ({ html }: { html?: string }) => { - if (!html) return null; - const sanitizedHtml = sanitise(html, {}); - return ( - <> -
      -
      - - ); -}; diff --git a/dotcom-rendering/src/components/MultiByline.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx similarity index 88% rename from dotcom-rendering/src/components/MultiByline.stories.tsx rename to dotcom-rendering/src/components/MultiBylines.stories.tsx index 59094817160..53f265661d1 100644 --- a/dotcom-rendering/src/components/MultiByline.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -10,12 +10,12 @@ import { } from '../lib/articleFormat'; import { RenderArticleElement } from '../lib/renderElement'; import type { TextBlockElement } from '../types/content'; -import { MultiByline } from './MultiByline'; +import { MultiBylines } from './MultiBylines'; const meta = { - component: MultiByline, - title: 'Components/MultiByline', -} satisfies Meta; + component: MultiBylines, + title: 'Components/MultiBylines', +} satisfies Meta; export default meta; @@ -36,7 +36,7 @@ const testBioText = testParagraph + testListHtml; export const ThemeVariations = { args: { - multiBylineItems: [ + multiBylines: [ { title: 'This subheading is quite long so is likely to run on to multiple lines', bio: testBioText, @@ -44,13 +44,7 @@ export const ThemeVariations = { byline: 'Richard Hillgrove Political Editor', bylineHtml: "Richard Hillgrove Political Editor", - contributors: [ - { - name: 'Richard Hillgrove', - imageUrl: - 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', - }, - ], + contributorIds: ['profile/richard-hillgrove'], }, { title: 'My hot take', @@ -59,13 +53,7 @@ export const ThemeVariations = { byline: 'Guardian Contributor', bylineHtml: "Richard Hillgrove", - contributors: [ - { - name: 'Richard Hillgrove', - imageUrl: - 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', - }, - ], + contributorIds: ['profile/richard-hillgrove'], imageOverrideUrl: 'https://i.guim.co.uk/img/uploads/2024/09/17/Maurice_Casey.png?width=180&dpr=1&s=none', }, @@ -214,7 +202,7 @@ export const Images = { display: ArticleDisplay.Standard, theme: Pillar.Culture, }, - multiBylineItems: [ + multiBylines: [ { title: 'The first byline', bio: testBioText, @@ -227,13 +215,7 @@ export const Images = { byline: 'Richard Hillgrove Guardian Contributor', bylineHtml: "Richard Hillgrove", - contributors: [ - { - name: 'Richard Hillgrove', - imageUrl: - 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', - }, - ], + contributorIds: ['profile/richard-hillgrove'], }, { title: 'The second byline', diff --git a/dotcom-rendering/src/components/MultiBylines.tsx b/dotcom-rendering/src/components/MultiBylines.tsx new file mode 100644 index 00000000000..6f02c1fed25 --- /dev/null +++ b/dotcom-rendering/src/components/MultiBylines.tsx @@ -0,0 +1,90 @@ +import { css } from '@emotion/react'; +import { palette } from '@guardian/source/foundations'; +import type { ArticleFormat } from '../lib/articleFormat'; +import type { EditionId } from '../lib/edition'; +import type { ArticleElementRenderer } from '../lib/renderElement'; +import type { ServerSideTests, Switches } from '../types/config'; +import type { + MultiByline as MultiBylineModel, + StarRating, +} from '../types/content'; +import { MultiByline as MultiBylineItemComponent } from './MultiByline'; + +interface MultiBylineProps { + format: ArticleFormat; + ajaxUrl: string; + host?: string; + pageId: string; + isAdFreeUser: boolean; + isSensitive: boolean; + abTests: ServerSideTests; + switches: Switches; + editionId: EditionId; + hideCaption?: boolean; + starRating?: StarRating; + multiBylines: MultiBylineModel[]; + RenderArticleElement: ArticleElementRenderer; + /** + * Whether this is the last element in the article. If true, no separator will be rendered. + */ + isLastElement: boolean; +} + +const separatorStyles = css` + width: 140px; + margin: 8px 0 2px 0; + border-top: 1px solid ${palette.neutral[86]}; +`; + +export const MultiBylines = ({ + multiBylines, + format, + ajaxUrl, + host, + pageId, + isAdFreeUser, + isSensitive, + switches, + abTests, + editionId, + hideCaption, + starRating, + RenderArticleElement, + isLastElement, +}: MultiBylineProps) => { + return ( +
        + {multiBylines.map((multiBylineItem, index) => ( + + {multiBylineItem.body.map((element) => ( + // eslint-disable-next-line react/jsx-key -- The element array should remain consistent as it's derived from the order of elements in CAPI + + ))} + + ))} + {!isLastElement &&
        } +
      + ); +}; diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index be18547e755..329c7807293 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -31,6 +31,7 @@ import { KnowledgeQuizAtom } from '../components/KnowledgeQuizAtom.importable'; import { MainMediaEmbedBlockComponent } from '../components/MainMediaEmbedBlockComponent'; import { MapEmbedBlockComponent } from '../components/MapEmbedBlockComponent.importable'; import { MiniProfiles } from '../components/MiniProfiles'; +import { MultiBylines } from '../components/MultiBylines'; import { MultiImageBlockComponent } from '../components/MultiImageBlockComponent'; import { NumberedTitleBlockComponent } from '../components/NumberedTitleBlockComponent'; import { PersonalityQuizAtom } from '../components/PersonalityQuizAtom.importable'; @@ -64,11 +65,11 @@ import { interactiveLegacyFigureClasses, isInteractive, } from '../layouts/lib/interactiveLegacyStyling'; -import { getSharingUrls } from '../lib/sharing-urls'; import type { ServerSideTests, Switches } from '../types/config'; import type { FEElement, RoleType, StarRating } from '../types/content'; import { ArticleDesign, type ArticleFormat } from './articleFormat'; import type { EditionId } from './edition'; +import { getSharingUrls } from './sharing-urls'; type Props = { format: ArticleFormat; @@ -492,6 +493,22 @@ export const renderElement = ({ isLastElement={index === totalElements - 1} /> ); + case 'model.dotcomrendering.pageElements.MultiBylinesBlockElement': + return ( + + ); case 'model.dotcomrendering.pageElements.MultiImageBlockElement': return ( FEElement[]; @@ -60,6 +60,36 @@ const constructMiniProfile = return []; }; +const constructMultiByline = + (enhanceElements: ElementsEnhancer) => + ({ + title, + elements, + bio, + endNote, + imageOverrideUrl, + contributorIds, + byline, + bylineHtml, + }: ListItem) => { + // if the element is missing its title for any reason, we will skip it + if (!isUndefined(title) && title !== '') { + return [ + { + title, + bio, + endNote, + imageOverrideUrl, + contributorIds, + byline, + bylineHtml, + body: enhanceElements(elements), + }, + ]; + } + return []; + }; + const enhanceListBlockElement = ( element: ListBlockElement, elementsEnhancer: ElementsEnhancer, @@ -92,6 +122,15 @@ const enhanceListBlockElement = ( ), }, ]; + case 'MultiByline': + return [ + { + _type: 'model.dotcomrendering.pageElements.MultiBylinesBlockElement', + multiBylines: element.items.flatMap( + constructMultiByline(elementsEnhancer), + ), + }, + ]; /** * If it's an unsupported list element, ignore the structure * and return the body elements. diff --git a/dotcom-rendering/src/model/insertPromotedNewsletter.ts b/dotcom-rendering/src/model/insertPromotedNewsletter.ts index a9172888886..11598e1ddfd 100644 --- a/dotcom-rendering/src/model/insertPromotedNewsletter.ts +++ b/dotcom-rendering/src/model/insertPromotedNewsletter.ts @@ -94,6 +94,7 @@ const listElements = [ 'model.dotcomrendering.pageElements.KeyTakeawaysBlockElement', 'model.dotcomrendering.pageElements.QAndAExplainerBlockElement', 'model.dotcomrendering.pageElements.MiniProfilesBlockElement', + 'model.dotcomrendering.pageElements.MultiBylinesBlockElement', 'model.dotcomrendering.pageElements.DCRSectionedTimelineBlockElement', 'model.dotcomrendering.pageElements.DCRTimelineBlockElement', ] as const; diff --git a/dotcom-rendering/src/types/content.ts b/dotcom-rendering/src/types/content.ts index 6098635c93a..0a3d4a76c44 100644 --- a/dotcom-rendering/src/types/content.ts +++ b/dotcom-rendering/src/types/content.ts @@ -333,13 +333,13 @@ export interface MiniProfile { endNote?: string; } -export interface MultiBylineItem { +export interface MultiByline { title: string; body: FEElement[]; bio?: string; endNote?: string; imageOverrideUrl?: string; - contributors?: BlockContributor[]; + contributorIds?: string[]; byline?: string; bylineHtml?: string; } @@ -359,16 +359,29 @@ interface MiniProfilesBlockElement { miniProfiles: MiniProfile[]; } -interface ListItem { +interface MultiBylinesBlockElement { + _type: 'model.dotcomrendering.pageElements.MultiBylinesBlockElement'; + multiBylines: MultiByline[]; +} + +export interface ListItem { title?: string; elements: FEElement[]; bio?: string; endNote?: string; + imageOverrideUrl?: string; + contributorIds?: string[]; + byline?: string; + bylineHtml?: string; } export interface ListBlockElement { _type: 'model.dotcomrendering.pageElements.ListBlockElement'; - listElementType: 'KeyTakeaways' | 'QAndAExplainer' | 'MiniProfiles'; + listElementType: + | 'KeyTakeaways' + | 'QAndAExplainer' + | 'MiniProfiles' + | 'MultiByline'; items: ListItem[]; elementId: string; } @@ -759,6 +772,7 @@ export type FEElement = | MapBlockElement | MediaAtomBlockElement | MiniProfilesBlockElement + | MultiBylinesBlockElement | MultiImageBlockElement | NumberedTitleBlockElement | NewsletterSignupBlockElement From 25ee54ef10d1e042935f9b3a76f2eea86f41ec70 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Thu, 31 Oct 2024 12:40:08 +0000 Subject: [PATCH 13/22] retrieve image url from contributor tag --- .../src/components/MultiByline.tsx | 21 +++++++++++-------- .../src/components/MultiBylines.stories.tsx | 18 ++++++++++++++++ .../src/components/MultiBylines.tsx | 4 ++++ dotcom-rendering/src/lib/ArticleRenderer.tsx | 1 + dotcom-rendering/src/lib/renderElement.tsx | 7 +++++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 15fda626c1d..6c88efe96a1 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -24,6 +24,7 @@ import { import { slugify } from '../model/enhance-H2s'; import { palette } from '../palette'; import type { MultiByline as MultiBylineModel } from '../types/content'; +import type { TagType } from '../types/tag'; import { subheadingStyles } from './Subheading'; const multiBylineItemStyles = css` @@ -239,7 +240,6 @@ const bylineTextStyles = css` flex-grow: 1; `; -// TODO: make this different size based on screen size const bylineImageStyles = css` width: 80px; border-radius: 50%; @@ -261,12 +261,14 @@ const bylineImageStyles = css` interface MultiBylineItemProps { multiByline: MultiBylineModel; format: ArticleFormat; + tags: TagType[]; children: React.ReactNode; } export const MultiByline = ({ multiByline, format, + tags, children, }: MultiBylineItemProps) => { return ( @@ -279,6 +281,7 @@ export const MultiByline = ({ contributorIds={multiByline.contributorIds ?? []} imageOverrideUrl={multiByline.imageOverrideUrl} format={format} + tags={tags} /> {children} @@ -294,6 +297,7 @@ type BylineProps = { imageOverrideUrl?: string; contributorIds: string[]; format: ArticleFormat; + tags: TagType[]; }; const Byline = ({ @@ -303,6 +307,7 @@ const Byline = ({ imageOverrideUrl, contributorIds, format, + tags, }: BylineProps) => { const sanitizedHtml = sanitise(bylineHtml, { allowedAttributes: { @@ -310,6 +315,9 @@ const Byline = ({ span: ['data-contributor-rel'], }, }); + const imageUrl = + imageOverrideUrl ?? + tags.find((tag) => tag.id === contributorIds[0])?.bylineImageUrl; return (
      @@ -333,14 +341,9 @@ const Byline = ({ /> ) : null}
      - {/* TODO: fetch image url from contributor tag */} - {imageOverrideUrl ?? contributorIds[0] ? ( - {byline} - ) : null} + {!!imageUrl && ( + {byline} + )}
      ); }; diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index 53f265661d1..ed8e1ed6bce 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -70,6 +70,15 @@ export const ThemeVariations = { "Steve McQueen on Paul Gilroy", }, ], + tags: [ + { + title: 'Richard Hillgrove', + id: 'profile/richard-hillgrove', + type: 'contributor', + bylineImageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', + }, + ], isLastElement: true, /** * This will be replaced by the `formats` parameter, but it's @@ -229,6 +238,15 @@ export const Images = { ], }, ], + tags: [ + { + title: 'Richard Hillgrove', + id: 'profile/richard-hillgrove', + type: 'contributor', + bylineImageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', + }, + ], }, decorators: [centreColumnDecorator], parameters: { diff --git a/dotcom-rendering/src/components/MultiBylines.tsx b/dotcom-rendering/src/components/MultiBylines.tsx index 6f02c1fed25..52213760c96 100644 --- a/dotcom-rendering/src/components/MultiBylines.tsx +++ b/dotcom-rendering/src/components/MultiBylines.tsx @@ -8,6 +8,7 @@ import type { MultiByline as MultiBylineModel, StarRating, } from '../types/content'; +import type { TagType } from '../types/tag'; import { MultiByline as MultiBylineItemComponent } from './MultiByline'; interface MultiBylineProps { @@ -24,6 +25,7 @@ interface MultiBylineProps { starRating?: StarRating; multiBylines: MultiBylineModel[]; RenderArticleElement: ArticleElementRenderer; + tags: TagType[]; /** * Whether this is the last element in the article. If true, no separator will be rendered. */ @@ -50,6 +52,7 @@ export const MultiBylines = ({ hideCaption, starRating, RenderArticleElement, + tags, isLastElement, }: MultiBylineProps) => { return ( @@ -58,6 +61,7 @@ export const MultiBylines = ({ {multiBylineItem.body.map((element) => ( diff --git a/dotcom-rendering/src/lib/ArticleRenderer.tsx b/dotcom-rendering/src/lib/ArticleRenderer.tsx index 028cd0f0b5b..4d79799bb75 100644 --- a/dotcom-rendering/src/lib/ArticleRenderer.tsx +++ b/dotcom-rendering/src/lib/ArticleRenderer.tsx @@ -84,6 +84,7 @@ export const ArticleRenderer = ({ abTests={abTests} editionId={editionId} totalElements={length} + tags={tags} /> ); }); diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index 329c7807293..e7444a81f78 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -67,6 +67,7 @@ import { } from '../layouts/lib/interactiveLegacyStyling'; import type { ServerSideTests, Switches } from '../types/config'; import type { FEElement, RoleType, StarRating } from '../types/content'; +import type { TagType } from '../types/tag'; import { ArticleDesign, type ArticleFormat } from './articleFormat'; import type { EditionId } from './edition'; import { getSharingUrls } from './sharing-urls'; @@ -92,6 +93,7 @@ type Props = { isTimeline?: boolean; totalElements?: number; isListElement?: boolean; + tags?: TagType[]; }; // updateRole modifies the role of an element in a way appropriate for most @@ -151,6 +153,7 @@ export const renderElement = ({ isTimeline = false, totalElements = 0, isListElement = false, + tags = [], }: Props) => { const isBlog = format.design === ArticleDesign.LiveBlog || @@ -506,6 +509,7 @@ export const renderElement = ({ switches={switches} editionId={editionId} RenderArticleElement={RenderArticleElement} + tags={tags} isLastElement={index === totalElements - 1} /> ); @@ -710,6 +714,7 @@ export const renderElement = ({ host, isPinnedPost, starRating, + tags, })} format={format} /> @@ -908,6 +913,7 @@ export const RenderArticleElement = ({ isTimeline, totalElements, isListElement, + tags = [], }: Props) => { const withUpdatedRole = updateRole(element, format); @@ -932,6 +938,7 @@ export const RenderArticleElement = ({ isTimeline, totalElements, isListElement, + tags, }); const needsFigure = !bareElements.has(element._type); From 3fa6d96d142e8e1783df77b16efa2d13792b992a Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Thu, 31 Oct 2024 12:45:00 +0000 Subject: [PATCH 14/22] run 'make gen-schema' --- .../src/model/article-schema.json | 156 +++++++++--------- dotcom-rendering/src/model/block-schema.json | 156 +++++++++--------- 2 files changed, 156 insertions(+), 156 deletions(-) diff --git a/dotcom-rendering/src/model/article-schema.json b/dotcom-rendering/src/model/article-schema.json index 01b4caca3ed..c420d71dce0 100644 --- a/dotcom-rendering/src/model/article-schema.json +++ b/dotcom-rendering/src/model/article-schema.json @@ -587,9 +587,9 @@ { "$ref": "#/definitions/MiniProfilesBlockElement" }, - { - "$ref": "#/definitions/MultiBylinesBlockElement" - }, + { + "$ref": "#/definitions/MultiBylinesBlockElement" + }, { "$ref": "#/definitions/MultiImageBlockElement" }, @@ -2401,7 +2401,7 @@ "enum": [ "KeyTakeaways", "MiniProfiles", - "MultiByline", + "MultiByline", "QAndAExplainer" ], "type": "string" @@ -2441,21 +2441,21 @@ "endNote": { "type": "string" }, - "imageOverrideUrl": { - "type": "string" - }, - "contributorIds": { - "type": "array", - "items": { - "type": "string" - } - }, - "byline": { - "type": "string" - }, - "bylineHtml": { - "type": "string" - } + "imageOverrideUrl": { + "type": "string" + }, + "contributorIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "byline": { + "type": "string" + }, + "bylineHtml": { + "type": "string" + } }, "required": [ "elements" @@ -2607,64 +2607,64 @@ "title" ] }, - "MultiBylinesBlockElement": { - "type": "object", - "properties": { - "_type": { - "type": "string", - "const": "model.dotcomrendering.pageElements.MultiBylinesBlockElement" - }, - "multiBylines": { - "type": "array", - "items": { - "$ref": "#/definitions/MultiByline" - } - } - }, - "required": [ - "_type", - "multiBylines" - ] - }, - "MultiByline": { - "type": "object", - "properties": { - "title": { - "type": "string" - }, - "body": { - "type": "array", - "items": { - "$ref": "#/definitions/FEElement" - } - }, - "bio": { - "type": "string" - }, - "endNote": { - "type": "string" - }, - "imageOverrideUrl": { - "type": "string" - }, - "contributorIds": { - "type": "array", - "items": { - "type": "string" - } - }, - "byline": { - "type": "string" - }, - "bylineHtml": { - "type": "string" - } - }, - "required": [ - "body", - "title" - ] - }, + "MultiBylinesBlockElement": { + "type": "object", + "properties": { + "_type": { + "type": "string", + "const": "model.dotcomrendering.pageElements.MultiBylinesBlockElement" + }, + "multiBylines": { + "type": "array", + "items": { + "$ref": "#/definitions/MultiByline" + } + } + }, + "required": [ + "_type", + "multiBylines" + ] + }, + "MultiByline": { + "type": "object", + "properties": { + "title": { + "type": "string" + }, + "body": { + "type": "array", + "items": { + "$ref": "#/definitions/FEElement" + } + }, + "bio": { + "type": "string" + }, + "endNote": { + "type": "string" + }, + "imageOverrideUrl": { + "type": "string" + }, + "contributorIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "byline": { + "type": "string" + }, + "bylineHtml": { + "type": "string" + } + }, + "required": [ + "body", + "title" + ] + }, "MultiImageBlockElement": { "type": "object", "properties": { @@ -5244,4 +5244,4 @@ } }, "$schema": "http://json-schema.org/draft-07/schema#" -} +} \ No newline at end of file diff --git a/dotcom-rendering/src/model/block-schema.json b/dotcom-rendering/src/model/block-schema.json index cfc87897b32..789c8636e84 100644 --- a/dotcom-rendering/src/model/block-schema.json +++ b/dotcom-rendering/src/model/block-schema.json @@ -174,9 +174,9 @@ { "$ref": "#/definitions/MiniProfilesBlockElement" }, - { - "$ref": "#/definitions/MultiBylinesBlockElement" - }, + { + "$ref": "#/definitions/MultiBylinesBlockElement" + }, { "$ref": "#/definitions/MultiImageBlockElement" }, @@ -1988,7 +1988,7 @@ "enum": [ "KeyTakeaways", "MiniProfiles", - "MultiByline", + "MultiByline", "QAndAExplainer" ], "type": "string" @@ -2028,21 +2028,21 @@ "endNote": { "type": "string" }, - "imageOverrideUrl": { - "type": "string" - }, - "contributorIds": { - "type": "array", - "items": { - "type": "string" - } - }, - "byline": { - "type": "string" - }, - "bylineHtml": { - "type": "string" - } + "imageOverrideUrl": { + "type": "string" + }, + "contributorIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "byline": { + "type": "string" + }, + "bylineHtml": { + "type": "string" + } }, "required": [ "elements" @@ -2194,64 +2194,64 @@ "title" ] }, - "MultiBylinesBlockElement": { - "type": "object", - "properties": { - "_type": { - "type": "string", - "const": "model.dotcomrendering.pageElements.MultiBylinesBlockElement" - }, - "multiBylines": { - "type": "array", - "items": { - "$ref": "#/definitions/MultiByline" - } - } - }, - "required": [ - "_type", - "multiBylines" - ] - }, - "MultiByline": { - "type": "object", - "properties": { - "title": { - "type": "string" - }, - "body": { - "type": "array", - "items": { - "$ref": "#/definitions/FEElement" - } - }, - "bio": { - "type": "string" - }, - "endNote": { - "type": "string" - }, - "imageOverrideUrl": { - "type": "string" - }, - "contributorIds": { - "type": "array", - "items": { - "type": "string" - } - }, - "byline": { - "type": "string" - }, - "bylineHtml": { - "type": "string" - } - }, - "required": [ - "body", - "title" - ] - }, + "MultiBylinesBlockElement": { + "type": "object", + "properties": { + "_type": { + "type": "string", + "const": "model.dotcomrendering.pageElements.MultiBylinesBlockElement" + }, + "multiBylines": { + "type": "array", + "items": { + "$ref": "#/definitions/MultiByline" + } + } + }, + "required": [ + "_type", + "multiBylines" + ] + }, + "MultiByline": { + "type": "object", + "properties": { + "title": { + "type": "string" + }, + "body": { + "type": "array", + "items": { + "$ref": "#/definitions/FEElement" + } + }, + "bio": { + "type": "string" + }, + "endNote": { + "type": "string" + }, + "imageOverrideUrl": { + "type": "string" + }, + "contributorIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "byline": { + "type": "string" + }, + "bylineHtml": { + "type": "string" + } + }, + "required": [ + "body", + "title" + ] + }, "MultiImageBlockElement": { "type": "object", "properties": { @@ -3689,4 +3689,4 @@ } }, "$schema": "http://json-schema.org/draft-07/schema#" -} +} \ No newline at end of file From 44cc1d91385c9f2c7597c3844c59e0becff6b588 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:04:22 +0000 Subject: [PATCH 15/22] add end note to component --- .../src/components/MiniProfile.tsx | 2 +- .../src/components/MultiByline.tsx | 4 +++ .../src/components/MultiBylines.stories.tsx | 35 ++----------------- 3 files changed, 7 insertions(+), 34 deletions(-) diff --git a/dotcom-rendering/src/components/MiniProfile.tsx b/dotcom-rendering/src/components/MiniProfile.tsx index c0e6d842f07..978128ac629 100644 --- a/dotcom-rendering/src/components/MiniProfile.tsx +++ b/dotcom-rendering/src/components/MiniProfile.tsx @@ -120,7 +120,7 @@ const Bio = ({ html }: { html?: string }) => { ); }; -const EndNote = ({ text }: { text?: string }) => { +export const EndNote = ({ text }: { text?: string }) => { if (!text) return null; return (

      diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 6c88efe96a1..5197034ea91 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -26,6 +26,7 @@ import { palette } from '../palette'; import type { MultiByline as MultiBylineModel } from '../types/content'; import type { TagType } from '../types/tag'; import { subheadingStyles } from './Subheading'; +import { EndNote } from './MiniProfile'; const multiBylineItemStyles = css` padding-top: 8px; @@ -285,6 +286,9 @@ export const MultiByline = ({ /> {children} + {multiByline.endNote ? ( + + ) : null} ); diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index ed8e1ed6bce..d45decefc34 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -60,6 +60,7 @@ export const ThemeVariations = { { title: 'A further subheading', body: [testTextElement], + endNote: 'This is an end note.', }, { title: 'This byline has a contributor with no link', @@ -116,7 +117,7 @@ export const ThemeVariations = { }, } satisfies Story; -export const DesignVariationsNews = { +export const DesignVariations = { args: ThemeVariations.args, decorators: [centreColumnDecorator], parameters: { @@ -132,38 +133,6 @@ export const DesignVariationsNews = { }, } satisfies Story; -export const DesignVariationsCulture = { - args: ThemeVariations.args, - decorators: [centreColumnDecorator], - parameters: { - formats: getAllDesigns({ - theme: Pillar.Culture, - display: ArticleDisplay.Standard, - }), - chromatic: { - modes: { - horizontal: allModes.splitHorizontal, - }, - }, - }, -} satisfies Story; - -export const DesignVariationsOpinion = { - args: ThemeVariations.args, - decorators: [centreColumnDecorator], - parameters: { - formats: getAllDesigns({ - theme: Pillar.Opinion, - display: ArticleDisplay.Standard, - }), - chromatic: { - modes: { - horizontal: allModes.splitHorizontal, - }, - }, - }, -} satisfies Story; - export const OtherVariations = { args: ThemeVariations.args, decorators: [centreColumnDecorator], From 09025027618e89a7e21482d58f478e1eef9b4a9a Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:33:26 +0000 Subject: [PATCH 16/22] replace img tag with Avatar component --- dotcom-rendering/src/components/Avatar.tsx | 6 ++++-- .../src/components/MultiByline.tsx | 18 ++++++++++++++---- .../src/components/MultiBylines.stories.tsx | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/dotcom-rendering/src/components/Avatar.tsx b/dotcom-rendering/src/components/Avatar.tsx index a79614e0590..a9d325c0ed1 100644 --- a/dotcom-rendering/src/components/Avatar.tsx +++ b/dotcom-rendering/src/components/Avatar.tsx @@ -1,3 +1,4 @@ +import type { SerializedStyles } from '@emotion/react'; import { css } from '@emotion/react'; import { Fragment } from 'react'; import { getSourceImageUrl } from '../lib/getSourceImageUrl_temp_fix'; @@ -49,9 +50,10 @@ type Props = { src: string; alt: string; shape?: AvatarShape; + cssOverrides?: SerializedStyles; }; -export const Avatar = ({ src, alt, shape = 'round' }: Props) => { +export const Avatar = ({ src, alt, shape = 'round', cssOverrides }: Props) => { const sources = generateSources(getSourceImageUrl(src), [ { breakpoint: 320, width: 75 }, { breakpoint: 740, width: 140 }, @@ -70,7 +72,7 @@ export const Avatar = ({ src, alt, shape = 'round' }: Props) => { return ( {sources.map((source) => { return ( diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 5197034ea91..a40791ce983 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -25,8 +25,9 @@ import { slugify } from '../model/enhance-H2s'; import { palette } from '../palette'; import type { MultiByline as MultiBylineModel } from '../types/content'; import type { TagType } from '../types/tag'; -import { subheadingStyles } from './Subheading'; +import { Avatar } from './Avatar'; import { EndNote } from './MiniProfile'; +import { subheadingStyles } from './Subheading'; const multiBylineItemStyles = css` padding-top: 8px; @@ -250,7 +251,6 @@ const bylineImageStyles = css` min-width: 80px; overflow: hidden; align-self: flex-end; - background-color: ${palette('--multi-byline-avatar-background')}; ${from.tablet} { height: 120px; min-width: 120px; @@ -259,6 +259,10 @@ const bylineImageStyles = css` } `; +const avatarStyles = css` + background-color: ${palette('--multi-byline-avatar-background')}; +`; + interface MultiBylineItemProps { multiByline: MultiBylineModel; format: ArticleFormat; @@ -321,7 +325,7 @@ const Byline = ({ }); const imageUrl = imageOverrideUrl ?? - tags.find((tag) => tag.id === contributorIds[0])?.bylineImageUrl; + tags.find((tag) => tag.id === contributorIds[0])?.bylineLargeImageUrl; return (

      @@ -346,7 +350,13 @@ const Byline = ({ ) : null}
      {!!imageUrl && ( - {byline} +
      + +
      )}
      ); diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index d45decefc34..459a9621aed 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -76,7 +76,7 @@ export const ThemeVariations = { title: 'Richard Hillgrove', id: 'profile/richard-hillgrove', type: 'contributor', - bylineImageUrl: + bylineLargeImageUrl: 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', }, ], From a35100218e3c960009641b17cafc95dc4efb0821 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:43:11 +0000 Subject: [PATCH 17/22] update bottom margin to match designs --- dotcom-rendering/src/components/MultiByline.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index a40791ce983..e2a6b70938e 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -207,7 +207,6 @@ export const nonAnchorHeadlineStyles = ({ export const multiBylineBylineStyles = (format: ArticleFormat) => css` ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} - padding-bottom: 8px; /* stylelint-disable-next-line declaration-no-important */ font-style: italic !important; margin-top: -4px; @@ -240,6 +239,11 @@ const bylineWrapperStyles = css` const bylineTextStyles = css` flex-grow: 1; + margin-bottom: ${space[6]}px; + ${from.tablet} { + margin-bottom: ${space[9]}px; + } +} `; const bylineImageStyles = css` From 86c292eb5b8096a330763175e1a08d571c8087e7 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:47:26 +0000 Subject: [PATCH 18/22] fix css linting --- dotcom-rendering/src/components/MultiByline.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index e2a6b70938e..308551ff0bd 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -243,7 +243,6 @@ const bylineTextStyles = css` ${from.tablet} { margin-bottom: ${space[9]}px; } -} `; const bylineImageStyles = css` From e1fd0aea260e9a4e192078d3d757c9345c2c25d4 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 12:54:21 +0000 Subject: [PATCH 19/22] filter out audio designs in stories --- dotcom-rendering/src/components/MiniProfiles.stories.tsx | 2 +- dotcom-rendering/src/components/MultiBylines.stories.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/components/MiniProfiles.stories.tsx b/dotcom-rendering/src/components/MiniProfiles.stories.tsx index afabcba7008..af1431153f3 100644 --- a/dotcom-rendering/src/components/MiniProfiles.stories.tsx +++ b/dotcom-rendering/src/components/MiniProfiles.stories.tsx @@ -91,7 +91,7 @@ export const ThemeVariations = { } satisfies Story; // Audio designs don't support mini profiles -const isNotAudioDesign = (format: ArticleFormat) => +export const isNotAudioDesign = (format: ArticleFormat) => format.design !== ArticleDesign.Audio; export const DesignVariations = { diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index 459a9621aed..4a4172d5677 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -10,6 +10,7 @@ import { } from '../lib/articleFormat'; import { RenderArticleElement } from '../lib/renderElement'; import type { TextBlockElement } from '../types/content'; +import { isNotAudioDesign } from './MiniProfiles.stories'; import { MultiBylines } from './MultiBylines'; const meta = { @@ -124,7 +125,7 @@ export const DesignVariations = { formats: getAllDesigns({ theme: Pillar.News, display: ArticleDisplay.Standard, - }), + }).filter(isNotAudioDesign), chromatic: { modes: { horizontal: allModes.splitHorizontal, From 2c78818b23ff8b39cfa74c1c72a0c7430dd05fd6 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 13:51:30 +0000 Subject: [PATCH 20/22] tidy up labs-specific styling and remove `!important` to keep linter happy. --- .../src/components/MultiByline.tsx | 36 ++++++------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/dotcom-rendering/src/components/MultiByline.tsx b/dotcom-rendering/src/components/MultiByline.tsx index 308551ff0bd..924defd44bb 100644 --- a/dotcom-rendering/src/components/MultiByline.tsx +++ b/dotcom-rendering/src/components/MultiByline.tsx @@ -9,14 +9,12 @@ import { neutral, space, textSans14, - textSans24, - textSans28, - textSans34, - textSansItalic17, + textSansItalic24, + textSansItalic28, + textSansItalic34, } from '@guardian/source/foundations'; import sanitise, { defaults } from 'sanitize-html'; import { - ArticleDesign, ArticleDisplay, type ArticleFormat, ArticleSpecial, @@ -33,11 +31,6 @@ const multiBylineItemStyles = css` padding-top: 8px; `; -const labsBylineStyles = css` - ${textSansItalic17}; - line-height: 1.4; -`; - const headingLineStyles = css` width: 140px; margin: 8px 0 2px 0; @@ -143,7 +136,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${textSans28}; + ${textSansItalic28}; font-weight: 300; line-height: 1.15; ` @@ -153,7 +146,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${textSans24}; + ${textSansItalic24}; ${ fontWeight === 'light' ? 'font-weight: 300;' @@ -172,7 +165,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${textSans34}; + ${textSansItalic34}; font-weight: 300; line-height: 1.15; ` @@ -182,7 +175,7 @@ export const nonAnchorHeadlineStyles = ({ * This has been done because the styles do not directly map to the new presets. * Please speak to your team's designer and update this to use a more appropriate preset. */ - ${textSans28}; + ${textSansItalic28}; ${ fontWeight === 'light' ? 'font-weight: 300;' @@ -205,13 +198,11 @@ export const nonAnchorHeadlineStyles = ({ } `; -export const multiBylineBylineStyles = (format: ArticleFormat) => css` +export const bylineStyles = (format: ArticleFormat) => css` ${nonAnchorHeadlineStyles({ format, fontWeight: 'light' })} - /* stylelint-disable-next-line declaration-no-important */ - font-style: italic !important; + font-style: italic; margin-top: -4px; - /* stylelint-disable-next-line declaration-no-important */ - font-weight: 300 !important; + font-weight: 300; color: ${neutral[46]}; a { ${subheadingStyles(format)} @@ -342,12 +333,7 @@ const Byline = ({ {bylineHtml ? (

      ) : null} From 9ba24e4f085cf3ae01792c3b9b90ecac1354b517 Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:09:26 +0000 Subject: [PATCH 21/22] make stories shorter to fix chromatic build --- .../src/components/MiniProfiles.stories.tsx | 2 +- .../src/components/MultiBylines.stories.tsx | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dotcom-rendering/src/components/MiniProfiles.stories.tsx b/dotcom-rendering/src/components/MiniProfiles.stories.tsx index af1431153f3..afabcba7008 100644 --- a/dotcom-rendering/src/components/MiniProfiles.stories.tsx +++ b/dotcom-rendering/src/components/MiniProfiles.stories.tsx @@ -91,7 +91,7 @@ export const ThemeVariations = { } satisfies Story; // Audio designs don't support mini profiles -export const isNotAudioDesign = (format: ArticleFormat) => +const isNotAudioDesign = (format: ArticleFormat) => format.design !== ArticleDesign.Audio; export const DesignVariations = { diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index 4a4172d5677..95b32c7ebfb 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -3,6 +3,7 @@ import type { Meta, StoryObj } from '@storybook/react'; import { centreColumnDecorator } from '../../.storybook/decorators/gridDecorators'; import { allModes } from '../../.storybook/modes'; import { images } from '../../fixtures/generated/images'; +import type { ArticleFormat } from '../lib/articleFormat'; import { ArticleDesign, getAllDesigns, @@ -10,7 +11,6 @@ import { } from '../lib/articleFormat'; import { RenderArticleElement } from '../lib/renderElement'; import type { TextBlockElement } from '../types/content'; -import { isNotAudioDesign } from './MiniProfiles.stories'; import { MultiBylines } from './MultiBylines'; const meta = { @@ -26,7 +26,7 @@ const testTextElement: TextBlockElement = { _type: 'model.dotcomrendering.pageElements.TextBlockElement', elementId: 'test-text-element-id-1', dropCap: 'on', // this should be overruled by multi byline which always sets forceDropCap="off" - html: '

      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesquepharetra libero nec varius feugiat. Nulla commodo sagittis erat amalesuada. Ut iaculis interdum eros, et tristique ex. In veldignissim arcu. Nulla nisi urna, laoreet a aliquam at, viverra eueros. Proin imperdiet pellentesque turpis sed luctus. Donecdignissim lacus in risus fermentum maximus eu vel justo. Duis nontortor ac elit dapibus imperdiet ut at risus. Etiam pretium, odioeget accumsan venenatis, tortor mi aliquet nisl, vel ullamcorperneque nulla vel elit. Etiam porta mauris nec sagittis luctus.

      ', + html: '

      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesquepharetra libero nec varius feugiat. Nulla commodo sagittis erat amalesuada.

      ', }; const testParagraph = @@ -48,7 +48,7 @@ export const ThemeVariations = { contributorIds: ['profile/richard-hillgrove'], }, { - title: 'My hot take', + title: 'A byline with an image override url', bio: testBioText, body: [testTextElement], byline: 'Guardian Contributor', @@ -118,6 +118,10 @@ export const ThemeVariations = { }, } satisfies Story; +// Audio designs don't support multi-bylines +const isNotAudioDesign = (format: ArticleFormat) => + format.design !== ArticleDesign.Audio; + export const DesignVariations = { args: ThemeVariations.args, decorators: [centreColumnDecorator], From 01ae5195cf451e94fbb21d5b6d066e670ea0a47b Mon Sep 17 00:00:00 2001 From: Rebecca Thompson <33927854+rebecca-thompson@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:43:18 +0000 Subject: [PATCH 22/22] break up design stories to fix chromatic build --- .../src/components/MultiBylines.stories.tsx | 180 +++++++++++------- 1 file changed, 111 insertions(+), 69 deletions(-) diff --git a/dotcom-rendering/src/components/MultiBylines.stories.tsx b/dotcom-rendering/src/components/MultiBylines.stories.tsx index 95b32c7ebfb..7604eb9f14d 100644 --- a/dotcom-rendering/src/components/MultiBylines.stories.tsx +++ b/dotcom-rendering/src/components/MultiBylines.stories.tsx @@ -9,8 +9,9 @@ import { getAllDesigns, getAllThemes, } from '../lib/articleFormat'; +import type { EditionId } from '../lib/edition'; import { RenderArticleElement } from '../lib/renderElement'; -import type { TextBlockElement } from '../types/content'; +import type { MultiByline, TextBlockElement } from '../types/content'; import { MultiBylines } from './MultiBylines'; const meta = { @@ -35,75 +36,84 @@ const testListHtml = '

      • This is the first item in the list

      • The second item has a hyperlink.

      '; const testBioText = testParagraph + testListHtml; -export const ThemeVariations = { - args: { - multiBylines: [ - { - title: 'This subheading is quite long so is likely to run on to multiple lines', - bio: testBioText, - body: [testTextElement], - byline: 'Richard Hillgrove Political Editor', - bylineHtml: - "Richard Hillgrove Political Editor", - contributorIds: ['profile/richard-hillgrove'], - }, - { - title: 'A byline with an image override url', - bio: testBioText, - body: [testTextElement], - byline: 'Guardian Contributor', - bylineHtml: - "Richard Hillgrove", - contributorIds: ['profile/richard-hillgrove'], - imageOverrideUrl: - 'https://i.guim.co.uk/img/uploads/2024/09/17/Maurice_Casey.png?width=180&dpr=1&s=none', - }, - { - title: 'A further subheading', - body: [testTextElement], - endNote: 'This is an end note.', - }, - { - title: 'This byline has a contributor with no link', - bio: testBioText, - body: [testTextElement], - byline: 'Steve McQueen on Paul Gilroy', - bylineHtml: - "Steve McQueen on Paul Gilroy", - }, - ], - tags: [ - { - title: 'Richard Hillgrove', - id: 'profile/richard-hillgrove', - type: 'contributor', - bylineLargeImageUrl: - 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', - }, - ], - isLastElement: true, - /** - * This will be replaced by the `formats` parameter, but it's - * required by the type. - */ - format: { - design: ArticleDesign.Standard, - display: ArticleDisplay.Standard, - theme: Pillar.News, +const multiBylineWithLongHeader = { + title: 'This subheading is quite long so is likely to run on to multiple lines', + bio: testBioText, + body: [testTextElement], + byline: 'Richard Hillgrove Political Editor', + bylineHtml: + "Richard Hillgrove Political Editor", + contributorIds: ['profile/richard-hillgrove'], +}; + +const multiBylineWithImageOverride = { + title: 'A byline with an image override url', + bio: testBioText, + body: [testTextElement], + byline: 'Guardian Contributor', + bylineHtml: "Richard Hillgrove", + contributorIds: ['profile/richard-hillgrove'], + imageOverrideUrl: + 'https://i.guim.co.uk/img/uploads/2024/09/17/Maurice_Casey.png?width=180&dpr=1&s=none', +}; + +const multiBylineWithNoByline = { + title: 'A further subheading', + body: [testTextElement], + endNote: 'This is an end note.', +}; + +const multiBylineWithNoContributorLink = { + title: 'This byline has a contributor with no link', + bio: testBioText, + body: [testTextElement], + byline: 'Steve McQueen on Paul Gilroy', + bylineHtml: + "Steve McQueen on Paul Gilroy", +}; + +const args = (multiBylines: MultiByline[]) => ({ + multiBylines, + tags: [ + { + title: 'Richard Hillgrove', + id: 'profile/richard-hillgrove', + type: 'contributor', + bylineLargeImageUrl: + 'https://i.guim.co.uk/img/static/sys-images/Guardian/Pix/pictures/2011/5/24/1306249890287/Richard-Hillgrove.jpg?width=100&dpr=2&s=none', }, - abTests: {}, - /** - * This is used for rich links. An empty string isn't technically valid, - * but there are no rich links in this example. - */ - ajaxUrl: '', - editionId: 'UK', - isAdFreeUser: false, - isSensitive: false, - pageId: 'testID', - switches: {}, - RenderArticleElement, + ], + isLastElement: true, + /** + * This will be replaced by the `formats` parameter, but it's + * required by the type. + */ + format: { + design: ArticleDesign.Standard, + display: ArticleDisplay.Standard, + theme: Pillar.News, }, + abTests: {}, + /** + * This is used for rich links. An empty string isn't technically valid, + * but there are no rich links in this example. + */ + ajaxUrl: '', + editionId: 'UK' as EditionId, + isAdFreeUser: false, + isSensitive: false, + pageId: 'testID', + switches: {}, + RenderArticleElement, +}); + +export const ThemeVariations = { + args: args([ + multiBylineWithLongHeader, + multiBylineWithImageOverride, + multiBylineWithNoByline, + multiBylineWithNoContributorLink, + ]), decorators: [centreColumnDecorator], parameters: { formats: getAllThemes({ @@ -123,7 +133,39 @@ const isNotAudioDesign = (format: ArticleFormat) => format.design !== ArticleDesign.Audio; export const DesignVariations = { - args: ThemeVariations.args, + args: args([multiBylineWithLongHeader, multiBylineWithNoByline]), + decorators: [centreColumnDecorator], + parameters: { + formats: getAllDesigns({ + theme: Pillar.News, + display: ArticleDisplay.Standard, + }).filter(isNotAudioDesign), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const DesignVariationsWithImageOverride = { + args: args([multiBylineWithImageOverride]), + decorators: [centreColumnDecorator], + parameters: { + formats: getAllDesigns({ + theme: Pillar.News, + display: ArticleDisplay.Standard, + }).filter(isNotAudioDesign), + chromatic: { + modes: { + horizontal: allModes.splitHorizontal, + }, + }, + }, +} satisfies Story; + +export const DesignVariationsWithNoContributorLink = { + args: args([multiBylineWithNoContributorLink]), decorators: [centreColumnDecorator], parameters: { formats: getAllDesigns({