Skip to content

Commit b85536a

Browse files
marjisoundmarjisound
committed
refactor to address pr comments
1 parent 863faeb commit b85536a

File tree

2 files changed

+87
-75
lines changed

2 files changed

+87
-75
lines changed

‎dotcom-rendering/src/components/FootballNavAtom.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ export const FootballNavAtom = ({
1313
title="Navigation for the current football competition"
1414
css={{
1515
width: '100%',
16+
// ['&'] part refers to the current element being styled.
17+
// It allows to mix stringified styles (like CSS selectors) with object styles
1618
['&']: css(grid.column.all),
1719
gridRow: 1,
1820
}}

‎dotcom-rendering/src/components/FootballTablesPage.tsx‎

Lines changed: 85 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { css } from '@emotion/react';
2+
import { isUndefined } from '@guardian/libs';
23
import {
34
from,
45
headlineBold20,
@@ -32,89 +33,98 @@ export const FootballTablesPage = ({
3233
renderAds,
3334
guardianBaseUrl,
3435
navAtom,
35-
}: Props) => (
36-
<main
37-
id="maincontent"
38-
data-layout="FootballDataPageLayout"
39-
css={css`
40-
${grid.paddedContainer}
41-
position: relative;
42-
${from.tablet} {
43-
&::before,
44-
&::after {
45-
content: '';
46-
position: absolute;
47-
border-left: 1px solid ${palette('--article-border')};
48-
top: 0;
49-
bottom: 0;
50-
}
36+
}: Props) => {
37+
const navAtomIsDefined = !isUndefined(navAtom);
38+
return (
39+
<main
40+
id="maincontent"
41+
data-layout="FootballDataPageLayout"
42+
css={css`
43+
${grid.paddedContainer}
44+
position: relative;
45+
${from.tablet} {
46+
&::before,
47+
&::after {
48+
content: '';
49+
position: absolute;
50+
border-left: 1px solid ${palette('--article-border')};
51+
top: 0;
52+
bottom: 0;
53+
}
5154
52-
&::after {
53-
right: 0;
55+
&::after {
56+
right: 0;
57+
}
5458
}
55-
}
5659
57-
padding-bottom: ${space[9]}px;
58-
`}
59-
>
60-
<FootballNavAtom navAtom={navAtom} />
61-
<h1
62-
css={css`
63-
${headlineBold20}
64-
padding: ${space[2]}px 0 ${space[3]}px;
65-
${grid.column.centre}
66-
grid-row: ${navAtom !== undefined ? 2 : 1};
67-
${from.leftCol} {
68-
${grid.between('left-column-start', 'centre-column-end')}
69-
}
60+
padding-bottom: ${space[9]}px;
7061
`}
7162
>
72-
Football tables
73-
</h1>
74-
<div
75-
css={css`
76-
margin-top: ${space[3]}px;
77-
margin-bottom: ${space[6]}px;
78-
${grid.column.centre}
79-
grid-row: ${navAtom !== undefined ? 3 : 2};
80-
`}
81-
>
82-
<Island priority="feature" defer={{ until: 'visible' }}>
83-
<FootballTablesCompetitionSelect
84-
regions={regions}
85-
pageId={pageId}
86-
guardianBaseUrl={guardianBaseUrl}
87-
/>
88-
</Island>
89-
</div>
90-
<div
91-
css={css`
92-
${grid.column.centre}
93-
grid-row: ${navAtom !== undefined ? 4 : 3};
94-
${from.leftCol} {
95-
${grid.between('left-column-start', 'centre-column-end')}
96-
}
97-
position: relative;
98-
`}
99-
>
100-
<FootballTableList
101-
competitions={competitions}
102-
guardianBaseUrl={guardianBaseUrl}
103-
/>
104-
</div>
105-
{renderAds && (
63+
<FootballNavAtom navAtom={navAtom} />
64+
<h1
65+
css={css`
66+
${headlineBold20}
67+
padding: ${space[2]}px 0 ${space[3]}px;
68+
${grid.column.centre}
69+
grid-row: ${navAtomIsDefined ? 2 : 1};
70+
${from.leftCol} {
71+
${grid.between(
72+
'left-column-start',
73+
'centre-column-end',
74+
)}
75+
}
76+
`}
77+
>
78+
Football tables
79+
</h1>
80+
<div
81+
css={css`
82+
margin-top: ${space[3]}px;
83+
margin-bottom: ${space[6]}px;
84+
${grid.column.centre}
85+
grid-row: ${navAtomIsDefined ? 3 : 2};
86+
`}
87+
>
88+
<Island priority="feature" defer={{ until: 'visible' }}>
89+
<FootballTablesCompetitionSelect
90+
regions={regions}
91+
pageId={pageId}
92+
guardianBaseUrl={guardianBaseUrl}
93+
/>
94+
</Island>
95+
</div>
10696
<div
10797
css={css`
108-
${grid.column.right}
109-
/** This allows the ad to grow beyond the third row content (up to line 5) */
110-
grid-row: ${navAtom !== undefined ? '2 / 5' : '1 / 4'};
111-
${until.desktop} {
112-
display: none;
98+
${grid.column.centre}
99+
grid-row: ${navAtomIsDefined ? 4 : 3};
100+
${from.leftCol} {
101+
${grid.between(
102+
'left-column-start',
103+
'centre-column-end',
104+
)}
113105
}
106+
position: relative;
114107
`}
115108
>
116-
<AdSlot position="football-right" />
109+
<FootballTableList
110+
competitions={competitions}
111+
guardianBaseUrl={guardianBaseUrl}
112+
/>
117113
</div>
118-
)}
119-
</main>
120-
);
114+
{renderAds && (
115+
<div
116+
css={css`
117+
${grid.column.right}
118+
/** This allows the ad to grow beyond the third row content (up to line 5) */
119+
grid-row: ${navAtomIsDefined ? '2 / 5' : '1 / 4'};
120+
${until.desktop} {
121+
display: none;
122+
}
123+
`}
124+
>
125+
<AdSlot position="football-right" />
126+
</div>
127+
)}
128+
</main>
129+
);
130+
};

0 commit comments

Comments
 (0)