Skip to content

Commit

Permalink
Extract showComments logic for consistency (#9634)
Browse files Browse the repository at this point in the history
* Don't show comments or most viewed on apps

* Ensure we are using the extracted showComments across layouts

* We should be showing most viewed after all

* Extract isPaidContent condition into the showComments const
  • Loading branch information
sophie-macmillan authored Nov 27, 2023
1 parent c3d2e8a commit 6951efb
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 150 deletions.
29 changes: 16 additions & 13 deletions dotcom-rendering/src/layouts/CommentLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,20 +274,23 @@ interface AppsProps extends CommonProps {

export const CommentLayout = (props: WebProps | AppsProps) => {
const { article, format, renderingTarget } = props;
const isWeb = renderingTarget === 'Web';
const isApps = renderingTarget === 'Apps';
const {
config: { isPaidContent, host },
} = article;

const showBodyEndSlot =
renderingTarget === 'Web' &&
isWeb &&
(parse(article.slotMachineFlags ?? '').showBodyEnd ||
article.config.switches.slotBodyEnd);

// TODO:
// 1) Read 'forceEpic' value from URL parameter and use it to force the slot to render
// 2) Otherwise, ensure slot only renders if `article.config.shouldHideReaderRevenue` equals false.

const showComments = article.isCommentable;
/** Mobile articles with comments should be filtered in MAPI but we leave this in for clarity **/
const showComments = isWeb && article.isCommentable && !isPaidContent;

const avatarUrl = getSoleContributor(article.tags, article.byline)
?.bylineLargeImageUrl;
Expand All @@ -296,11 +299,11 @@ export const CommentLayout = (props: WebProps | AppsProps) => {

const contributionsServiceUrl = getContributionsServiceUrl(article);

const renderAds = renderingTarget === 'Web' && canRenderAds(article);
const renderAds = isWeb && canRenderAds(article);

return (
<>
{renderingTarget === 'Web' && (
{isWeb && (
<div id="bannerandheader">
{renderAds && (
<Stuck>
Expand Down Expand Up @@ -434,7 +437,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
)}

<main data-layout="CommentLayout">
{renderingTarget === 'Apps' && (
{isApps && (
<Island priority="critical">
<AdPortals />
</Island>
Expand Down Expand Up @@ -574,7 +577,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={article.isCommentable}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -686,7 +689,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
webTitle={article.webTitle}
showBottomSocialButtons={
article.showBottomSocialButtons &&
renderingTarget === 'Web'
isWeb
}
badge={article.badge?.enhanced}
/>
Expand Down Expand Up @@ -759,7 +762,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
</Section>
)}

{renderingTarget === 'Web' && (
{isWeb && (
<Island priority="feature" defer={{ until: 'visible' }}>
<OnwardsUpper
ajaxUrl={article.config.ajaxUrl}
Expand All @@ -783,7 +786,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
</Island>
)}

{!isPaidContent && showComments && (
{showComments && (
<Section
fullWidth={true}
sectionId="comments"
Expand All @@ -807,7 +810,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
</Section>
)}

{renderingTarget === 'Web' && !isPaidContent && (
{!isPaidContent && (
<Section
title="Most viewed"
padContent={false}
Expand Down Expand Up @@ -850,7 +853,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
)}
</main>

{renderingTarget === 'Web' && props.NAV.subNavSections && (
{isWeb && props.NAV.subNavSections && (
<Section fullWidth={true} padSides={false} element="aside">
<Island priority="enhancement" defer={{ until: 'visible' }}>
<SubNav
Expand All @@ -864,7 +867,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
</Island>
</Section>
)}
{renderingTarget === 'Web' && !isPaidContent && (
{isWeb && !isPaidContent && (
<>
<Section
fullWidth={true}
Expand Down Expand Up @@ -919,7 +922,7 @@ export const CommentLayout = (props: WebProps | AppsProps) => {
<MobileStickyContainer />
</>
)}
{renderingTarget === 'Apps' && (
{isApps && (
<Section
fullWidth={true}
data-print-layout="hide"
Expand Down
30 changes: 16 additions & 14 deletions dotcom-rendering/src/layouts/ImmersiveLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,20 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
const {
config: { isPaidContent, host },
} = article;
const isWeb = renderingTarget === 'Web';
const isApps = renderingTarget === 'Apps';

const showBodyEndSlot =
renderingTarget === 'Web' &&
isWeb &&
(parse(article.slotMachineFlags ?? '').showBodyEnd ||
article.config.switches.slotBodyEnd);

// TODO:
// 1) Read 'forceEpic' value from URL parameter and use it to force the slot to render
// 2) Otherwise, ensure slot only renders if `article.config.shouldHideReaderRevenue` equals false.

const showComments = article.isCommentable;
/** Mobile articles with comments should be filtered in MAPI but we leave this in for clarity **/
const showComments = isWeb && article.isCommentable && !isPaidContent;

const mainMedia = article.mainMediaElements[0];

Expand Down Expand Up @@ -301,11 +304,11 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
</div>
);

const renderAds = renderingTarget === 'Web' && canRenderAds(article);
const renderAds = isWeb && canRenderAds(article);

return (
<>
{renderingTarget === 'Web' && (
{isWeb && (
<>
<div
css={css`
Expand Down Expand Up @@ -453,7 +456,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
</header>

<main data-layout="ImmersiveLayout">
{renderingTarget === 'Apps' && (
{isApps && (
<Island priority="critical">
<AdPortals />
</Island>
Expand Down Expand Up @@ -585,7 +588,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={article.isCommentable}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -682,8 +685,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
webUrl={article.webURL}
webTitle={article.webTitle}
showBottomSocialButtons={
article.showBottomSocialButtons &&
renderingTarget === 'Web'
article.showBottomSocialButtons && isWeb
}
badge={article.badge?.enhanced}
/>
Expand Down Expand Up @@ -770,7 +772,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
</Section>
)}

{renderingTarget === 'Web' && (
{isWeb && (
<Island priority="feature" defer={{ until: 'visible' }}>
<OnwardsUpper
ajaxUrl={article.config.ajaxUrl}
Expand All @@ -796,7 +798,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
</Island>
)}

{!isPaidContent && showComments && (
{showComments && (
<Section
fullWidth={true}
sectionId="comments"
Expand All @@ -819,7 +821,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
/>
</Section>
)}
{renderingTarget === 'Web' && !isPaidContent && (
{!isPaidContent && (
<Section
title="Most viewed"
padContent={false}
Expand Down Expand Up @@ -861,7 +863,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
)}
</main>

{renderingTarget === 'Web' && props.NAV.subNavSections && (
{isWeb && props.NAV.subNavSections && (
<Section fullWidth={true} padSides={false} element="aside">
<Island priority="enhancement" defer={{ until: 'visible' }}>
<SubNav
Expand All @@ -876,7 +878,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
</Section>
)}

{renderingTarget === 'Web' && (
{isWeb && (
<>
<Section
fullWidth={true}
Expand Down Expand Up @@ -931,7 +933,7 @@ export const ImmersiveLayout = (props: WebProps | AppProps) => {
{renderAds && <MobileStickyContainer />}
</>
)}
{renderingTarget === 'Apps' && (
{isApps && (
<Section
fullWidth={true}
data-print-layout="hide"
Expand Down
12 changes: 7 additions & 5 deletions dotcom-rendering/src/layouts/InteractiveLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,16 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => {
config: { isPaidContent, host },
} = article;

const showComments = article.isCommentable;
const isApps = renderingTarget === 'Apps';
const isWeb = renderingTarget === 'Web';

/** Mobile articles with comments should be filtered in MAPI but we leave this in for clarity **/
const showComments = isWeb && article.isCommentable && !isPaidContent;

const { branding } = article.commercialProperties[article.editionId];

const contributionsServiceUrl = getContributionsServiceUrl(article);

const isApps = renderingTarget === 'Apps';
const isWeb = renderingTarget === 'Web';
/**
* This property currently only applies to the header and merchandising slots
*/
Expand Down Expand Up @@ -508,7 +510,7 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={article.isCommentable}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -689,7 +691,7 @@ export const InteractiveLayout = (props: WebProps | AppsProps) => {
/>
</Island>

{!isPaidContent && showComments && (
{showComments && (
<Section
fullWidth={true}
sectionId="comments"
Expand Down
11 changes: 6 additions & 5 deletions dotcom-rendering/src/layouts/LiveLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,9 @@ export const LiveLayout = (props: WebProps | AppsProps) => {
const isWeb = renderingTarget === 'Web';
const isApps = renderingTarget === 'Apps';

/** Mobile articles with comments should be filtered in MAPI but we leave this in for clarity **/
const showComments = isWeb && article.isCommentable && !isPaidContent;

const isInLiveblogAdSlotTest =
article.config.abTests.serverSideLiveblogInlineAdsVariant === 'variant';

Expand Down Expand Up @@ -584,7 +587,7 @@ export const LiveLayout = (props: WebProps | AppsProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={article.isCommentable}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -748,9 +751,7 @@ export const LiveLayout = (props: WebProps | AppsProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={
article.isCommentable
}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -1217,7 +1218,7 @@ export const LiveLayout = (props: WebProps | AppsProps) => {
/>
</Island>

{!isPaidContent && article.isCommentable && (
{showComments && (
<Section
fullWidth={true}
showTopBorder={false}
Expand Down
9 changes: 5 additions & 4 deletions dotcom-rendering/src/layouts/PictureLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ export const PictureLayout = (props: WebProps | AppsProps) => {
// 1) Read 'forceEpic' value from URL parameter and use it to force the slot to render
// 2) Otherwise, ensure slot only renders if `article.config.shouldHideReaderRevenue` equals false.

const showComments = article.isCommentable;
/** Mobile articles with comments should be filtered in MAPI but we leave this in for clarity **/
const showComments = isWeb && article.isCommentable && !isPaidContent;

const { branding } = article.commercialProperties[article.editionId];

Expand Down Expand Up @@ -559,7 +560,7 @@ export const PictureLayout = (props: WebProps | AppsProps) => {
secondaryDateline={
article.webPublicationSecondaryDateDisplay
}
isCommentable={article.isCommentable}
isCommentable={showComments}
discussionApiUrl={
article.config.discussionApiUrl
}
Expand Down Expand Up @@ -655,7 +656,7 @@ export const PictureLayout = (props: WebProps | AppsProps) => {
/>
</Island>
)}
{!isPaidContent && showComments && (
{showComments && (
<Section
fullWidth={true}
sectionId="comments"
Expand All @@ -679,7 +680,7 @@ export const PictureLayout = (props: WebProps | AppsProps) => {
</Section>
)}

{isWeb && !isPaidContent && (
{!isPaidContent && (
<Section
title="Most viewed"
padContent={false}
Expand Down
Loading

0 comments on commit 6951efb

Please sign in to comment.