Skip to content

Commit 373ce95

Browse files
jonrohanjonrohan
authored
refactor(Avatar): Refactor Avatar component to use CSS modules behind feature flag (#4885)
* Import postcsspresetprimer to storybook config * Converting to CSS module * Create tame-boats-hide.md * Add testing in the feature flag * Convert to require * chore: Update postcss.config.js to use postcss-preset-primer * convert to module * Pass along className to StyledAvatar * fix for lint * Render boolean props with '' * Adjust typescript stuff
1 parent f8749ee commit 373ce95

File tree

5 files changed

+240
-119
lines changed

5 files changed

+240
-119
lines changed

‎.changeset/tame-boats-hide.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/react": minor
3+
---
4+
5+
Refactor Avatar component to use CSS modules behind feature flag

‎e2e/components/Avatar.test.ts‎

Lines changed: 128 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -3,139 +3,151 @@ import {visit} from '../test-helpers/storybook'
33
import {themes} from '../test-helpers/themes'
44

55
test.describe('Avatar', () => {
6-
test.describe('Default', () => {
7-
for (const theme of themes) {
8-
test.describe(theme, () => {
9-
test('default @vrt', async ({page}) => {
10-
await visit(page, {
11-
id: 'components-avatar--default',
12-
globals: {
13-
colorScheme: theme,
14-
},
15-
})
6+
for (const enabled of [true, false]) {
7+
test.describe(`Feature flag enabled: ${enabled}`, () => {
8+
test.describe('Default', () => {
9+
for (const theme of themes) {
10+
test.describe(theme, () => {
11+
test('default @vrt', async ({page}) => {
12+
await visit(page, {
13+
id: 'components-avatar--default',
14+
globals: {
15+
colorScheme: theme,
16+
primer_react_css_modules_team: enabled,
17+
},
18+
})
1619

17-
// Default state
18-
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Default.${theme}.png`)
19-
})
20+
// Default state
21+
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Default.${theme}.png`)
22+
})
2023

21-
test('axe @aat', async ({page}) => {
22-
await visit(page, {
23-
id: 'components-avatar--default',
24-
globals: {
25-
colorScheme: theme,
26-
},
27-
})
28-
await expect(page).toHaveNoViolations({
29-
rules: {
30-
'color-contrast': {
31-
enabled: theme !== 'dark_dimmed',
32-
},
33-
},
24+
test('axe @aat', async ({page}) => {
25+
await visit(page, {
26+
id: 'components-avatar--default',
27+
globals: {
28+
colorScheme: theme,
29+
primer_react_css_modules_team: enabled,
30+
},
31+
})
32+
await expect(page).toHaveNoViolations({
33+
rules: {
34+
'color-contrast': {
35+
enabled: theme !== 'dark_dimmed',
36+
},
37+
},
38+
})
39+
})
3440
})
35-
})
41+
}
3642
})
37-
}
38-
})
3943

40-
test.describe('Size', () => {
41-
for (const theme of themes) {
42-
test.describe(theme, () => {
43-
test('default @vrt', async ({page}) => {
44-
await visit(page, {
45-
id: 'components-avatar-features--size',
46-
globals: {
47-
colorScheme: theme,
48-
},
49-
})
44+
test.describe('Size', () => {
45+
for (const theme of themes) {
46+
test.describe(theme, () => {
47+
test('default @vrt', async ({page}) => {
48+
await visit(page, {
49+
id: 'components-avatar-features--size',
50+
globals: {
51+
colorScheme: theme,
52+
primer_react_css_modules_team: enabled,
53+
},
54+
})
5055

51-
// Default state
52-
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Size.${theme}.png`)
53-
})
56+
// Default state
57+
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Size.${theme}.png`)
58+
})
5459

55-
test('axe @aat', async ({page}) => {
56-
await visit(page, {
57-
id: 'components-avatar-features--size',
58-
globals: {
59-
colorScheme: theme,
60-
},
61-
})
62-
await expect(page).toHaveNoViolations({
63-
rules: {
64-
'color-contrast': {
65-
enabled: theme !== 'dark_dimmed',
66-
},
67-
},
60+
test('axe @aat', async ({page}) => {
61+
await visit(page, {
62+
id: 'components-avatar-features--size',
63+
globals: {
64+
colorScheme: theme,
65+
primer_react_css_modules_team: enabled,
66+
},
67+
})
68+
await expect(page).toHaveNoViolations({
69+
rules: {
70+
'color-contrast': {
71+
enabled: theme !== 'dark_dimmed',
72+
},
73+
},
74+
})
75+
})
6876
})
69-
})
77+
}
7078
})
71-
}
72-
})
7379

74-
test.describe('Size Responsive', () => {
75-
for (const theme of themes) {
76-
test.describe(theme, () => {
77-
test('default @vrt', async ({page}) => {
78-
await visit(page, {
79-
id: 'components-avatar-features--size-responsive',
80-
globals: {
81-
colorScheme: theme,
82-
},
83-
})
80+
test.describe('Size Responsive', () => {
81+
for (const theme of themes) {
82+
test.describe(theme, () => {
83+
test('default @vrt', async ({page}) => {
84+
await visit(page, {
85+
id: 'components-avatar-features--size-responsive',
86+
globals: {
87+
colorScheme: theme,
88+
primer_react_css_modules_team: enabled,
89+
},
90+
})
8491

85-
// Default state
86-
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Size Responsive.${theme}.png`)
87-
})
92+
// Default state
93+
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Size Responsive.${theme}.png`)
94+
})
8895

89-
test('axe @aat', async ({page}) => {
90-
await visit(page, {
91-
id: 'components-avatar-features--size-responsive',
92-
globals: {
93-
colorScheme: theme,
94-
},
95-
})
96-
await expect(page).toHaveNoViolations({
97-
rules: {
98-
'color-contrast': {
99-
enabled: theme !== 'dark_dimmed',
100-
},
101-
},
96+
test('axe @aat', async ({page}) => {
97+
await visit(page, {
98+
id: 'components-avatar-features--size-responsive',
99+
globals: {
100+
colorScheme: theme,
101+
primer_react_css_modules_team: enabled,
102+
},
103+
})
104+
await expect(page).toHaveNoViolations({
105+
rules: {
106+
'color-contrast': {
107+
enabled: theme !== 'dark_dimmed',
108+
},
109+
},
110+
})
111+
})
102112
})
103-
})
113+
}
104114
})
105-
}
106-
})
107115

108-
test.describe('Square', () => {
109-
for (const theme of themes) {
110-
test.describe(theme, () => {
111-
test('default @vrt', async ({page}) => {
112-
await visit(page, {
113-
id: 'components-avatar-features--square',
114-
globals: {
115-
colorScheme: theme,
116-
},
117-
})
116+
test.describe('Square', () => {
117+
for (const theme of themes) {
118+
test.describe(theme, () => {
119+
test('default @vrt', async ({page}) => {
120+
await visit(page, {
121+
id: 'components-avatar-features--square',
122+
globals: {
123+
colorScheme: theme,
124+
primer_react_css_modules_team: enabled,
125+
},
126+
})
118127

119-
// Default state
120-
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Square.${theme}.png`)
121-
})
128+
// Default state
129+
expect(await page.screenshot()).toMatchSnapshot(`Avatar.Square.${theme}.png`)
130+
})
122131

123-
test('axe @aat', async ({page}) => {
124-
await visit(page, {
125-
id: 'components-avatar-features--square',
126-
globals: {
127-
colorScheme: theme,
128-
},
129-
})
130-
await expect(page).toHaveNoViolations({
131-
rules: {
132-
'color-contrast': {
133-
enabled: theme !== 'dark_dimmed',
134-
},
135-
},
132+
test('axe @aat', async ({page}) => {
133+
await visit(page, {
134+
id: 'components-avatar-features--square',
135+
globals: {
136+
colorScheme: theme,
137+
primer_react_css_modules_team: enabled,
138+
},
139+
})
140+
await expect(page).toHaveNoViolations({
141+
rules: {
142+
'color-contrast': {
143+
enabled: theme !== 'dark_dimmed',
144+
},
145+
},
146+
})
147+
})
136148
})
137-
})
149+
}
138150
})
139-
}
140-
})
151+
})
152+
}
141153
})

‎packages/react/src/Avatar/Avatar.features.stories.tsx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ export default {
99

1010
export const Square = () => <Avatar square alt="primer" src="https://avatars.githubusercontent.com/primer" />
1111

12+
export const SquareSxProp = () => (
13+
<Avatar
14+
square
15+
sx={{border: '1px solid red'}}
16+
size={40}
17+
alt="primer"
18+
src="https://avatars.githubusercontent.com/primer"
19+
/>
20+
)
21+
1222
export const Size = () => (
1323
<div>
1424
<Avatar size={4} alt="mona" src="https://avatars.githubusercontent.com/u/92997159?v=4" />

‎packages/react/src/Avatar/Avatar.module.css‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
:where(.Avatar) {
2+
display: inline-block;
3+
width: var(--avatarSize-regular);
4+
height: var(--avatarSize-regular);
5+
overflow: hidden; /* Ensure page layout in Firefox should images fail to load */
6+
line-height: 1;
7+
vertical-align: middle;
8+
border-radius: 50%;
9+
box-shadow: 0 0 0 1px var(--avatar-borderColor);
10+
11+
&:where([data-square]) {
12+
border-radius: var(--borderRadius-medium);
13+
}
14+
15+
&:where([data-responsive]) {
16+
@media screen and (--viewportRange-narrow) {
17+
width: var(--avatarSize-narrow);
18+
height: var(--avatarSize-narrow);
19+
}
20+
21+
@media screen and (--viewportRange-regular) {
22+
width: var(--avatarSize-regular);
23+
height: var(--avatarSize-regular);
24+
}
25+
26+
@media screen and (--viewportRange-wide) {
27+
width: var(--avatarSize-wide);
28+
height: var(--avatarSize-wide);
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)