Skip to content

Commit cf39bf7

Browse files
frederickobrienfrederickobrien
committed
Review polishes and refactors
1 parent 621cff3 commit cf39bf7

3 files changed

Lines changed: 91 additions & 122 deletions

File tree

‎dotcom-rendering/src/grid.ts‎

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -87,70 +87,75 @@ const paddedContainer = `
8787

8888
// ----- Vertical Rules ----- //
8989

90+
type VerticalRuleOptions = {
91+
centre?: boolean;
92+
};
93+
9094
/**
91-
* Optional vertical grid rules.
95+
* Render Guardian grid vertical rules.
96+
*
97+
* Left and right rules are always present.
98+
* A centre rule can optionally be enabled.
9299
*
93100
* Usage:
94-
* <div css={css`${grid.container} ${grid.verticalRules}`}>
95-
* <div className="grid-rule rule-left" />
96-
* <div className="grid-rule rule-centre" />
97-
* <div className="grid-rule rule-right" />
98-
* ...
99-
* </div>
101+
* css([grid.container, grid.verticalRules()])
102+
* css([grid.container, grid.verticalRules({ centre: true })])
100103
*/
101-
const verticalRules = `
104+
const verticalRules = (options: VerticalRuleOptions = {}): string => `
102105
${fromBreakpoint.tablet} {
103106
position: relative;
104-
--centre-transform: translateX(-${columnGap});
105107
106-
${fromBreakpoint.leftCol} {
107-
--centre-transform: translateX(calc(${columnGap} / -2));
108-
}
108+
--centre-transform: translateX(-${columnGap});
109109
110-
.grid-rule {
111-
position: absolute;
112-
top: 0;
113-
bottom: 0;
114-
width: 1px;
115-
background-color: ${themePalette('--article-border')};
116-
pointer-events: none;
117-
}
110+
${fromBreakpoint.leftCol} {
111+
--centre-transform: translateX(calc(${columnGap} / -2));
112+
}
118113
119-
/* LEFT OUTER RULE — where the left column starts */
120-
.rule-left {
121-
grid-column: left-column-start;
122-
justify-self: start;
123-
transform: translateX(-${columnGap});
114+
&::before,
115+
&::after
116+
${options.centre ? ', & > *:first-child::before' : ''} {
117+
position: absolute;
118+
top: 0;
119+
bottom: 0;
120+
width: 1px;
121+
background-color: ${themePalette('--article-border')};
122+
pointer-events: none;
123+
content: '';
124124
}
125125
126-
/* CENTRE RULE — start of centre column */
127-
.rule-centre {
126+
/* LEFT OUTER RULE */
127+
&::before {
128128
grid-column: centre-column-start;
129129
justify-self: start;
130-
transform: var(--centre-transform); /* centre it in gap */
130+
transform: translateX(-${columnGap});
131+
132+
${fromBreakpoint.leftCol} {
133+
grid-column: left-column-start;
134+
}
131135
}
132136
133-
/* RIGHT OUTER RULE — end of right column */
134-
.rule-right {
137+
/* RIGHT OUTER RULE */
138+
&::after {
135139
grid-column: right-column-end;
136140
justify-self: start;
137-
transform: translateX(-1px);
138-
${betweenBreakpoint.tablet.and.desktop} {
139-
grid-column: centre-column-end;
140-
}
141-
}
142-
}
141+
transform: translateX(-1px);
143142
144-
${fromBreakpoint.leftCol} {
145-
.rule-left {
146-
grid-column: left-column-start;
143+
${betweenBreakpoint.tablet.and.desktop} {
144+
grid-column: centre-column-end;
145+
}
147146
}
148-
}
149147
150-
${fromBreakpoint.wide} {
151-
.rule-right {
152-
grid-column: right-column-end;
153-
}
148+
${
149+
options.centre
150+
? `
151+
/* CENTRE RULE */
152+
& > *:first-child::before {
153+
grid-column: centre-column-start;
154+
justify-self: start;
155+
transform: var(--centre-transform);
156+
}`
157+
: ''
158+
}
154159
}
155160
`;
156161

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

Lines changed: 38 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -105,39 +105,27 @@ const GridItem = ({
105105
customCss,
106106
children,
107107
}: GridItemProps) => {
108-
const mobileCol = 'centre';
109-
const tabletCol = columns?.tablet ?? mobileCol;
110-
const leftColCol = columns?.leftCol ?? mobileCol;
111-
const desktopCol = columns?.desktop ?? mobileCol;
108+
const columnCss = (columnsConfig?: GridItemProps['columns']) => [
109+
grid.column.centre,
110+
Object.entries({
111+
tablet: columnsConfig?.tablet,
112+
desktop: columnsConfig?.desktop,
113+
leftCol: columnsConfig?.leftCol,
114+
})
115+
.filter(([, value]) => value != null)
116+
.map(
117+
([bp, col]) => css`
118+
${from[bp as keyof typeof from]} {
119+
${grid.column[col!]};
120+
}
121+
`,
122+
),
123+
];
112124

113125
return (
114126
<Element
115127
data-gu-name={area}
116-
css={css([
117-
grid.column[mobileCol],
118-
rowCss(area, layoutType),
119-
customCss,
120-
121-
// Override column at breakpoints if specified
122-
columns?.tablet &&
123-
css`
124-
${from.tablet} {
125-
${grid.column[tabletCol]};
126-
}
127-
`,
128-
columns?.desktop &&
129-
css`
130-
${from.desktop} {
131-
${grid.column[desktopCol]};
132-
}
133-
`,
134-
columns?.leftCol &&
135-
css`
136-
${from.leftCol} {
137-
${grid.column[leftColCol]};
138-
}
139-
`,
140-
])}
128+
css={css([columnCss(columns), rowCss(area, layoutType), customCss])}
141129
>
142130
{children}
143131
</Element>
@@ -219,9 +207,7 @@ export const StandardLayout = (props: WebProps | AppProps) => {
219207

220208
const renderAds = canRenderAds(article);
221209

222-
const layoutType: LayoutType = isLabs
223-
? 'labs'
224-
: isMatchReport
210+
const layoutType: LayoutType = isMatchReport
225211
? 'matchReport'
226212
: isMedia
227213
? 'media'
@@ -303,17 +289,21 @@ export const StandardLayout = (props: WebProps | AppProps) => {
303289
)};
304290
`}
305291
>
306-
<div css={css([grid.container, grid.verticalRules])}>
307-
<div className="grid-rule rule-left" />
308-
<div className="grid-rule rule-centre" />
309-
<div className="grid-rule rule-right" />
310-
<GridItem
311-
area="match-nav"
312-
layoutType={layoutType}
313-
element="aside"
314-
>
315-
<div css={maxWidth}>
316-
{isMatchReport && (
292+
<div
293+
css={css([
294+
grid.container,
295+
grid.verticalRules({
296+
centre: isLabs ? false : true,
297+
}),
298+
])}
299+
>
300+
{isMatchReport && !isInFootballRedesignVariantGroup && (
301+
<GridItem
302+
area="match-summary"
303+
layoutType={layoutType}
304+
element="aside"
305+
>
306+
<div css={maxWidth}>
317307
<Island
318308
priority="feature"
319309
defer={{ until: 'visible' }}
@@ -328,16 +318,8 @@ export const StandardLayout = (props: WebProps | AppProps) => {
328318
}
329319
/>
330320
</Island>
331-
)}
332-
</div>
333-
</GridItem>
334-
<GridItem
335-
area="match-tabs"
336-
layoutType={layoutType}
337-
element="aside"
338-
>
339-
<div css={maxWidth}>
340-
{isMatchReport && (
321+
</div>
322+
<div css={maxWidth}>
341323
<Island
342324
priority="critical"
343325
defer={{ until: 'visible' }}
@@ -347,9 +329,9 @@ export const StandardLayout = (props: WebProps | AppProps) => {
347329
format={format}
348330
/>
349331
</Island>
350-
)}
351-
</div>
352-
</GridItem>
332+
</div>
333+
</GridItem>
334+
)}
353335
<GridItem
354336
area="main-media"
355337
layoutType={layoutType}
@@ -698,7 +680,6 @@ export const StandardLayout = (props: WebProps | AppProps) => {
698680
${from.desktop} {
699681
display: block;
700682
padding-top: 6px;
701-
${grid.column.right};
702683
grid-row: 1 / span 999;
703684
}
704685
`}

‎dotcom-rendering/src/layouts/lib/furnitureLayouts.ts‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { css, type SerializedStyles } from '@emotion/react';
22
import { from, until } from '@guardian/source/foundations';
33

4-
export type LayoutType = 'standard' | 'matchReport' | 'media' | 'labs';
4+
export type LayoutType = 'standard' | 'matchReport' | 'media';
55

66
export type Area =
77
// Common areas
@@ -13,8 +13,7 @@ export type Area =
1313
| 'body'
1414
| 'right-column'
1515
// Match report specific areas
16-
| 'match-nav'
17-
| 'match-tabs';
16+
| 'match-summary';
1817

1918
type LayoutRows = Partial<
2019
Record<
@@ -49,17 +48,15 @@ const furnitureRowLayouts: Record<LayoutType, LayoutDefinition> = {
4948
},
5049
matchReport: {
5150
tablet: [
52-
['match-nav'],
53-
['match-tabs'],
51+
['match-summary'],
5452
['title'],
5553
['headline'],
5654
['standfirst'],
5755
['main-media'],
5856
['meta'],
5957
],
6058
leftCol: [
61-
['title', 'match-nav'],
62-
['match-tabs'],
59+
['title', 'match-summary'],
6360
['headline'],
6461
['meta', 'main-media'],
6562
],
@@ -85,20 +82,6 @@ const furnitureRowLayouts: Record<LayoutType, LayoutDefinition> = {
8582
['standfirst'],
8683
],
8784
},
88-
labs: {
89-
tablet: [
90-
['title'],
91-
['headline'],
92-
['standfirst'],
93-
['main-media'],
94-
['meta'],
95-
],
96-
leftCol: [
97-
['title', 'headline'],
98-
['standfirst'],
99-
['meta', 'main-media'],
100-
],
101-
},
10285
};
10386

10487
const buildRowMap = (layout: LayoutDefinition): LayoutRows => {

0 commit comments

Comments
 (0)