Skip to content

Commit

Permalink
refactor: remove unused decideContainerOverrides
Browse files Browse the repository at this point in the history
  • Loading branch information
mxdvl committed Jul 23, 2024
1 parent bae758e commit 27ea2c8
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 127 deletions.
122 changes: 62 additions & 60 deletions dotcom-rendering/src/components/Card/Card.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { splitTheme } from '../../../.storybook/decorators/splitThemeDecorator';
import type { Branding } from '../../types/branding';
import type { DCRContainerPalette } from '../../types/front';
import type { MainMedia } from '../../types/mainMedia';
import { ContainerOverrides } from '../ContainerOverrides';
import { FrontSection } from '../FrontSection';
import { LabsSection } from '../LabsSection';
import { Section } from '../Section';
Expand Down Expand Up @@ -1182,69 +1183,70 @@ export const WithBranding = () => {
} satisfies Branding;

return [undefined, ...containerPalettes].map((containerPalette) => (
<Section
<ContainerOverrides
key={containerPalette}
title={containerPalette ?? 'Standard'}
containerPalette={containerPalette}
>
<UL direction="row" padBottom={true}>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Standard,
theme: ArticleSpecial.Labs,
}}
headlineText="guardian.org branding on a Standard card"
kickerText="Kicker"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
containerPalette={containerPalette}
branding={branding}
/>
</LI>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Gallery,
theme: ArticleSpecial.Labs,
}}
kickerText="Kicker"
headlineText="guardian.org branding on a Gallery card"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
mainMedia={mainGallery}
containerPalette={containerPalette}
branding={branding}
/>
</LI>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Standard,
theme: Pillar.News,
}}
headlineText="guardian.org branding does not appear on non Labs articles"
kickerText="Kicker"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
containerPalette={containerPalette}
branding={branding}
/>
</LI>
</UL>
</Section>
<Section title={containerPalette ?? 'Standard'}>
<UL direction="row" padBottom={true}>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Standard,
theme: ArticleSpecial.Labs,
}}
headlineText="guardian.org branding on a Standard card"
kickerText="Kicker"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
containerPalette={containerPalette}
branding={branding}
/>
</LI>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Gallery,
theme: ArticleSpecial.Labs,
}}
kickerText="Kicker"
headlineText="guardian.org branding on a Gallery card"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
mainMedia={mainGallery}
containerPalette={containerPalette}
branding={branding}
/>
</LI>
<LI percentage={'33.333%'} padSides={true}>
<Card
{...basicCardProps}
format={{
display: ArticleDisplay.Standard,
design: ArticleDesign.Standard,
theme: Pillar.News,
}}
headlineText="guardian.org branding does not appear on non Labs articles"
kickerText="Kicker"
trailText=""
imagePositionOnDesktop="top"
imagePositionOnMobile="left"
imageSize="small"
containerPalette={containerPalette}
branding={branding}
/>
</LI>
</UL>
</Section>
</ContainerOverrides>
));
};

Expand Down
10 changes: 4 additions & 6 deletions dotcom-rendering/src/components/CardHeadline.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,11 @@ const containerPalettes = [
export const WithContainerOverrides: StoryObj = ({ format }: StoryProps) => (
<>
{containerPalettes.map((containerPalette) => (
<Section
<ContainerOverrides
key={containerPalette}
fullWidth={true}
showSideBorders={false}
containerPalette={containerPalette}
>
<ContainerOverrides containerPalette={containerPalette}>
<Section fullWidth={true} showSideBorders={false}>
<CardHeadline
headlineText={`This is a ${
Pillar[format.theme] ??
Expand All @@ -462,8 +460,8 @@ export const WithContainerOverrides: StoryObj = ({ format }: StoryProps) => (
byline={`inside a ${containerPalette} container`}
showByline={true}
/>
</ContainerOverrides>
</Section>
</Section>
</ContainerOverrides>
))}
</>
);
Expand Down
21 changes: 10 additions & 11 deletions dotcom-rendering/src/components/LatestLinks.importable.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { breakpoints } from '@guardian/source/foundations';
import fetchMock from 'fetch-mock';
import type { PropsWithChildren } from 'react';
import { splitTheme } from '../../.storybook/decorators/splitThemeDecorator';
import { decideContainerOverrides } from '../lib/decideContainerOverrides';
import { palette } from '../palette';
import type { DCRContainerPalette } from '../types/front';
import { ContainerOverrides } from './ContainerOverrides';
import { Island } from './Island';
Expand Down Expand Up @@ -147,16 +147,15 @@ const Wrapper = ({

export const WorldCupFinal2023 = () => {
const containerPalette = 'EventAltPalette' satisfies DCRContainerPalette;
const overrides = decideContainerOverrides(containerPalette);

return (
<Wrapper
styles={css`
max-width: 600px;
background-color: ${overrides.background.container};
`}
>
<ContainerOverrides containerPalette={containerPalette}>
<ContainerOverrides containerPalette={containerPalette}>
<Wrapper
styles={css`
max-width: 600px;
background-color: ${palette('--section-background-outer')};
`}
>
<Island priority="critical">
<LatestLinks
id="/football/live/2023/aug/20/spain-v-england-womens-world-cup-final-live"
Expand All @@ -166,8 +165,8 @@ export const WorldCupFinal2023 = () => {
absoluteServerTimes={true}
/>
</Island>
</ContainerOverrides>
</Wrapper>
</Wrapper>
</ContainerOverrides>
);
};
WorldCupFinal2023.storyName = 'World Cup Final 2023';
Expand Down
11 changes: 2 additions & 9 deletions dotcom-rendering/src/components/LatestLinks.importable.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { css } from '@emotion/react';
import {
lineHeights,
palette,
space,
textSans14,
textSansBold14,
} from '@guardian/source/foundations';
import { decideContainerOverrides } from '../lib/decideContainerOverrides';
import { revealStyles } from '../lib/revealStyles';
import { useApi } from '../lib/useApi';
import { palette as themePalette } from '../palette';
Expand Down Expand Up @@ -38,6 +36,7 @@ const linkStyles = css`
`;

const dividerStyles = css`
color: ${themePalette('--article-border')}
border-top: 1px solid currentColor;
border-left: 1px solid currentColor;
`;
Expand Down Expand Up @@ -107,12 +106,6 @@ export const LatestLinks = ({
refreshInterval: 9_600,
});

const dividerColour = css`
color: ${containerPalette
? decideContainerOverrides(containerPalette).border.container
: palette.neutral[86]};
`;

/** Reserve space for the latest links to avoid CLS while loading */
const minHeight = isDynamo
? `calc(${space[1]}px + 4 * ${lineHeights.regular}em);`
Expand Down Expand Up @@ -150,7 +143,7 @@ export const LatestLinks = ({
{index > 0 && (
<li
key={block.id + ' : divider'}
css={[dividerStyles, dividerColour]}
css={[dividerStyles]}
></li>
)}
<li
Expand Down
53 changes: 13 additions & 40 deletions dotcom-rendering/src/components/Section.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { css } from '@emotion/react';
import type { ArticleFormat } from '@guardian/libs';
import { ArticleDesign } from '@guardian/libs';
import { background, from, space, until } from '@guardian/source/foundations';
import { decideContainerOverrides } from '../lib/decideContainerOverrides';
import { from, space, until } from '@guardian/source/foundations';
import type { EditionId } from '../lib/edition';
import { hiddenStyles } from '../lib/hiddenStyles';
import type { DCRContainerPalette, TreatType } from '../types/front';
import { palette } from '../palette';
import type { TreatType } from '../types/front';
import { ContainerTitle } from './ContainerTitle';
import { ElementContainer } from './ElementContainer';
import { Flex } from './Flex';
Expand Down Expand Up @@ -78,8 +78,6 @@ type Props = {
* @see https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/client/debug/README.md
*/
containerName?: string;
/** Fronts containers can have their styling overridden using a `containerPalette` */
containerPalette?: DCRContainerPalette;
/** Defaults to `false`. If true a Hide button is show top right allowing this section
* to be collapsed
*/
Expand Down Expand Up @@ -211,23 +209,6 @@ const MaybeHideAboveLeftCol = ({
</Hide>
);

const decideBackgroundColour = (
backgroundColour: string | undefined,
overrideBackgroundColour: string | undefined,
hasPageSkin: boolean,
) => {
if (backgroundColour) {
return backgroundColour;
}
if (overrideBackgroundColour) {
return overrideBackgroundColour;
}
if (hasPageSkin) {
return background.primary;
}
return undefined;
};

/**
*
* A Section component represents a horizontal slice of a page. It defaults to
Expand Down Expand Up @@ -262,7 +243,6 @@ export const Section = ({
format,
ophanComponentLink,
ophanComponentName,
containerPalette,
toggleable = false,
innerBackgroundColour,
showDateHeader = false,
Expand All @@ -276,9 +256,6 @@ export const Section = ({
hasPageSkinContentSelfConstrain = false,
className,
}: Props) => {
const overrides =
containerPalette && decideContainerOverrides(containerPalette);

if (fullWidth) {
return (
<ElementContainer
Expand All @@ -287,12 +264,10 @@ export const Section = ({
showTopBorder={showTopBorder}
padSides={padSides}
padBottom={padBottom}
borderColour={borderColour ?? overrides?.border.container}
backgroundColour={decideBackgroundColour(
backgroundColour,
overrides?.background.container,
hasPageSkin,
)}
borderColour={borderColour ?? palette('--article-border')}
backgroundColour={
backgroundColour ?? palette('--section-background-inner')
}
ophanComponentLink={ophanComponentLink}
ophanComponentName={ophanComponentName}
containerName={containerName}
Expand All @@ -316,12 +291,10 @@ export const Section = ({
showSideBorders={showSideBorders}
showTopBorder={showTopBorder}
padSides={padSides}
borderColour={borderColour ?? overrides?.border.container}
backgroundColour={decideBackgroundColour(
backgroundColour,
overrides?.background.container,
hasPageSkin,
)}
borderColour={borderColour ?? palette('--article-border')}
backgroundColour={
backgroundColour ?? palette('--section-background-inner')
}
element="section"
ophanComponentLink={ophanComponentLink}
ophanComponentName={ophanComponentName}
Expand All @@ -333,7 +306,7 @@ export const Section = ({
<Flex>
<LeftColumn
borderType={centralBorder}
borderColour={borderColour ?? overrides?.border.container}
borderColour={borderColour ?? palette('--article-border')}
size={leftColSize}
verticalMargins={verticalMargins}
hasPageSkin={hasPageSkin}
Expand Down Expand Up @@ -361,7 +334,7 @@ export const Section = ({
<Treats
treats={treats}
borderColour={
borderColour ?? overrides?.border.container
borderColour ?? palette('--article-border')
}
/>
)}
Expand Down
1 change: 0 additions & 1 deletion dotcom-rendering/src/layouts/FrontLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,6 @@ export const FrontLayout = ({ front, NAV }: Props) => {
showTopBorder={index > 0}
padContent={false}
url={collection.href}
containerPalette={containerPalette}
showDateHeader={
collection.config.showDateHeader
}
Expand Down

0 comments on commit 27ea2c8

Please sign in to comment.