Skip to content

Commit 799b553

Browse files
authored
hero component cleanup (#597)
Addresses: #596 Hero work: - aligned with cfpb heroes pattern docs - dropped headingLevel subheadingLevel textColor props - heading and subheading typed as strings - use DSR Heading type 1 with m-hero__heading not raw h1 - subheading still p.m-hero__subhead - root is section not div - backgroundColor on wrapper normally, on section for knockout - fixed useBackgroundImage clearing wrapper backgroundColor - HeroProperties omits children and color from html attrs - only render heading/subheading when provided - Added mobileImage option for photograph heroes - updated stories to point to images from DS github page
1 parent 718b120 commit 799b553

6 files changed

Lines changed: 228 additions & 123 deletions

File tree

src/components/Hero/hero-image.tsx

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,47 @@
1+
import type { CSSProperties } from 'react';
12
import type { JSXElement } from '../../types/jsx-element';
23

34
interface HeroImageProperties {
45
image?: string;
6+
mobileImage?: string;
57
altText?: string;
68
}
79

10+
type HeroImageStyle = CSSProperties & {
11+
'--m-hero-image'?: string;
12+
'--m-hero-mobile-image'?: string;
13+
};
14+
15+
const createBackgroundImageValue = (image: string): string =>
16+
`url(${JSON.stringify(image)})`;
17+
818
/**
919
* DS pattern: empty `.m-hero__image` with `background-image` (not `<img>`), so
1020
* `background-size: contain` and aspect ratio control the slot height.
1121
* https://cfpb.github.io/design-system/patterns/heroes
1222
*/
1323
export const HeroImage = ({
1424
image,
25+
mobileImage,
1526
altText,
1627
}: HeroImageProperties): JSXElement => {
17-
if (!image) return null;
28+
if (!image && !mobileImage) return null;
29+
30+
const imageStyle: HeroImageStyle = {};
31+
32+
if (image) imageStyle['--m-hero-image'] = createBackgroundImageValue(image);
33+
if (mobileImage) {
34+
imageStyle['--m-hero-mobile-image'] =
35+
createBackgroundImageValue(mobileImage);
36+
}
1837

1938
return (
2039
<div className='m-hero__image-wrapper'>
2140
<div
2241
className='m-hero__image'
2342
role='img'
2443
aria-label={altText}
25-
style={{
26-
backgroundImage: `url(${JSON.stringify(image)})`,
27-
}}
44+
style={imageStyle}
2845
/>
2946
</div>
3047
);

src/components/Hero/hero.scss

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@
44
background-color: var(--gray-30);
55
}
66

7+
.m-hero__image {
8+
background-image: var(--m-hero-mobile-image, var(--m-hero-image));
9+
}
10+
11+
.m-hero--overlay .m-hero__wrapper {
12+
background-image: none;
13+
}
14+
15+
@media screen and (min-width: 37.5625em) {
16+
.m-hero__image {
17+
background-image: var(--m-hero-image, var(--m-hero-mobile-image));
18+
}
19+
20+
.m-hero--overlay .m-hero__wrapper {
21+
background-image: var(--m-hero-wrapper-image);
22+
}
23+
}
24+
725
// Illustration slot: match common DS hero artwork (940×390).
826
//
927
// Production DS/Jinja pages often set this on an empty `.m-hero__image` with `background-image`,

src/components/Hero/hero.stories.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const WithIllustration: Story = {
3232
args: {
3333
heading: '41 characters max for a one-line heading',
3434
image:
35-
'https://cfpb.github.io/design-system/images/uploads/hero_illustration_example_keys.png',
35+
'https://cfpb.github.io/design-system/images/uploads/design_system_illustration_hero_example.png',
3636
subheading:
3737
'This text has a recommended count of 165-186 characters (three lines at 1230px) following a one-line heading and 108-124 characters (two lines at 1230px) following a two-line heading.',
3838
backgroundColor: '#d4e7e6',
@@ -44,7 +44,10 @@ export const WithPhotograph: Story = {
4444
args: {
4545
...WithIllustration.args,
4646
imageIsPhoto: true,
47-
image: 'https://files.consumerfinance.gov/f/images/PC_hero.original.jpg',
47+
image:
48+
'https://cfpb.github.io/design-system/images/uploads/design_system_photo_hero_example.png',
49+
mobileImage:
50+
'https://cfpb.github.io/design-system/images/uploads/design_system_photo_hero_sm_example.jpg',
4851
backgroundColor: '#f7f8f9',
4952
},
5053
};
@@ -59,6 +62,6 @@ export const WithKnockoutText: Story = {
5962
backgroundColor: '#207676',
6063
isKnockout: true,
6164
image:
62-
'https://cfpb.github.io/design-system/images/uploads/design_system_hero_example.png',
65+
'https://cfpb.github.io/design-system/images/uploads/design_system_knockout_hero_example.png',
6366
},
6467
};

src/components/Hero/hero.test.tsx

Lines changed: 87 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import '@testing-library/jest-dom';
12
import { render, screen } from '@testing-library/react';
23
import Hero from './hero';
34

45
describe('Hero', () => {
5-
it('Renders all elements: heading, subheading, image', () => {
6+
it('renders heading, subheading, and image with DS classes', () => {
67
const heading = 'heading';
78
const subheading = 'subheading';
89
const image = '../../assets/images/credit-card-hero.png';
@@ -17,69 +18,117 @@ describe('Hero', () => {
1718
/>,
1819
);
1920

20-
expect(screen.getByText(heading).className).toMatch(/m-hero__heading/g);
21-
expect(screen.getByText(subheading).className).toMatch(/m-hero__subhead/g);
22-
expect(screen.getByRole('img', { name: imageText }).className).toMatch(
23-
/m-hero__image/g,
21+
const headingElement = screen.getByRole('heading', { level: 1 });
22+
expect(headingElement).toHaveClass('m-hero__heading');
23+
expect(headingElement).toHaveTextContent(heading);
24+
25+
const subheadingElement = screen.getByText(subheading);
26+
expect(subheadingElement.tagName).toBe('P');
27+
expect(subheadingElement).toHaveClass('m-hero__subhead');
28+
29+
expect(screen.getByRole('img', { name: imageText })).toHaveClass(
30+
'm-hero__image',
2431
);
2532
});
2633

27-
it('Derives CSS classes for component variations', () => {
34+
it('derives CSS classes for component variations', () => {
2835
const isKnockout = 'm-hero--knockout';
2936
const imageIsPhoto = 'm-hero--overlay';
3037

3138
render(<Hero isKnockout data-testid={isKnockout} />);
32-
expect(screen.getByTestId(isKnockout).className).toMatch(isKnockout);
39+
expect(screen.getByTestId(isKnockout)).toHaveClass(isKnockout);
3340

34-
render(<Hero imageIsPhoto data-testid={imageIsPhoto} />);
35-
expect(screen.getByTestId(imageIsPhoto).className).toMatch(imageIsPhoto);
41+
render(
42+
<Hero
43+
imageIsPhoto
44+
mobileImage='/mobile-photo.jpg'
45+
data-testid={imageIsPhoto}
46+
/>,
47+
);
48+
expect(screen.getByTestId(imageIsPhoto)).toHaveClass(imageIsPhoto);
3649
});
3750

38-
it('Applies direct color settings', () => {
39-
const textColor = 'orange';
40-
const backgroundColor = 'purple';
41-
const wrapperSelector = 'wrapper';
42-
const textId = 'hero-text';
51+
it('applies background color on the section for standard heroes', () => {
52+
render(
53+
<Hero backgroundColor='#800080' heading='test' data-testid='hero' />,
54+
);
55+
56+
expect(screen.getByTestId('hero')).toHaveStyle({
57+
backgroundColor: 'rgb(128, 0, 128)',
58+
});
59+
expect(screen.getByTestId('hero-wrapper')).not.toHaveAttribute('style');
60+
});
4361

62+
it('applies background color on the section for knockout heroes', () => {
4463
render(
4564
<Hero
46-
data-testid='wrapper'
47-
textColor={textColor}
48-
backgroundColor={backgroundColor}
65+
isKnockout
66+
backgroundColor='#207676'
4967
heading='test'
68+
data-testid='hero'
5069
/>,
5170
);
5271

53-
// Background color
54-
const wrapper = screen.getByTestId(wrapperSelector);
55-
expect(wrapper.style.backgroundColor).toMatch(backgroundColor);
72+
expect(screen.getByTestId('hero')).toHaveStyle({
73+
backgroundColor: '#207676',
74+
});
75+
});
5676

57-
// Text color
58-
const text = screen.getByTestId(textId);
59-
expect(text.style.color).toMatch(textColor);
77+
it('does not set inline text color (knockout uses DS styles)', () => {
78+
render(
79+
<Hero isKnockout heading='test' subheading='sub' data-testid='hero' />,
80+
);
81+
82+
expect(screen.getByTestId('hero-text')).not.toHaveAttribute('style');
6083
});
6184

62-
it('Applies heading levels', () => {
63-
const headingLevel = 2;
64-
const headingText = 'Heading text';
65-
const subheadingLevel = 3;
66-
const subheadingText = 'Subheading text';
85+
it('uses the mobile image in the image slot for photo heroes', () => {
86+
render(
87+
<Hero
88+
imageIsPhoto
89+
image='/desktop-photo.png'
90+
mobileImage='/mobile-photo.jpg'
91+
imageAltText='photo hero'
92+
/>,
93+
);
94+
95+
const heroImage = screen.getByRole('img', { name: 'photo hero' });
96+
expect(heroImage.style.getPropertyValue('--m-hero-image')).toBe(
97+
'url("/desktop-photo.png")',
98+
);
99+
expect(heroImage.style.getPropertyValue('--m-hero-mobile-image')).toBe(
100+
'url("/mobile-photo.jpg")',
101+
);
102+
});
67103

104+
it('supports a mobile image in the image slot for non-photo heroes', () => {
68105
render(
69106
<Hero
70-
heading='Heading text'
71-
headingLevel={`h${headingLevel}`}
72-
subheading='Subheading text'
73-
subheadingLevel={`h${subheadingLevel}`}
107+
image='/illustration.png'
108+
mobileImage='/mobile-photo.jpg'
109+
imageAltText='illustration hero'
74110
/>,
75111
);
76112

77-
// Heading
78-
const header = screen.getByRole('heading', { level: headingLevel });
79-
expect(header).toHaveProperty('textContent', headingText);
113+
const heroImage = screen.getByRole('img', { name: 'illustration hero' });
114+
expect(heroImage.style.getPropertyValue('--m-hero-image')).toBe(
115+
'url("/illustration.png")',
116+
);
117+
expect(heroImage.style.getPropertyValue('--m-hero-mobile-image')).toBe(
118+
'url("/mobile-photo.jpg")',
119+
);
120+
});
80121

81-
// Subheading
82-
const subheading = screen.getByRole('heading', { level: subheadingLevel });
83-
expect(subheading).toHaveProperty('textContent', subheadingText);
122+
it('requires a mobile image for photo heroes', () => {
123+
expect(() =>
124+
render(
125+
// @ts-expect-error mobileImage is required when imageIsPhoto is true.
126+
<Hero
127+
imageIsPhoto
128+
image='/desktop-photo.png'
129+
imageAltText='photo hero'
130+
/>,
131+
),
132+
).toThrow('Hero requires mobileImage when imageIsPhoto is true.');
84133
});
85134
});

0 commit comments

Comments
 (0)