WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit f6b1812

Browse files
JamieB-guJamieB-gu
authored
Gallery Sub-Components: Body (#14626)
Refactoring the `GalleryLayout` into sub-components, to make it easier to read. ESLint reports that a function doesn't have a `return` statement, but it does. It's possible ESLint thinks the switch might not be exhaustive, but it is and TypeScript is able to verify this. Therefore it may be an issue with the integration between TS and ESLint.
1 parent 07e7546 commit f6b1812

File tree

1 file changed

+142
-110
lines changed

1 file changed

+142
-110
lines changed

‎dotcom-rendering/src/layouts/GalleryLayout.tsx‎

Lines changed: 142 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
space,
77
} from '@guardian/source/foundations';
88
import { Hide } from '@guardian/source/react-components';
9-
import { Fragment } from 'react';
109
import { AdPlaceholder } from '../components/AdPlaceholder.apps';
1110
import { AdPortals } from '../components/AdPortals.importable';
1211
import { AdSlot } from '../components/AdSlot.web';
@@ -75,61 +74,6 @@ const headerStyles = css`
7574
}
7675
`;
7776

78-
const galleryItemAdvertStyles = css`
79-
${grid.paddedContainer}
80-
grid-auto-flow: row dense;
81-
background-color: ${palette('--article-inner-background')};
82-
83-
${from.tablet} {
84-
border-left: 1px solid ${palette('--article-border')};
85-
border-right: 1px solid ${palette('--article-border')};
86-
}
87-
`;
88-
89-
const galleryInlineAdContainerStyles = css`
90-
${grid.column.centre}
91-
z-index: 1;
92-
93-
${from.desktop} {
94-
padding-bottom: ${space[10]}px;
95-
}
96-
97-
${from.leftCol} {
98-
${grid.between('centre-column-start', 'right-column-end')}
99-
}
100-
`;
101-
102-
const galleryBorder = css`
103-
position: relative;
104-
${between.desktop.and.leftCol} {
105-
${grid.column.right}
106-
107-
&::before {
108-
content: '';
109-
position: absolute;
110-
left: -10px; /* 10px to the left of this element */
111-
top: 0;
112-
bottom: 0;
113-
width: 1px;
114-
background-color: ${palette('--article-border')};
115-
}
116-
}
117-
118-
${from.leftCol} {
119-
${grid.column.left}
120-
121-
&::after {
122-
content: '';
123-
position: absolute;
124-
right: -10px;
125-
top: 0;
126-
bottom: 0;
127-
width: 1px;
128-
background-color: ${palette('--article-border')};
129-
}
130-
}
131-
`;
132-
13377
export const GalleryLayout = (props: WebProps | AppProps) => {
13478
const { gallery, renderingTarget } = props;
13579

@@ -233,60 +177,14 @@ export const GalleryLayout = (props: WebProps | AppProps) => {
233177
frontendData={frontendData}
234178
/>
235179
</header>
236-
{gallery.bodyElements.map((element, index) => {
237-
const isImage =
238-
element._type ===
239-
'model.dotcomrendering.pageElements.ImageBlockElement';
240-
const isAdPlaceholder =
241-
element._type ===
242-
'model.dotcomrendering.pageElements.AdPlaceholderBlockElement';
243-
return (
244-
<Fragment key={isImage ? element.elementId : index}>
245-
{isImage && (
246-
<GalleryImage
247-
image={element}
248-
format={format}
249-
pageId={frontendData.pageId}
250-
webTitle={frontendData.webTitle}
251-
renderingTarget={props.renderingTarget}
252-
/>
253-
)}
254-
{isAdPlaceholder && renderAds && (
255-
<>
256-
{isWeb && (
257-
<div css={galleryItemAdvertStyles}>
258-
<div
259-
css={
260-
galleryInlineAdContainerStyles
261-
}
262-
>
263-
<Hide until="tablet">
264-
<DesktopAdSlot
265-
renderAds={renderAds}
266-
adSlotIndex={
267-
element.adPosition
268-
}
269-
/>
270-
</Hide>
271-
<Hide from="tablet">
272-
<MobileAdSlot
273-
renderAds={renderAds}
274-
adSlotIndex={
275-
element.adPosition
276-
}
277-
/>
278-
</Hide>
279-
</div>
280-
<div css={galleryBorder}></div>
281-
</div>
282-
)}
283-
{isApps && <AdPlaceholder />}
284-
</>
285-
)}
286-
</Fragment>
287-
);
288-
})}
289-
180+
<Body
181+
renderingTarget={renderingTarget}
182+
format={format}
183+
bodyElements={gallery.bodyElements}
184+
renderAds={renderAds}
185+
pageId={frontendData.pageId}
186+
webTitle={frontendData.webTitle}
187+
/>
290188
<SubMeta
291189
format={format}
292190
subMetaKeywordLinks={frontendData.subMetaKeywordLinks}
@@ -602,6 +500,140 @@ const Meta = ({
602500
</div>
603501
);
604502

503+
const Body = (props: {
504+
renderingTarget: RenderingTarget;
505+
format: ArticleFormat;
506+
bodyElements: Gallery['bodyElements'];
507+
renderAds: boolean;
508+
pageId: string;
509+
webTitle: string;
510+
}) => (
511+
<>
512+
{props.bodyElements
513+
// Filter out ad elements if we don't want to render them.
514+
.filter(
515+
(element) =>
516+
props.renderAds ||
517+
element._type !==
518+
'model.dotcomrendering.pageElements.AdPlaceholderBlockElement',
519+
)
520+
/* eslint-disable-next-line array-callback-return -- ESLint bug,
521+
* this function does contain `return` statements. TypeScript will
522+
* confirm the switch is exhaustive, but it's possible ESLint does
523+
* not know this. */
524+
.map((element) => {
525+
switch (element._type) {
526+
case 'model.dotcomrendering.pageElements.ImageBlockElement':
527+
return (
528+
<GalleryImage
529+
image={element}
530+
format={props.format}
531+
pageId={props.pageId}
532+
webTitle={props.webTitle}
533+
renderingTarget={props.renderingTarget}
534+
key={element.elementId}
535+
/>
536+
);
537+
case 'model.dotcomrendering.pageElements.AdPlaceholderBlockElement':
538+
return (
539+
<BodyAdSlot
540+
renderingTarget={props.renderingTarget}
541+
adIndex={element.adPosition}
542+
key={element.adPosition}
543+
/>
544+
);
545+
}
546+
})}
547+
</>
548+
);
549+
550+
const BodyAdSlot = (props: {
551+
renderingTarget: RenderingTarget;
552+
adIndex: number;
553+
}) => {
554+
switch (props.renderingTarget) {
555+
case 'Web':
556+
return <WebAdSlot adIndex={props.adIndex} />;
557+
case 'Apps':
558+
return <AdPlaceholder />;
559+
}
560+
};
561+
562+
const WebAdSlot = (props: { adIndex: number }) => (
563+
<div
564+
css={{
565+
'&': css(grid.paddedContainer),
566+
gridAutoFlow: 'row dense',
567+
backgroundColor: palette('--article-inner-background'),
568+
569+
[from.tablet]: {
570+
borderColor: palette('--article-border'),
571+
borderStyle: 'solid',
572+
borderLeftWidth: 1,
573+
borderRightWidth: 1,
574+
},
575+
}}
576+
>
577+
<div
578+
css={{
579+
'&': css(grid.column.centre),
580+
zIndex: 1,
581+
582+
[from.desktop]: {
583+
paddingBottom: space[10],
584+
},
585+
586+
[from.leftCol]: css(
587+
grid.between('centre-column-start', 'right-column-end'),
588+
),
589+
}}
590+
>
591+
<Hide until="tablet">
592+
<DesktopAdSlot renderAds={true} adSlotIndex={props.adIndex} />
593+
</Hide>
594+
<Hide from="tablet">
595+
<MobileAdSlot renderAds={true} adSlotIndex={props.adIndex} />
596+
</Hide>
597+
</div>
598+
<AdSlotBorders />
599+
</div>
600+
);
601+
602+
const AdSlotBorders = () => (
603+
<div
604+
css={{
605+
position: 'relative',
606+
[between.desktop.and.leftCol]: {
607+
'&': css(grid.column.right),
608+
609+
'&::before': {
610+
content: '""',
611+
position: 'absolute',
612+
left: -10,
613+
top: 0,
614+
bottom: 0,
615+
width: 1,
616+
backgroundColor: palette('--article-border'),
617+
},
618+
},
619+
620+
[from.leftCol]: {
621+
'&': css(grid.column.left),
622+
623+
'&::after': {
624+
content: '""',
625+
position: 'absolute',
626+
right: -10,
627+
top: 0,
628+
bottom: 0,
629+
width: 1,
630+
backgroundColor: palette('--article-border'),
631+
},
632+
},
633+
}}
634+
/>
635+
);
636+
605637
const MerchandisingHigh = (props: {
606638
show: boolean;
607639
display: ArticleDisplay;

0 commit comments

Comments
 (0)