From 063cf77ded424023cc8a954483f75717eb4324f2 Mon Sep 17 00:00:00 2001 From: Lukas Freimonas Date: Mon, 23 Mar 2026 15:11:30 +0200 Subject: [PATCH 1/5] fix: update HIP to use relatedTopic prop --- .../Curation/HighImpactPromo/index.test.tsx | 44 ++++++++++++++++++- .../Curation/HighImpactPromo/index.tsx | 29 +++++++----- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/app/components/Curation/HighImpactPromo/index.test.tsx b/src/app/components/Curation/HighImpactPromo/index.test.tsx index f96b2b0a79f..3febb7e0c6a 100644 --- a/src/app/components/Curation/HighImpactPromo/index.test.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.test.tsx @@ -12,18 +12,21 @@ const promoFixtureData = summaries?.[0] as HighImpactPromoProps; interface FixtureProps { promoData?: HighImpactPromoProps; headingLevel?: number; - attributions?: { title: string; link: { url: string } }[] | null; + attributions?: { title: string; link: { url: string } }[] | null | undefined; + relatedTopic?: { title: string; link: { url: string } } | null | undefined; } const Fixture = ({ promoData = promoFixtureData, headingLevel, attributions, + relatedTopic, }: FixtureProps) => ( ); @@ -123,4 +126,41 @@ describe('High Impact Promo', () => { const promo = screen.getByTestId('high-impact-promo'); expect(promo).toHaveAttribute('dir', dir); }); + + it('should render relatedTopic when provided', () => { + const relatedTopic = { + title: 'Россия', + link: { url: 'https://www.bbc.com/russian/topics/cw6eyw7m0m1t' }, + }; + render(); + + const relatedTopicLink = screen.getByRole('link', { + name: 'Россия', + }); + expect(relatedTopicLink).toBeInTheDocument(); + expect(relatedTopicLink).toHaveAttribute( + 'href', + 'https://www.bbc.com/russian/topics/cw6eyw7m0m1t', + ); + }); + + it('should prioritize relatedTopic over attributions when both are provided', () => { + const relatedTopic = { + title: 'Related Topic Title', + link: { url: '/related/path' }, + }; + const attributions = [ + { + title: 'Attribution Title', + link: { url: '/attribution/path' }, + }, + ]; + render(); + + const relatedTopicLink = screen.getByRole('link', { + name: 'Related Topic Title', + }); + expect(relatedTopicLink).toBeInTheDocument(); + expect(relatedTopicLink).toHaveAttribute('href', '/related/path'); + }); }); diff --git a/src/app/components/Curation/HighImpactPromo/index.tsx b/src/app/components/Curation/HighImpactPromo/index.tsx index c8d75ab8dcd..1b55d3dd149 100644 --- a/src/app/components/Curation/HighImpactPromo/index.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.tsx @@ -15,11 +15,12 @@ type AttributionLink = { }; type Attribution = { - title: string; link: AttributionLink; + title: string; }; export interface HighImpactPromoProps extends Summary { attributions?: Attribution[] | null; + relatedTopic?: Attribution | null; } const HighImpactPromo = ({ @@ -31,15 +32,23 @@ const HighImpactPromo = ({ headingLevel = 3, eventTrackingData, attributions, + relatedTopic, }: HighImpactPromoProps) => { const { isAmp } = use(RequestContext); const { dir, service, brandName } = use(ServiceContext) || {}; - const [firstAttribution] = attributions || []; - const attributionLink = - firstAttribution?.link?.url || (service ? getBrandPath(service) : null); - const attributionText = firstAttribution?.title || brandName; - const hasAttribution = Boolean(attributionLink && attributionText); + let subjectLink: string | undefined; + let subjectText: string | undefined; + if (relatedTopic?.link?.url && relatedTopic?.title) { + subjectLink = relatedTopic.link.url; + subjectText = relatedTopic.title; + } else { + subjectLink = + attributions?.[0]?.link?.url || + (service ? getBrandPath(service) : undefined); + subjectText = attributions?.[0]?.title || brandName; + } + const hasSubject = Boolean(subjectLink && subjectText); const clickTrackerHandler = useClickTrackerHandler(eventTrackingData); @@ -65,14 +74,14 @@ const HighImpactPromo = ({ {title} - {hasAttribution &&
} - {hasAttribution && attributionLink && ( + {hasSubject &&
} + {hasSubject && ( - {attributionText} + {subjectText} )}
From 0b9aedf2c052760814a8ce611563adcdb0d93f61 Mon Sep 17 00:00:00 2001 From: Lukas Freimonas Date: Tue, 24 Mar 2026 10:34:52 +0200 Subject: [PATCH 2/5] refactor: replace divider with margin styling in HighImpactPromo component --- .../Curation/HighImpactPromo/index.styles.ts | 22 +++++++++++-------- .../Curation/HighImpactPromo/index.test.tsx | 9 ++------ .../Curation/HighImpactPromo/index.tsx | 1 - 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/app/components/Curation/HighImpactPromo/index.styles.ts b/src/app/components/Curation/HighImpactPromo/index.styles.ts index dd698dcdfe2..39822d8b5e3 100644 --- a/src/app/components/Curation/HighImpactPromo/index.styles.ts +++ b/src/app/components/Curation/HighImpactPromo/index.styles.ts @@ -79,15 +79,6 @@ export default { }, }), - divider: ({ spacings }: Theme) => - css({ - marginTop: 'auto', - marginBottom: `${spacings.HALF + spacings.FULL}rem`, - height: `${pixelsToRem(3)}rem`, - width: `${pixelsToRem(40)}rem`, - background: '#EB0000', - }), - subject: ({ palette, fontSizes, fontVariants, spacings }: Theme) => css({ ...fontSizes.brevier, @@ -95,6 +86,19 @@ export default { color: palette.GREY_2, position: 'relative', alignSelf: 'flex-start', + marginTop: 'auto', + paddingTop: `${spacings.HALF + spacings.FULL}rem`, + + '&::after': { + content: '""', + display: 'block', + position: 'absolute', + top: 0, + insetInlineStart: 0, + height: `${pixelsToRem(3)}rem`, + width: `${pixelsToRem(40)}rem`, + background: '#EB0000', + }, '&:before': { content: '""', diff --git a/src/app/components/Curation/HighImpactPromo/index.test.tsx b/src/app/components/Curation/HighImpactPromo/index.test.tsx index 3febb7e0c6a..5ed751d0cbd 100644 --- a/src/app/components/Curation/HighImpactPromo/index.test.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.test.tsx @@ -75,13 +75,8 @@ describe('High Impact Promo', () => { }); expect(attributionLink).toBeInTheDocument(); expect(attributionLink).toHaveAttribute('href', '/mundo'); - - const divider = attributionLink.previousElementSibling; - expect(divider).toBeInTheDocument(); - expect(divider).toHaveStyle({ - 'background-color': '#EB0000', - width: '2.5rem', - height: '0.1875rem', + expect(attributionLink).toHaveStyle({ + 'margin-top': 'auto', }); }); diff --git a/src/app/components/Curation/HighImpactPromo/index.tsx b/src/app/components/Curation/HighImpactPromo/index.tsx index 1b55d3dd149..7fac6b63601 100644 --- a/src/app/components/Curation/HighImpactPromo/index.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.tsx @@ -74,7 +74,6 @@ const HighImpactPromo = ({ {title} - {hasSubject &&
} {hasSubject && ( Date: Tue, 24 Mar 2026 14:39:36 +0200 Subject: [PATCH 3/5] refactor: update HighImpactPromo stories to use relatedTopic and attributions props --- .../HighImpactPromo/index.stories.tsx | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/src/app/components/Curation/HighImpactPromo/index.stories.tsx b/src/app/components/Curation/HighImpactPromo/index.stories.tsx index 48fe2d3413e..4438f35f399 100644 --- a/src/app/components/Curation/HighImpactPromo/index.stories.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.stories.tsx @@ -5,56 +5,59 @@ import metadata from './metadata.json'; import readme from './README.md'; const highImpactFixtureCuration = fixture.data.curations[0] as BaseCuration; +const baseProps = highImpactFixtureCuration.summaries?.[0] as Summary; -const Component = () => { - return ( -
- - - -
- ); -}; +interface ExternalProps { + attributions?: { title: string; link: { url: string } }[] | null; + relatedTopic?: { title: string; link: { url: string } } | null; +} + +const Component = ({ attributions, relatedTopic }: ExternalProps) => ( + +); export default { title: 'Components/Curation/High Impact Promo', - component: Component, + Component, + decorators: [ + Story => ( +
+ +
+ ), + ], parameters: { metadata, docs: { readme }, chromatic: { disable: true }, }, + args: { + attributions: [ + { + title: 'BBC News Pidgin', + link: { url: '/pidgin' }, + }, + ], + relatedTopic: { + title: 'Related Topic Example', + link: { url: '/topic/example' }, + }, + }, + argTypes: { + attributions: { control: 'object' }, + relatedTopic: { control: 'object' }, + }, }; -export const Example = {}; +export const Example = Component; From 91c51e37d3224cded26a617c3c629b5bd93b5e51 Mon Sep 17 00:00:00 2001 From: Lukas Freimonas Date: Wed, 25 Mar 2026 11:09:58 +0200 Subject: [PATCH 4/5] refactor: remove attributions prop in HighImpactPromo component --- .../Curation/HighImpactPromo/README.md | 22 +++--- .../HighImpactPromo/index.stories.tsx | 16 +---- .../Curation/HighImpactPromo/index.test.tsx | 68 ++----------------- .../Curation/HighImpactPromo/index.tsx | 24 ++----- 4 files changed, 26 insertions(+), 104 deletions(-) diff --git a/src/app/components/Curation/HighImpactPromo/README.md b/src/app/components/Curation/HighImpactPromo/README.md index d37bef10a27..8d529e1a8b2 100644 --- a/src/app/components/Curation/HighImpactPromo/README.md +++ b/src/app/components/Curation/HighImpactPromo/README.md @@ -18,22 +18,22 @@ Typically used when editors set a curation item's prominence to "Maximum" in Tip - Renders promotional content with enhanced visual styling. - Integrates with the existing Standard Grid flow. - Provides clear visual distinction from standard curation items. -- Attribution is automatically derived from the service context (e.g., brand name and service URL) but can be overridden with the `attribution` prop. +- Subject link is derived from `relatedTopic` when present, otherwise it falls back to service context (brand name and service URL). --- ## Props -| Prop | Type | Required | Description | -| ------------------- | ------- | -------- | --------------------------------------------------------------------- | -| `title` | string | Yes | The promotional headline. | -| `link` | string | Yes | URL destination for the promo. | -| `imageUrl` | string | Yes | Image URL for the promotional content. | -| `imageAlt` | string | Yes | Alt text for the promotional image. | -| `lazy` | boolean | No | Enables lazy loading for the image. | -| `headingLevel` | number | No | The heading level for the title (defaults to 3). | -| `eventTrackingData` | object | No | Tracking metadata for analytics. | -| `attribution` | object | No | An object with `text` and `link` to override the default attribution. | +| Prop | Type | Required | Description | +| ------------------- | ------- | -------- | ---------------------------------------------------------------- | +| `title` | string | Yes | The promotional headline. | +| `link` | string | Yes | URL destination for the promo. | +| `imageUrl` | string | Yes | Image URL for the promotional content. | +| `imageAlt` | string | Yes | Alt text for the promotional image. | +| `lazy` | boolean | No | Enables lazy loading for the image. | +| `headingLevel` | number | No | The heading level for the title (defaults to 3). | +| `eventTrackingData` | object | No | Tracking metadata for analytics. | +| `relatedTopic` | object | No | An object with `title` and `link.url` used for the subject link. | --- diff --git a/src/app/components/Curation/HighImpactPromo/index.stories.tsx b/src/app/components/Curation/HighImpactPromo/index.stories.tsx index 4438f35f399..af2f8dad613 100644 --- a/src/app/components/Curation/HighImpactPromo/index.stories.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.stories.tsx @@ -8,16 +8,11 @@ const highImpactFixtureCuration = fixture.data.curations[0] as BaseCuration; const baseProps = highImpactFixtureCuration.summaries?.[0] as Summary; interface ExternalProps { - attributions?: { title: string; link: { url: string } }[] | null; relatedTopic?: { title: string; link: { url: string } } | null; } -const Component = ({ attributions, relatedTopic }: ExternalProps) => ( - +const Component = ({ relatedTopic }: ExternalProps) => ( + ); export default { @@ -43,19 +38,12 @@ export default { chromatic: { disable: true }, }, args: { - attributions: [ - { - title: 'BBC News Pidgin', - link: { url: '/pidgin' }, - }, - ], relatedTopic: { title: 'Related Topic Example', link: { url: '/topic/example' }, }, }, argTypes: { - attributions: { control: 'object' }, relatedTopic: { control: 'object' }, }, }; diff --git a/src/app/components/Curation/HighImpactPromo/index.test.tsx b/src/app/components/Curation/HighImpactPromo/index.test.tsx index 5ed751d0cbd..a24fc8e31f4 100644 --- a/src/app/components/Curation/HighImpactPromo/index.test.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.test.tsx @@ -12,20 +12,17 @@ const promoFixtureData = summaries?.[0] as HighImpactPromoProps; interface FixtureProps { promoData?: HighImpactPromoProps; headingLevel?: number; - attributions?: { title: string; link: { url: string } }[] | null | undefined; relatedTopic?: { title: string; link: { url: string } } | null | undefined; } const Fixture = ({ promoData = promoFixtureData, headingLevel, - attributions, relatedTopic, }: FixtureProps) => ( ); @@ -67,52 +64,19 @@ describe('High Impact Promo', () => { ); }); - it('should render default values if attribution prop is not provided', () => { + it('should render default subject values when relatedTopic prop is not provided', () => { render(, { service: 'mundo' }); - const attributionLink = screen.getByRole('link', { + const subjectLink = screen.getByRole('link', { name: 'BBC News Mundo', }); - expect(attributionLink).toBeInTheDocument(); - expect(attributionLink).toHaveAttribute('href', '/mundo'); - expect(attributionLink).toHaveStyle({ + expect(subjectLink).toBeInTheDocument(); + expect(subjectLink).toHaveAttribute('href', '/mundo'); + expect(subjectLink).toHaveStyle({ 'margin-top': 'auto', }); }); - it('should render correct attribution when an attributions prop is provided', () => { - const customAttributions = [ - { - title: 'Pidgin Related Topic', - link: { url: '/pidgin/topics/234567' }, - }, - ]; - render(); - - const attributionLink = screen.getByRole('link', { - name: 'Pidgin Related Topic', - }); - expect(attributionLink).toBeInTheDocument(); - expect(attributionLink).toHaveAttribute('href', '/pidgin/topics/234567'); - }); - it('should render default attribution when attributions prop is null', () => { - render(, { service: 'mundo' }); - const attributionLink = screen.getByRole('link', { - name: 'BBC News Mundo', - }); - expect(attributionLink).toBeInTheDocument(); - expect(attributionLink).toHaveAttribute('href', '/mundo'); - }); - - it('should render default attribution when attributions prop is an empty array', () => { - render(, { service: 'mundo' }); - const attributionLink = screen.getByRole('link', { - name: 'BBC News Mundo', - }); - expect(attributionLink).toBeInTheDocument(); - expect(attributionLink).toHaveAttribute('href', '/mundo'); - }); - it.each<[Services, string]>([ ['mundo', 'ltr'], ['arabic', 'rtl'], @@ -127,7 +91,7 @@ describe('High Impact Promo', () => { title: 'Россия', link: { url: 'https://www.bbc.com/russian/topics/cw6eyw7m0m1t' }, }; - render(); + render(); const relatedTopicLink = screen.getByRole('link', { name: 'Россия', @@ -138,24 +102,4 @@ describe('High Impact Promo', () => { 'https://www.bbc.com/russian/topics/cw6eyw7m0m1t', ); }); - - it('should prioritize relatedTopic over attributions when both are provided', () => { - const relatedTopic = { - title: 'Related Topic Title', - link: { url: '/related/path' }, - }; - const attributions = [ - { - title: 'Attribution Title', - link: { url: '/attribution/path' }, - }, - ]; - render(); - - const relatedTopicLink = screen.getByRole('link', { - name: 'Related Topic Title', - }); - expect(relatedTopicLink).toBeInTheDocument(); - expect(relatedTopicLink).toHaveAttribute('href', '/related/path'); - }); }); diff --git a/src/app/components/Curation/HighImpactPromo/index.tsx b/src/app/components/Curation/HighImpactPromo/index.tsx index 7fac6b63601..4b688d4d96f 100644 --- a/src/app/components/Curation/HighImpactPromo/index.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.tsx @@ -7,20 +7,19 @@ import { ServiceContext } from '#app/contexts/ServiceContext'; import { getBrandPath } from '#app/legacy/containers/Brand'; import styles from './index.styles'; -type AttributionLink = { +type RelatedTopicLink = { url: string; scheme?: string; host?: string; path?: string; }; -type Attribution = { - link: AttributionLink; +type RelatedTopic = { + link: RelatedTopicLink; title: string; }; export interface HighImpactPromoProps extends Summary { - attributions?: Attribution[] | null; - relatedTopic?: Attribution | null; + relatedTopic?: RelatedTopic | null; } const HighImpactPromo = ({ @@ -31,23 +30,14 @@ const HighImpactPromo = ({ link, headingLevel = 3, eventTrackingData, - attributions, relatedTopic, }: HighImpactPromoProps) => { const { isAmp } = use(RequestContext); const { dir, service, brandName } = use(ServiceContext) || {}; - let subjectLink: string | undefined; - let subjectText: string | undefined; - if (relatedTopic?.link?.url && relatedTopic?.title) { - subjectLink = relatedTopic.link.url; - subjectText = relatedTopic.title; - } else { - subjectLink = - attributions?.[0]?.link?.url || - (service ? getBrandPath(service) : undefined); - subjectText = attributions?.[0]?.title || brandName; - } + const subjectLink = + relatedTopic?.link?.url || (service ? getBrandPath(service) : undefined); + const subjectText = relatedTopic?.title || brandName; const hasSubject = Boolean(subjectLink && subjectText); const clickTrackerHandler = useClickTrackerHandler(eventTrackingData); From d1173e7376ba4d64cac2331c16f3bc27a5d3c52b Mon Sep 17 00:00:00 2001 From: Lukas Freimonas Date: Wed, 25 Mar 2026 11:51:13 +0200 Subject: [PATCH 5/5] revert "refactor: replace divider with margin styling in HighImpactPromo component" --- .../Curation/HighImpactPromo/index.styles.ts | 22 ++++++++----------- .../Curation/HighImpactPromo/index.test.tsx | 9 ++++++-- .../Curation/HighImpactPromo/index.tsx | 1 + 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/app/components/Curation/HighImpactPromo/index.styles.ts b/src/app/components/Curation/HighImpactPromo/index.styles.ts index 39822d8b5e3..dd698dcdfe2 100644 --- a/src/app/components/Curation/HighImpactPromo/index.styles.ts +++ b/src/app/components/Curation/HighImpactPromo/index.styles.ts @@ -79,6 +79,15 @@ export default { }, }), + divider: ({ spacings }: Theme) => + css({ + marginTop: 'auto', + marginBottom: `${spacings.HALF + spacings.FULL}rem`, + height: `${pixelsToRem(3)}rem`, + width: `${pixelsToRem(40)}rem`, + background: '#EB0000', + }), + subject: ({ palette, fontSizes, fontVariants, spacings }: Theme) => css({ ...fontSizes.brevier, @@ -86,19 +95,6 @@ export default { color: palette.GREY_2, position: 'relative', alignSelf: 'flex-start', - marginTop: 'auto', - paddingTop: `${spacings.HALF + spacings.FULL}rem`, - - '&::after': { - content: '""', - display: 'block', - position: 'absolute', - top: 0, - insetInlineStart: 0, - height: `${pixelsToRem(3)}rem`, - width: `${pixelsToRem(40)}rem`, - background: '#EB0000', - }, '&:before': { content: '""', diff --git a/src/app/components/Curation/HighImpactPromo/index.test.tsx b/src/app/components/Curation/HighImpactPromo/index.test.tsx index a24fc8e31f4..f22d0850900 100644 --- a/src/app/components/Curation/HighImpactPromo/index.test.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.test.tsx @@ -72,8 +72,13 @@ describe('High Impact Promo', () => { }); expect(subjectLink).toBeInTheDocument(); expect(subjectLink).toHaveAttribute('href', '/mundo'); - expect(subjectLink).toHaveStyle({ - 'margin-top': 'auto', + + const divider = subjectLink.previousElementSibling; + expect(divider).toBeInTheDocument(); + expect(divider).toHaveStyle({ + 'background-color': '#EB0000', + width: '2.5rem', + height: '0.1875rem', }); }); diff --git a/src/app/components/Curation/HighImpactPromo/index.tsx b/src/app/components/Curation/HighImpactPromo/index.tsx index 4b688d4d96f..67b9d96b456 100644 --- a/src/app/components/Curation/HighImpactPromo/index.tsx +++ b/src/app/components/Curation/HighImpactPromo/index.tsx @@ -64,6 +64,7 @@ const HighImpactPromo = ({ {title} + {hasSubject &&
} {hasSubject && (