diff --git a/src/app/components/Byline/index.styles.tsx b/src/app/components/Byline/index.styles.tsx index 9c4407d992b..396ebd33ea9 100644 --- a/src/app/components/Byline/index.styles.tsx +++ b/src/app/components/Byline/index.styles.tsx @@ -4,47 +4,39 @@ import pixelsToRem from '../../utilities/pixelsToRem'; export default { bylineContainer: ({ spacings, mq }: Theme) => css({ - display: 'grid', paddingInlineStart: `${spacings.FULL}rem`, - [mq.GROUP_1_MIN_WIDTH]: { - gridTemplateColumns: 'repeat(2, auto)', - }, + paddingBottom: `${spacings.TRIPLE}rem`, + lineHeight: '1.35rem', // not sure I should do this due to script sizes [mq.GROUP_2_MIN_WIDTH]: { - display: 'flex', - flexWrap: 'wrap', paddingInlineStart: `${spacings.DOUBLE}rem`, }, [mq.GROUP_4_MIN_WIDTH]: { paddingInlineStart: 0 }, }), - bylineSection: ({ spacings, mq }: Theme) => + bylineContainerSingleContributor: () => css({ - marginRight: `${spacings.FULL}rem`, - marginBottom: `${spacings.FULL}rem`, - [mq.GROUP_3_MIN_WIDTH]: { - marginRight: `${spacings.DOUBLE}rem`, - marginBottom: 0, - }, + display: 'flex', + flexWrap: 'wrap', }), - bylineList: () => css({ listStyle: 'none', padding: 0, margin: 0 }), + list: () => css({ listStyle: 'none', padding: 0, margin: 0 }), author: ({ palette, isDarkUi }: Theme) => css({ color: isDarkUi ? palette.GREY_2 : palette.GREY_10, display: 'inline-block', + }), + + authorSingleContributor: () => + css({ verticalAlign: 'middle', }), jobRole: ({ palette, isDarkUi }: Theme) => css({ color: isDarkUi ? palette.GREY_2 : palette.GREY_6 }), - twitterText: ({ palette }: Theme) => - css({ - color: palette.POSTBOX, - display: 'inline-block', - verticalAlign: 'middle', - }), + comma: ({ palette, isDarkUi }: Theme) => + css({ color: isDarkUi ? palette.GREY_2 : palette.GREY_6 }), authorChevron: ({ palette, isDarkUi, spacings, mq }: Theme) => css({ @@ -52,30 +44,16 @@ export default { margin: `0 ${spacings.HALF}rem`, color: isDarkUi ? palette.GREY_2 : palette.GREY_10, fill: 'currentcolor', - width: `${spacings.FULL + spacings.HALF}rem`, - height: `${spacings.FULL + spacings.HALF}rem`, - [mq.FORCED_COLOURS]: { fill: 'linkText' }, - }), - - twitterChevron: ({ palette, spacings, mq }: Theme) => - css({ - verticalAlign: 'middle', - margin: `0 ${spacings.HALF}rem`, - color: palette.POSTBOX, - fill: 'currentcolor', - width: `${spacings.FULL}rem`, - height: `${spacings.FULL}rem`, + width: `${pixelsToRem(10)}rem`, + height: `${pixelsToRem(10)}rem`, [mq.FORCED_COLOURS]: { fill: 'linkText' }, }), link: ({ mq, palette }: Theme) => css({ display: 'inline-block', - textDecoration: 'none', - paddingInlineEnd: '2.75rem', '&:focus, &:hover': { '.byline-link': { - textDecoration: 'underline', color: palette.POSTBOX, }, }, @@ -93,37 +71,36 @@ export default { }, }), - location: () => + linkMultipleContributor: ({ palette }: Theme) => css({ - margin: '0', - display: 'block', - clear: 'both', - }), - - locationText: ({ palette, isDarkUi }: Theme) => - css({ - color: isDarkUi ? palette.GREY_2 : palette.SHADOW, - display: 'block', - paddingTop: ` ${pixelsToRem(4)}rem`, + '.byline-link': { + textDecoration: 'underline', + textDecorationThickness: `${pixelsToRem(1)}rem`, + textUnderlineOffset: `${pixelsToRem(4)}rem`, + textDecorationColor: palette.GREY_5, + '&:focus, &:hover': { + textDecorationColor: palette.POSTBOX, + }, + }, }), - reportingFromText: ({ palette, isDarkUi }: Theme) => + linkSingleContributor: () => css({ - color: isDarkUi ? palette.GREY_2 : palette.SHADOW, + paddingInlineEnd: '2.75rem', + textDecoration: 'none', + '&:focus, &:hover': { + '.byline-link': { + textDecoration: 'underline', + textDecorationColor: 'currentcolor', + textDecorationThickness: `${pixelsToRem(2)}rem`, + textUnderlineOffset: `${pixelsToRem(4)}rem`, + }, + }, }), - timestampLineBreak: ({ palette, spacings, mq }: Theme) => + locationText: ({ palette, isDarkUi }: Theme) => css({ - // EXPERIMENT: Article Read Time - '&:nth-child(2)::before': { - content: '""', - borderTop: `${pixelsToRem(2)}rem solid ${palette.GREY_5}`, - width: `${pixelsToRem(40)}rem`, - display: 'block', - margin: `${spacings.DOUBLE}rem ${spacings.FULL}rem`, - [mq.GROUP_2_MIN_WIDTH]: { margin: `${spacings.DOUBLE}rem` }, - [mq.GROUP_4_MIN_WIDTH]: { margin: `${spacings.DOUBLE}rem 0` }, - }, + color: isDarkUi ? palette.GREY_2 : palette.GREY_6, }), ImageWrapper: ({ palette }: Theme) => @@ -163,9 +140,8 @@ export default { height: `${pixelsToRem(80)}rem`, }), - twitterLink: () => + displayInline: () => css({ - paddingBottom: `${pixelsToRem(22)}rem`, - paddingTop: `${pixelsToRem(4)}rem`, + display: `inline`, }), }; diff --git a/src/app/components/Byline/index.test.tsx b/src/app/components/Byline/index.test.tsx index 052e9d761b9..81cc7ee1536 100644 --- a/src/app/components/Byline/index.test.tsx +++ b/src/app/components/Byline/index.test.tsx @@ -37,14 +37,11 @@ describe('Byline', () => { render(); const AuthorLink = screen.getByText('Single Byline (all values)'); - const TwitterLink = screen.getByText('@test'); const Links = screen.getAllByRole('link'); expect(AuthorLink).toBeInTheDocument(); - expect(TwitterLink).toBeInTheDocument(); - expect(Links.length).toBe(2); + expect(Links.length).toBe(1); expect(Links[0]).toHaveAttribute('href', '/news/topics/c8qx38nq177t'); - expect(Links[1]).toHaveAttribute('href', 'https://twitter.com/test'); }); it('should render a section with role region', () => { @@ -70,7 +67,7 @@ describe('Byline', () => { const firstContributorItems = within(firstContributor).getAllByRole('listitem'); - expect(firstContributorItems.length).toBe(5); + expect(firstContributorItems.length).toBe(4); }); it('should correctly use the buildIChefURL function to create the image url', () => { @@ -135,14 +132,10 @@ describe('Byline', () => { render(); const AuthorLink = screen.getByText('Mayeni Jones'); - const TwitterLink = screen.getByText('@MayeniJones'); - const Links = screen.getAllByRole('link'); const Location = screen.getByText('Lagos, Nigeria'); const Image = screen.getByRole('img'); expect(AuthorLink).toBeInTheDocument(); - expect(TwitterLink).toBeInTheDocument(); - expect(Links.length).toBe(1); expect(Location).toBeInTheDocument(); expect(Image).toBeInTheDocument(); }); @@ -151,8 +144,7 @@ describe('Byline', () => { expectation | info | text ${'Author'} | ${'Author'} | ${'Author,'} ${'Role'} | ${'Role'} | ${'Role,'} - ${'X'} | ${'X'} | ${'X,'} - ${'Reporting from'} | ${'Reporting from'} | ${'Reporting from'} + ${'Reporting from'} | ${'Reporting from'} | ${'Reporting from,'} `('should correctly announce $expectation for $info', ({ text }) => { render( @@ -173,7 +165,7 @@ describe('Byline', () => { info | translation ${'author'} | ${'Barreessaa,'} ${'role'} | ${'Gahee,'} - ${'reportingFrom'} | ${'Gabaasni irraati'} + ${'reportingFrom'} | ${'Gabaasni irraati,'} `('should translate $info announcement correctly', ({ translation }) => { render( diff --git a/src/app/components/Byline/index.tsx b/src/app/components/Byline/index.tsx index 4fe79767db5..6769ef857d9 100644 --- a/src/app/components/Byline/index.tsx +++ b/src/app/components/Byline/index.tsx @@ -12,81 +12,93 @@ import Text from '../Text'; import Image from '../Image'; import bylineExtractor from './utilities/bylineExtractor'; -const Byline = ({ - blocks, - children = null, -}: PropsWithChildren) => { +const Comma = () => { + return ( + + ); +}; + +const Contributors = ({ contributorValues, isSingleContributor }) => { const { translations, dir } = use(ServiceContext); const isRtl = dir === 'rtl'; - const bylineValues = bylineExtractor(blocks); - const { byline: { author = 'Author', - articleInformation = 'Article Information', reportingFrom = 'Reporting from', role = 'Role', } = {}, } = translations ?? {}; - const contributors = - bylineValues.length === 0 - ? null - : bylineValues?.map(values => { - if (!values) return null; + const areMultipleContributors = !isSingleContributor; - const { - authorName, - jobRole, - twitterText, - twitterLink, - authorImage, - location, - authorTopicUrl, - } = values; + const lastContributorIndex = contributorValues.length - 1; - return ( - + ); + })} + + ); +}; + +const Byline = ({ + blocks, + children = null, +}: PropsWithChildren) => { + const { translations } = use(ServiceContext); + + const contributorValues = bylineExtractor(blocks); + + const isSingleContributor = contributorValues.length === 1; + + const { byline: { articleInformation = 'Article Information' } = {} } = + translations ?? {}; return ( - contributors?.[0] && ( -
+ contributorValues?.[0] && ( +
{articleInformation} -
    -
  • {contributors}
    • +
      +
    • + +
      • {/* EXPERIMENT: Article Read Time */} {children && React.Children.map(children, (child, index) => ( -
    • - {child} -
      • +
    • {child}
      • ))}
diff --git a/src/app/components/Byline/utilities/bylineExtractor/index.test.tsx b/src/app/components/Byline/utilities/bylineExtractor/index.test.tsx index 94ceaf51a55..a7ce71afa60 100644 --- a/src/app/components/Byline/utilities/bylineExtractor/index.test.tsx +++ b/src/app/components/Byline/utilities/bylineExtractor/index.test.tsx @@ -12,51 +12,27 @@ describe('bylineExtractor', () => { expect(bylineValues).toHaveLength(0); }); - it('should return an array with a maximum of 4 objects containing all byline data', () => { + it('should return an array with all contributors containing all byline data', () => { const bylineValues = bylineExtractor(bylineWithMultipleContributors); - expect(bylineValues).toHaveLength(4); + const sampleContributor = { + authorImage: + 'https://ichef.bbci.co.uk/ace/ws/160/cpsprodpb/f974/live/36226e20-94aa-11ec-9acc-37a09ce5ea88.png.webp', + authorName: 'Mayeni Jones', + authorTopicUrl: '/news/topics/c8qx38nq177t', + jobRole: 'Journalist', + location: 'Lagos, Nigeria', + twitterLink: 'https://twitter.com/MayeniJones', + twitterText: 'MayeniJones', + }; + + expect(bylineValues).toHaveLength(5); expect(bylineValues).toEqual([ - { - authorImage: - 'https://ichef.bbci.co.uk/ace/ws/160/cpsprodpb/f974/live/36226e20-94aa-11ec-9acc-37a09ce5ea88.png.webp', - authorName: 'Mayeni Jones', - authorTopicUrl: '/news/topics/c8qx38nq177t', - jobRole: 'Journalist', - location: 'Lagos, Nigeria', - twitterLink: 'https://twitter.com/MayeniJones', - twitterText: 'MayeniJones', - }, - { - authorImage: - 'https://ichef.bbci.co.uk/ace/ws/160/cpsprodpb/f974/live/36226e20-94aa-11ec-9acc-37a09ce5ea88.png.webp', - authorName: 'Mayeni Jones', - authorTopicUrl: '/news/topics/c8qx38nq177t', - jobRole: 'Journalist', - location: 'Lagos, Nigeria', - twitterLink: 'https://twitter.com/MayeniJones', - twitterText: 'MayeniJones', - }, - { - authorImage: - 'https://ichef.bbci.co.uk/ace/ws/160/cpsprodpb/f974/live/36226e20-94aa-11ec-9acc-37a09ce5ea88.png.webp', - authorName: 'Mayeni Jones', - authorTopicUrl: '/news/topics/c8qx38nq177t', - jobRole: 'Journalist', - location: 'Lagos, Nigeria', - twitterLink: 'https://twitter.com/MayeniJones', - twitterText: 'MayeniJones', - }, - { - authorImage: - 'https://ichef.bbci.co.uk/ace/ws/160/cpsprodpb/f974/live/36226e20-94aa-11ec-9acc-37a09ce5ea88.png.webp', - authorName: 'Mayeni Jones', - authorTopicUrl: '/news/topics/c8qx38nq177t', - jobRole: 'Journalist', - location: 'Lagos, Nigeria', - twitterLink: 'https://twitter.com/MayeniJones', - twitterText: 'MayeniJones', - }, + sampleContributor, + sampleContributor, + sampleContributor, + sampleContributor, + sampleContributor, ]); }); diff --git a/src/app/components/Byline/utilities/bylineExtractor/index.tsx b/src/app/components/Byline/utilities/bylineExtractor/index.tsx index bad30519362..10caa8de158 100644 --- a/src/app/components/Byline/utilities/bylineExtractor/index.tsx +++ b/src/app/components/Byline/utilities/bylineExtractor/index.tsx @@ -27,7 +27,6 @@ const pathOrZeroIndexModelBlocks = ( const bylineExtractor = (blocks: OptimoBylineContributorBlock[]) => { return blocks - .slice(0, 4) .map(contribBlock => { const bylineBlocks = contribBlock?.model?.blocks || []; @@ -67,8 +66,7 @@ const bylineExtractor = (blocks: OptimoBylineContributorBlock[]) => { if (!authorImage.endsWith('.png.webp')) authorImage = ''; - const contributorBlock = blocks?.[0] ?? []; - const authorTopicUrl = contributorBlock?.model?.topicUrl ?? ''; + const authorTopicUrl = contribBlock?.model?.topicUrl ?? ''; return { authorName, diff --git a/src/app/models/blocks/index.js b/src/app/models/blocks/index.js index 138fa736984..3aa59494620 100644 --- a/src/app/models/blocks/index.js +++ b/src/app/models/blocks/index.js @@ -77,7 +77,7 @@ export const rawVideoModel = ( duration: videoDuration, }); -export const bylineBlock = (text, id = null) => { +export const bylineBlock = (bylineType, text, id = null) => { const fragment = singleFragmentBlock(`@${text}`, id); const urlLink = optionalIdBlock( blockBase('urlLink', { @@ -118,13 +118,22 @@ export const bylineBlock = (text, id = null) => { id, ); const byline = optionalIdBlock( - blockBase('byline', { blocks: [contributor] }), + blockBase(bylineType, { blocks: [contributor] }), id, ); return byline; }; +export const timestampBlock = (id = null) => + optionalIdBlock( + blockBase('timestamp', { + firstPublished: 1574854374815, + lastPublished: 1574854374815, + }), + id, + ); + export const rawImageModel = imageLocator => ({ locator: imageLocator, }); diff --git a/src/app/models/blocks/index.test.js b/src/app/models/blocks/index.test.js index 190e6be9d69..765022b1680 100644 --- a/src/app/models/blocks/index.test.js +++ b/src/app/models/blocks/index.test.js @@ -226,128 +226,140 @@ describe('High order blocks', () => { }); describe('Byline block', () => { - test('generates a byline block json', () => { - const testJson = { - type: 'byline', - model: { - blocks: [ - { - type: 'contributor', - model: { - blocks: [ - { - type: 'name', - model: { - blocks: [ - { - type: 'text', - model: { - blocks: [ - { - type: 'paragraph', - model: { - text: 'Test', - blocks: [ - { - type: 'fragment', - model: { - text: 'Test', - attributes: [], - }, - }, - ], - }, + const contributorJson = { + type: 'contributor', + model: { + blocks: [ + { + type: 'name', + model: { + blocks: [ + { + type: 'text', + model: { + blocks: [ + { + type: 'paragraph', + model: { + text: 'Test', + blocks: [ + { + type: 'fragment', + model: { + text: 'Test', + attributes: [], }, - ], - }, + }, + ], }, - ], - }, + }, + ], }, - { - type: 'role', - model: { - blocks: [ - { - type: 'text', - model: { - blocks: [ - { - type: 'paragraph', - model: { - text: 'Test', - blocks: [ - { - type: 'fragment', - model: { - text: 'Test', - attributes: [], - }, - }, - ], - }, + }, + ], + }, + }, + { + type: 'role', + model: { + blocks: [ + { + type: 'text', + model: { + blocks: [ + { + type: 'paragraph', + model: { + text: 'Test', + blocks: [ + { + type: 'fragment', + model: { + text: 'Test', + attributes: [], }, - ], - }, + }, + ], }, - ], - }, + }, + ], }, - { - type: 'link', - locator: 'urn:bbc:twitter:user:@mary_harper', - model: { - blocks: [ - { - type: 'text', - model: { - blocks: [ - { - type: 'paragraph', - model: { - text: '@mary_harper', - blocks: [ - { - type: 'urlLink', - model: { - text: '@mary_harper', - locator: - 'https://twitter.com/mary_harper', - blocks: [ - { - type: 'fragment', - model: { - text: '@mary_harper', - attributes: [], - }, - id: 'testId', - }, - ], - }, - id: 'testId', + }, + ], + }, + }, + { + type: 'link', + locator: 'urn:bbc:twitter:user:@mary_harper', + model: { + blocks: [ + { + type: 'text', + model: { + blocks: [ + { + type: 'paragraph', + model: { + text: '@mary_harper', + blocks: [ + { + type: 'urlLink', + model: { + text: '@mary_harper', + locator: 'https://twitter.com/mary_harper', + blocks: [ + { + type: 'fragment', + model: { + text: '@mary_harper', + attributes: [], }, - ], - }, - id: 'testId', + id: 'testId', + }, + ], }, - ], - }, - id: 'testId', + id: 'testId', + }, + ], }, - ], - }, - id: 'testId', + id: 'testId', + }, + ], }, - ], - }, - id: 'testId', + id: 'testId', + }, + ], }, - ], + id: 'testId', + }, + ], + }, + id: 'testId', + }; + test('generates a byline block json', () => { + const testJson = { + type: 'byline', + model: { + blocks: [contributorJson], + }, + id: 'testId', + }; + + const block = bylineBlock('byline', 'mary_harper', 'testId'); + + expect(block).toEqual(testJson); + }); + + test('generates a subByline block json', () => { + const testJson = { + type: 'subByline', + model: { + blocks: [contributorJson], }, id: 'testId', }; - const block = bylineBlock('mary_harper', 'testId'); + const block = bylineBlock('subByline', 'mary_harper', 'testId'); expect(block).toEqual(testJson); }); diff --git a/src/app/pages/ArticlePage/ArticlePage.tsx b/src/app/pages/ArticlePage/ArticlePage.tsx index 050ef04bf8f..538ac670039 100644 --- a/src/app/pages/ArticlePage/ArticlePage.tsx +++ b/src/app/pages/ArticlePage/ArticlePage.tsx @@ -267,9 +267,11 @@ const ArticlePage = ({ pageData }: { pageData: Article }) => { const topics = pageData?.metadata?.topics ?? []; const blocks = pageData?.content?.model?.blocks ?? []; const startsWithHeading = blocks?.[0]?.type === 'headline' || false; + const bylineBlock = blocks.find( - block => block.type === 'byline', - ) as OptimoBylineBlock; + (block): block is OptimoBylineBlock => + block.type === 'byline' || block.type === 'subByline', + ); const bylineContribBlocks = bylineBlock?.model?.blocks || []; diff --git a/src/app/pages/ArticlePage/fixtureData.ts b/src/app/pages/ArticlePage/fixtureData.ts index ae95c3ddce3..772b0601c72 100644 --- a/src/app/pages/ArticlePage/fixtureData.ts +++ b/src/app/pages/ArticlePage/fixtureData.ts @@ -8,6 +8,7 @@ import { bylineBlock, singleTextBlock, textBlock, + timestampBlock, } from '#models/blocks'; const plainOptimoBlock = (blocks: (object | string)[]) => @@ -30,9 +31,27 @@ const blocksWithHeadlineTextAndByline = (blockValues: (object | string)[]) => { return [ blockContainingText('headline', headlineText, 1), // @ts-expect-error - type checking not added for block helpers - bylineBlock(twitterHandle, 2), + bylineBlock('byline', twitterHandle, 2), // @ts-expect-error - type checking not added for block helpers - singleTextBlock(paragraphText, 3), + timestampBlock(3), + // @ts-expect-error - type checking not added for block helpers + singleTextBlock(paragraphText, 4), + ]; +}; + +const blocksWithHeadlineTextAndSubByline = ( + blockValues: (object | string)[], +) => { + const [headlineText, paragraphText, twitterHandle] = blockValues; + + return [ + blockContainingText('headline', headlineText, 1), + // @ts-expect-error - type checking not added for block helpers + bylineBlock('subByline', twitterHandle, 2), + // @ts-expect-error - type checking not added for block helpers + timestampBlock(3), + // @ts-expect-error - type checking not added for block helpers + singleTextBlock(paragraphText, 4), ]; }; @@ -779,6 +798,20 @@ export const articleDataPidginWithByline = articleDataBuilder( blocksWithHeadlineTextAndByline, ) as unknown as Article; +export const articleDataPidginWithSubByline = articleDataBuilder( + 'cwl08rd38l6o', + 'Pidgin', + 'pcm', + 'http://www.bbc.co.uk/ontologies/passport/home/Pidgin', + ['Article Headline in Pidgin', 'A paragraph in Pidgin.', 'mary_harper'], + 'Article Headline for SEO in Pidgin', + 'Article Headline for Promo in Pidgin', + 'Article summary in Pidgin', + emptyThings, + undefined, + blocksWithHeadlineTextAndSubByline, +) as unknown as Article; + export const articlePglDataPidgin = articleDataBuilder( 'cwl08rd38l6o', 'Pidgin', diff --git a/src/app/pages/ArticlePage/index.test.tsx b/src/app/pages/ArticlePage/index.test.tsx index 837db3ae1ee..09057446aea 100644 --- a/src/app/pages/ArticlePage/index.test.tsx +++ b/src/app/pages/ArticlePage/index.test.tsx @@ -12,6 +12,7 @@ import { articleDataPidgin, articleDataPidginWithAds, articleDataPidginWithByline, + articleDataPidginWithSubByline, articleDataRussianWithPVButNoWatchMomentsTranslation, articleDataPortugueseWithPVNotUnderHeadline, articleDataPortugueseWithPVUnderHeadline, @@ -768,6 +769,34 @@ describe('Article Page', () => { expect(getByText('UGC Core Features 1 - Custom Form')).toBeInTheDocument(); }); + it('should render a byline when passed a byline', async () => { + const pageDataWithByline = { + ...articleDataPidginWithByline, + }; + + const { getByTestId } = render( + + + , + ); + + expect(getByTestId('byline')).toBeInTheDocument(); + }); + + it('should render a byline when passed a subByline', async () => { + const pageDataWithSubByline = { + ...articleDataPidginWithSubByline, + }; + + const { getByTestId } = render( + + + , + ); + + expect(getByTestId('byline')).toBeInTheDocument(); + }); + it('should set "amphtml" link tag for asset', async () => { render(