-
Notifications
You must be signed in to change notification settings - Fork 30
Include ArticleBody in HostedArticleLayout
#15379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { | |
| headlineBold24, | ||
| remSpace, | ||
| textEgyptian17, | ||
| textSansBold20, | ||
| } from '@guardian/source/foundations'; | ||
| import { | ||
| ArticleDesign, | ||
|
|
@@ -73,6 +74,16 @@ const globalOlStyles = () => css` | |
| } | ||
| `; | ||
|
|
||
| const hostedContentH2Styles = (design: ArticleDesign) => css` | ||
| ${design === ArticleDesign.HostedArticle && | ||
| ` | ||
| h2 { | ||
| ${textSansBold20}; | ||
| margin-bottom: ${remSpace[2]}; | ||
| } | ||
| `} | ||
| `; | ||
|
|
||
| const globalH3Styles = (display: ArticleDisplay) => css` | ||
| ${display === ArticleDisplay.NumberedList && | ||
| ` | ||
|
|
@@ -145,6 +156,7 @@ export const ArticleBody = ({ | |
| const isInteractiveContent = | ||
| format.design === ArticleDesign.Interactive || | ||
| format.design === ArticleDesign.Crossword; | ||
| const isHostedContent = format.design === ArticleDesign.HostedArticle; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should include the other hosted types in here too? Even though they won't ever actually render the ArticleBody, it would be better to be thorough |
||
| const language = decideLanguage(lang); | ||
| const languageDirection = decideLanguageDirection(isRightToLeftLang); | ||
| const hasObserverPublicationTag = tags.find( | ||
|
|
@@ -230,6 +242,7 @@ export const ArticleBody = ({ | |
| css={[ | ||
| `margin-top: ${remSpace[3]}`, | ||
| isInteractiveContent ? null : bodyPadding, | ||
| isHostedContent && hostedContentH2Styles(format.design), | ||
| globalH3Styles(format.display), | ||
| globalOlStyles(), | ||
| globalStrongStyles, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,9 @@ import { | |
| palette as sourcePalette, | ||
| space, | ||
| } from '@guardian/source/foundations'; | ||
| import { StraightLines } from '@guardian/source-development-kitchen/react-components'; | ||
| import { ArticleBody } from '../components/ArticleBody'; | ||
| import { ArticleContainer } from '../components/ArticleContainer'; | ||
| import { ArticleHeadline } from '../components/ArticleHeadline'; | ||
| import { CallToActionAtom } from '../components/CallToActionAtom'; | ||
| import { Caption } from '../components/Caption'; | ||
|
|
@@ -15,7 +18,9 @@ import { ShareButton } from '../components/ShareButton.importable'; | |
| import { Standfirst } from '../components/Standfirst'; | ||
| import { grid } from '../grid'; | ||
| import type { ArticleFormat } from '../lib/articleFormat'; | ||
| import { getContributionsServiceUrl } from '../lib/contributions'; | ||
| import { decideMainMediaCaption } from '../lib/decide-caption'; | ||
| import { palette as themePalette } from '../palette'; | ||
| import type { Article } from '../types/article'; | ||
| import type { RenderingTarget } from '../types/renderingTarget'; | ||
| import { Stuck } from './lib/stickiness'; | ||
|
|
@@ -59,6 +64,8 @@ export const HostedArticleLayout = (props: WebProps | AppProps) => { | |
| format, | ||
| } = props; | ||
|
|
||
| const contributionsServiceUrl = getContributionsServiceUrl(frontendData); | ||
|
|
||
| const mainMedia = frontendData.mainMediaElements[0]; | ||
| const mainMediaCaptionText = decideMainMediaCaption(mainMedia); | ||
|
|
||
|
|
@@ -171,35 +178,80 @@ export const HostedArticleLayout = (props: WebProps | AppProps) => { | |
| </div> | ||
| <div | ||
| css={[ | ||
| grid.column.right, | ||
| grid.column.centre, | ||
| css` | ||
| grid-row: 1; | ||
| `, | ||
| ]} | ||
| > | ||
| <Caption | ||
| captionText={mainMediaCaptionText} | ||
| <Standfirst | ||
| format={format} | ||
| isMainMedia={true} | ||
| standfirst={frontendData.standfirst} | ||
| /> | ||
| </div> | ||
| <div id="maincontent" css={[grid.column.centre]}> | ||
| <ArticleContainer format={format}> | ||
| <ArticleBody | ||
| format={format} | ||
| blocks={frontendData.blocks} | ||
| editionId={frontendData.editionId} | ||
| host={frontendData.config.host} | ||
| pageId={frontendData.pageId} | ||
| webTitle={frontendData.webTitle} | ||
| ajaxUrl={frontendData.config.ajaxUrl} | ||
| isAdFreeUser={frontendData.isAdFreeUser} | ||
| switches={frontendData.config.switches} | ||
| sectionId={frontendData.config.section} | ||
| shouldHideReaderRevenue={ | ||
| frontendData.shouldHideReaderRevenue | ||
| } | ||
| tags={frontendData.tags} | ||
| isPaidContent={ | ||
| !!frontendData.config.isPaidContent | ||
| } | ||
| contributionsServiceUrl={ | ||
| contributionsServiceUrl | ||
| } | ||
| contentType={frontendData.contentType} | ||
| idUrl={frontendData.config.idUrl ?? ''} | ||
| isSensitive={ | ||
| frontendData.config.isSensitive | ||
| } | ||
| isDev={!!frontendData.config.isDev} | ||
| keywordIds={frontendData.config.keywordIds} | ||
| abTests={frontendData.config.abTests} | ||
| shouldHideAds={frontendData.shouldHideAds} | ||
| /> | ||
| </ArticleContainer> | ||
| <StraightLines | ||
| data-print-layout="hide" | ||
| count={1} | ||
| cssOverrides={css` | ||
| display: block; | ||
| `} | ||
| color={themePalette('--straight-lines')} | ||
| /> | ||
|
|
||
| {'Onward content'} | ||
| </div> | ||
| <div | ||
| css={[ | ||
| grid.column.centre, | ||
| css` | ||
| grid-row: 1; | ||
| grid-row: auto; | ||
|
|
||
| ${from.leftCol} { | ||
| ${grid.column.right} | ||
| grid-row: 1; | ||
| } | ||
| `, | ||
| ]} | ||
| > | ||
| <Standfirst | ||
| <Caption | ||
| captionText={mainMediaCaptionText} | ||
| format={format} | ||
| standfirst={frontendData.standfirst} | ||
| isMainMedia={true} | ||
|
Comment on lines
+248
to
+251
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logically it feels strange to have the caption for the main media so far down in the DOM. Maybe this could go above the article body? |
||
| /> | ||
| </div> | ||
| <div id="maincontent" css={[grid.column.centre]}> | ||
| {'body'} | ||
|
|
||
| {'Onward content'} | ||
| </div> | ||
| </div> | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could make this pure CSS rather than a function since we already have the name of the styles in the function name,
hostedContentH2Styles. We can conditionally render these styles if it is hosted contentSo instead of
we could do the following: