Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use card headline fonts for card byline #12873

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
15 changes: 4 additions & 11 deletions dotcom-rendering/src/components/Byline.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { css } from '@emotion/react';
import { css, type SerializedStyles } from '@emotion/react';
import {
headlineMedium17,
headlineMedium20,
Expand All @@ -10,12 +10,10 @@
until,
} from '@guardian/source/foundations';
import { palette } from '../palette';
import type { SmallHeadlineSize } from '../types/layout';

type Props = {
text: string;
isLabs: boolean;
size: SmallHeadlineSize;
fontStyles: SerializedStyles;
/** Optional override of the standard text colour */
colour?: string;
};
Expand All @@ -25,7 +23,7 @@
color: ${colour};
`;

const bylineStyles = (size: SmallHeadlineSize, isLabs: boolean) => {

Check failure on line 26 in dotcom-rendering/src/components/Byline.tsx

View workflow job for this annotation

GitHub Actions / lint / check

'bylineStyles' is assigned a value but never used
switch (size) {
case 'ginormous':
case 'huge':
Expand Down Expand Up @@ -91,13 +89,8 @@

export const Byline = ({
text,
isLabs,
size,
fontStyles,
colour = palette('--byline'),
}: Props) => {
return (
<span css={[baseStyles(colour), bylineStyles(size, isLabs)]}>
{text}
</span>
);
return <span css={[baseStyles(colour), fontStyles]}>{text}</span>;
};
213 changes: 117 additions & 96 deletions dotcom-rendering/src/components/CardHeadline.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import { css } from '@emotion/react';
import {
between,
headlineLight14,
headlineLight15,
headlineLight17,
headlineLight20,
headlineLight24,
headlineLight28,
headlineLight34,
headlineLight42,
headlineLight50,
headlineLight64,
headlineMedium14,
headlineMedium15,
headlineMedium17,
Expand All @@ -20,60 +30,111 @@ import {
} from '@guardian/source/foundations';
import { Link, SvgExternal } from '@guardian/source/react-components';
import React from 'react';
import { type ArticleFormat, ArticleSpecial } from '../lib/articleFormat';
import {
ArticleDesign,
type ArticleFormat,
ArticleSpecial,
} from '../lib/articleFormat';
import { getZIndex } from '../lib/getZIndex';
import { palette } from '../palette';
import type { SmallHeadlineSize } from '../types/layout';
import { Byline } from './Byline';
import { Kicker } from './Kicker';
import { QuoteIcon } from './QuoteIcon';

type Props = {
headlineText: string; // The text shown
format: ArticleFormat; // Used to decide when to add type specific styles
/** The text shown */
headlineText: string;
/** Used to decide when to add type specific styles */
format: ArticleFormat;
kickerText?: string;
showPulsingDot?: boolean;
hasInlineKicker?: boolean;
showQuotes?: boolean; // Even with design !== Comment, a piece can be opinion
/** Even with design !== Comment, a piece can be opinion */
showQuotes?: boolean;
fontSizes?: ResponsiveFontSize;
byline?: string;
showByline?: boolean;
linkTo?: string; // If provided, the headline is wrapped in a link
/** If provided, the headline is wrapped in a link */
linkTo?: string;
isExternalLink?: boolean;
/** Optional override of the standard card headline colour */
headlineColour?: string;
/** Optional override of the standard card kicker colour */
kickerColour?: string;
};

const headlineSize = {
xxxlarge: headlineMedium64,
xxlarge: headlineMedium50,
xlarge: headlineMedium42,
large: headlineMedium34,
medium: headlineMedium28,
small: headlineMedium24,
xsmall: headlineMedium20,
xxsmall: headlineMedium17,
xxxsmall: headlineMedium15,
tiny: headlineMedium14,
};
const sublinkStyles = css`
display: block;
/* See: https://css-tricks.com/nested-links/ */
z-index: ${getZIndex('card-nested-link')};
/* The following styles turn off those provided by Link */
color: inherit;
text-decoration: none;
/* stylelint-disable-next-line property-disallowed-list */
font-family: inherit;
font-size: inherit;
line-height: inherit;

const textSansSize = {
xxxlarge: textSans20,
xxlarge: textSans20,
xlarge: textSans20,
large: textSans20,
medium: textSans20,
small: textSans20,
xsmall: textSans20,
xxsmall: textSans17,
xxxsmall: textSans15,
tiny: textSans12,
};
/* This css is used to remove any underline from the kicker but still
* have it applied to the headline when the kicker is hovered */
:hover {
color: inherit;
text-decoration: none;
.show-underline {
text-decoration: underline;
text-underline-offset: auto;
text-underline-position: auto;
}
}
`;

export type HeadlineSize = keyof typeof headlineSize;
export type TextSansSize = keyof typeof textSansSize;
/** These represent the font groups used by card headline */
const fontFamilies = {
headlineMedium: {
xxxlarge: headlineMedium64,
xxlarge: headlineMedium50,
xlarge: headlineMedium42,
large: headlineMedium34,
medium: headlineMedium28,
small: headlineMedium24,
xsmall: headlineMedium20,
xxsmall: headlineMedium17,
xxxsmall: headlineMedium15,
tiny: headlineMedium14,
},
headlineLight: {
xxxlarge: headlineLight64,
xxlarge: headlineLight50,
xlarge: headlineLight42,
large: headlineLight34,
medium: headlineLight28,
small: headlineLight24,
xsmall: headlineLight20,
xxsmall: headlineLight17,
xxxsmall: headlineLight15,
tiny: headlineLight14,
},
textSans: {
xxxlarge: textSans20,
xxlarge: textSans20,
xlarge: textSans20,
large: textSans20,
medium: textSans20,
small: textSans20,
xsmall: textSans20,
xxsmall: textSans17,
xxxsmall: textSans15,
tiny: textSans12,
},
} as const;

export enum FontFamily {
HeadlineMedium = 'headlineMedium',
HeadlineLight = 'headlineLight',
TextSans = 'textSans',
}

export type HeadlineSize = keyof typeof fontFamilies.headlineMedium;

export type ResponsiveFontSize = {
desktop: HeadlineSize;
Expand All @@ -82,64 +143,53 @@ export type ResponsiveFontSize = {
mobileMedium?: HeadlineSize;
};

const getFontSize = (
sizes: ResponsiveFontSize,
family: 'headline' | 'textSans',
) => {
const { desktop, tablet, mobileMedium, mobile } = sizes;
const getFontSize = (sizes: ResponsiveFontSize, family: FontFamily) => {
const font = fontFamilies[family];

const fontSize = family === 'headline' ? headlineSize : textSansSize;
const { desktop, tablet, mobileMedium, mobile } = sizes;

return css`
${fontSize[desktop]};
${font[desktop]};

${tablet &&
css`
${until.desktop} {
${fontSize[tablet]};
${font[tablet]};
}
`}

${mobile &&
css`
${until.tablet} {
${fontSize[mobile]};
${font[mobile]};
}
`}

${mobileMedium &&
css`
${between.mobileMedium.and.tablet} {
${fontSize[mobileMedium]};
${font[mobileMedium]};
}
`}
`;
};

const sublinkStyles = css`
display: block;
/* See: https://css-tricks.com/nested-links/ */
z-index: ${getZIndex('card-nested-link')};
/* The following styles turn off those provided by Link */
color: inherit;
text-decoration: none;
/* stylelint-disable-next-line property-disallowed-list */
font-family: inherit;
font-size: inherit;
line-height: inherit;
const getFonts = (format: ArticleFormat, fontSizes: ResponsiveFontSize) => {
if (format.theme === ArticleSpecial.Labs) {
return getFontSize(fontSizes, FontFamily.TextSans);
}

/* This css is used to remove any underline from the kicker but still
* have it applied to the headline when the kicker is hovered */
:hover {
color: inherit;
text-decoration: none;
.show-underline {
text-decoration: underline;
text-underline-offset: auto;
text-underline-position: auto;
}
if (
/** Any of these designs are considered an "opinion" */
format.design === ArticleDesign.Comment ||
format.design === ArticleDesign.Editorial ||
format.design === ArticleDesign.Letter
) {
return getFontSize(fontSizes, FontFamily.HeadlineLight);
}
`;

return getFontSize(fontSizes, FontFamily.HeadlineMedium);
};

export const WithLink = ({
linkTo,
Expand All @@ -158,29 +208,6 @@ export const WithLink = ({
return <>{children}</>;
};

/**
* The Byline component uses a different type to determine font sizes but infers the size from the desktop headline size.
* This function converts the headline size to the appropriate byline size.
*/
const getBylineFontSizes = (size: HeadlineSize): SmallHeadlineSize => {
switch (size) {
case 'xxlarge':
return 'ginormous';
case 'medium':
return 'huge';
case 'small':
return 'large';
case 'xsmall':
return 'medium';
case 'xxsmall':
return 'small';
case 'tiny':
return 'tiny';
default:
return 'medium';
}
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we no longer need to convert the sizes here as we pass font styles directly to byline

export const CardHeadline = ({
headlineText,
format,
Expand All @@ -200,12 +227,7 @@ export const CardHeadline = ({
// The link is only applied directly to the headline if it is a sublink
const isSublink = !!linkTo;

const fonts =
format.theme === ArticleSpecial.Labs
? getFontSize(fontSizes, 'textSans')
: getFontSize(fontSizes, 'headline');

const bylineSize = getBylineFontSizes(fontSizes.desktop);
const fontStyles = getFonts(format, fontSizes);

return (
<WithLink linkTo={linkTo}>
Expand All @@ -218,7 +240,7 @@ export const CardHeadline = ({
? css`
${textSans14}
`
: fonts,
: fontStyles,
]}
>
{!!kickerText && (
Expand Down Expand Up @@ -251,9 +273,8 @@ export const CardHeadline = ({
{!!byline && showByline && (
<Byline
text={byline}
isLabs={format.theme === ArticleSpecial.Labs}
size={bylineSize}
colour={kickerColour}
fontStyles={fontStyles}
/>
)}
</WithLink>
Expand Down
Loading
Loading