WS-1883 - Bring Article Links Block up to component standards #13560
Conversation
…ponent-standards
…rds' of ssh://github.com/bbc/simorgh into WS-1883-bring-article-link-block-up-to-component-standards
This reverts commit cb15ae0.
…ponent-standards
| const title = | ||
| textBlock?.model?.blocks?.[0]?.model?.blocks?.[0]?.model?.text ?? ''; | ||
|
|
||
| const isOperaMini = useOperaMiniDetection(); |
There was a problem hiding this comment.
We can't rely on the useOperaMiniDetection hook anymore unfortunately. Instead, we have a special Opera Mini CSS class name that can be used directly in the stylesheet to change the display if its Opera Mini, e.g:
simorgh/src/app/components/PortraitVideoCarousel/index.styles.ts
Lines 16 to 18 in 705fb7f
| )} | ||
| {isSingleItem ? ( | ||
| <div css={styles.promoContainer} dir={dir} {...viewTracker}> | ||
| <Promo block={blocksWithoutTitle[0]} {...clickTracker} /> |
There was a problem hiding this comment.
Are the clickTracker props being used here and in PromoList? I can only see ({ blocks, eventTrackingData }) as the props for both of those components. May be a cause for the multiple clicks? I can see the clickTracker being defined again at https://github.com/bbc/simorgh/pull/13560/files#diff-c107cd281b9228bf828ec21a61ff874354a2bcd795f71f324d56206190fc14eaR18 inside Promo, so I think we should keep it there and remove it from here?
There was a problem hiding this comment.
So just to be clear, we are wanting to remove the clickTracker props for both Promo and PromoList within src/app/components/ArticleLinksBlock/index.tsx, but keep it within the promo file?
There was a problem hiding this comment.
Yea I think it should be removed from ArticleLinksBlock. I can't see it actually being used as a prop in Promo or PromoList.
| ) : ( | ||
| <PromoList | ||
| blocks={blocksWithoutTitle} | ||
| {...viewTracker} |
There was a problem hiding this comment.
Similar here, is viewTracker used? I can't see it as a prop in PromoList.
There was a problem hiding this comment.
Not sure, I only see blocks, eventTrackingData as props so perhaps can be removed? Maybe one for @louisearchibald?
| clickTracker={clickTracker} | ||
| a11yAttributes={a11yAttributes} |
There was a problem hiding this comment.
Same with these, can't see them used in PromoList itself
| }; | ||
|
|
||
| return ( | ||
| <GridItemMediumNoMargin {...a11yAttributes} data-e2e="article-links-block"> |
There was a problem hiding this comment.
We tend to avoid using the old 'grid' components in new components. Can we achieve the desired layout without using this?
Resolves JIRA: WS-1883
Summary
Bringing the
ScrollablePromocomponent out of legacy and in line with our modern component standards. We have renamed the component toArticleLinksBlockas we will also be replacing the existing scrollable cards styling entirely and replacing with a links block styling instead as part of some future work.Code changes
ScrollablePromotoArticleLinksBlock.Developer Checklist
Testing
Ready-For-Test, Local)Ready-For-Test, Test)Ready-For-Test, Preview)Ready-For-Test, Live)Additional Testing Steps
Useful Links