Skip to content

Commit f25da81

Browse files
author
Luke Bowerman
authored
Avatar Improvements (#1249)
Improved a11y, eventHandling, tests, role="button"
1 parent 67c6f12 commit f25da81

File tree

9 files changed

+354
-87
lines changed

9 files changed

+354
-87
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- `AvatarCombo`, `AvatarIcon` & `AvatarUser` now supports `role="button"`
13+
- Added support for common DOM properties and event handlers (e.g.: `onClick`)
14+
- Improved a11y for all `Avatar*` components
15+
- Added Storybook with knobs
1216
- `IconButton` now supports `tooltipWidth` property
1317
- `Popover` now supports `cancelClickOutside` (`true` by default) to determine whether the "dismissal" click event is allowed to propagate
1418
- New `Icon` artwork `Logout`, `AddComment`, `Comment`, `Feedback`

‎packages/components/src/Avatar/Avatar.test.tsx‎

Lines changed: 105 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -24,74 +24,116 @@
2424
2525
*/
2626

27-
import { assertSnapshot } from '@looker/components-test-utils'
27+
import { assertSnapshot, renderWithTheme } from '@looker/components-test-utils'
28+
import { screen, fireEvent } from '@testing-library/react'
2829
import React from 'react'
2930
import { AvatarCombo } from './AvatarCombo'
3031
import { AvatarIcon } from './AvatarIcon'
3132
import { AvatarUser } from './AvatarUser'
3233

3334
/* eslint-disable @typescript-eslint/camelcase */
3435

35-
test('AvatarCombo renders Avatar and its secondary avatar', () => {
36-
const data = {
37-
avatar_url:
38-
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
39-
first_name: 'John',
40-
last_name: 'Smith',
41-
}
42-
assertSnapshot(<AvatarCombo secondaryIcon="Code" user={data} />)
43-
})
44-
45-
test('AvatarCombo renders Avatar initials and secondary with Code icon', () => {
46-
const data = {
47-
avatar_url:
48-
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
49-
first_name: 'John',
50-
last_name: 'Smith',
51-
}
52-
assertSnapshot(<AvatarCombo secondaryIcon="Code" user={data} />)
53-
})
54-
55-
test('AvatarCombo renders AvatarIcon and secondary avatar if user is not available and updates icon if passed.', () => {
56-
assertSnapshot(<AvatarCombo secondaryIcon="LogoRings" />)
57-
})
58-
59-
test('AvatarIcon renders ', () => {
60-
assertSnapshot(<AvatarIcon />)
61-
})
62-
63-
test('AvatarIcon renders different icon if specified', () => {
64-
assertSnapshot(<AvatarIcon icon="Code" />)
65-
})
66-
67-
test('AvatarUser shows user profile picture if it has good avatar_url ', () => {
68-
const data = {
69-
avatar_url:
70-
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
71-
first_name: 'John',
72-
last_name: 'Smith',
73-
}
74-
75-
assertSnapshot(<AvatarUser user={data} />)
76-
})
77-
78-
test('AvatarUser shows initials if has broken url as avatar_url', () => {
79-
const data = {
80-
avatar_url:
81-
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
82-
first_name: 'John',
83-
last_name: 'Smith',
84-
}
85-
86-
assertSnapshot(<AvatarUser user={data} />)
87-
})
88-
89-
test('AvatarUser shows initials if it has null as avatar_url ', () => {
90-
const data = {
91-
avatar_url: null,
92-
first_name: 'John',
93-
last_name: 'Smith',
94-
}
95-
96-
assertSnapshot(<AvatarUser user={data} />)
36+
describe('Avatar', () => {
37+
describe('AvatarCombo', () => {
38+
test('renders Avatar and its secondary avatar', () => {
39+
const data = {
40+
avatar_url:
41+
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
42+
first_name: 'John',
43+
last_name: 'Smith',
44+
}
45+
assertSnapshot(<AvatarCombo secondaryIcon="Code" user={data} />)
46+
})
47+
48+
test('renders Avatar initials and secondary with Code icon', () => {
49+
const data = {
50+
avatar_url:
51+
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
52+
first_name: 'John',
53+
last_name: 'Smith',
54+
}
55+
assertSnapshot(<AvatarCombo secondaryIcon="Code" user={data} />)
56+
})
57+
58+
test('renders AvatarIcon and secondary avatar if user is not available and updates icon if passed.', () => {
59+
assertSnapshot(<AvatarCombo secondaryIcon="LogoRings" />)
60+
})
61+
62+
test('supports role as button & onClick event', () => {
63+
const fauxOnClick = jest.fn()
64+
renderWithTheme(<AvatarCombo onClick={fauxOnClick} role="button" />)
65+
66+
const button = screen.getByRole('button')
67+
68+
expect(button).toBeInTheDocument()
69+
fireEvent.click(button)
70+
expect(fauxOnClick.mock.calls.length).toBe(1)
71+
})
72+
})
73+
74+
describe('AvatarIcon', () => {
75+
test('renders ', () => {
76+
assertSnapshot(<AvatarIcon />)
77+
})
78+
79+
test('supports role as button & onClick event', () => {
80+
const fauxOnClick = jest.fn()
81+
renderWithTheme(<AvatarIcon onClick={fauxOnClick} role="button" />)
82+
83+
const button = screen.getByRole('button')
84+
85+
expect(button).toBeInTheDocument()
86+
fireEvent.click(button)
87+
expect(fauxOnClick.mock.calls.length).toBe(1)
88+
})
89+
90+
test('renders different icon if specified', () => {
91+
assertSnapshot(<AvatarIcon icon="Code" />)
92+
})
93+
})
94+
95+
describe('AvatarUser', () => {
96+
test('shows user profile picture if it has good avatar_url ', () => {
97+
const data = {
98+
avatar_url:
99+
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
100+
first_name: 'John',
101+
last_name: 'Smith',
102+
}
103+
104+
assertSnapshot(<AvatarUser user={data} />)
105+
})
106+
107+
test('shows initials if has broken url as avatar_url', () => {
108+
const data = {
109+
avatar_url:
110+
'https://gravatar.lookercdn.com/avatar/e8ebbdf1a64411721503995731?s=156&d=blank',
111+
first_name: 'John',
112+
last_name: 'Smith',
113+
}
114+
115+
assertSnapshot(<AvatarUser user={data} />)
116+
})
117+
118+
test('shows initials if it has null as avatar_url ', () => {
119+
const data = {
120+
avatar_url: null,
121+
first_name: 'John',
122+
last_name: 'Smith',
123+
}
124+
125+
assertSnapshot(<AvatarUser user={data} />)
126+
})
127+
})
128+
129+
test('supports role as button & onClick event', () => {
130+
const fauxOnClick = jest.fn()
131+
renderWithTheme(<AvatarUser onClick={fauxOnClick} role="button" />)
132+
133+
const button = screen.getByRole('button')
134+
135+
expect(button).toBeInTheDocument()
136+
fireEvent.click(button)
137+
expect(fauxOnClick.mock.calls.length).toBe(1)
138+
})
97139
})

‎packages/components/src/Avatar/Avatar.tsx‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
SizeSmall,
3636
SizeXSmall,
3737
SizeXXSmall,
38+
CompatibleHTMLProps,
3839
} from '@looker/design-tokens'
3940
import { css } from 'styled-components'
4041
import { variant } from 'styled-system'
@@ -46,7 +47,9 @@ export type AvatarSizes =
4647
| SizeMedium
4748
| SizeLarge
4849

49-
export interface AvatarProps extends SpaceProps {
50+
export interface AvatarProps
51+
extends SpaceProps,
52+
CompatibleHTMLProps<HTMLElement> {
5053
className?: string
5154
/**
5255
* @default `key`
@@ -57,6 +60,12 @@ export interface AvatarProps extends SpaceProps {
5760
* @default "small"
5861
*/
5962
size?: AvatarSizes | string
63+
64+
/**
65+
* Render as a button instead of a div
66+
* @default false (renders as <div />)
67+
*/
68+
role?: 'button'
6069
}
6170

6271
/* eslint-disable sort-keys-fix/sort-keys-fix */
@@ -100,8 +109,15 @@ export const avatarCSS = css`
100109
${size}
101110
102111
align-items: center;
112+
border: none;
113+
/* Need this in case Avatar is rendered as a <button /> */
103114
border-radius: 100%;
115+
${({ role }: AvatarProps) => role === 'button' && 'cursor: pointer;'}
104116
display: grid;
105117
justify-items: center;
106118
overflow: hidden;
119+
120+
&:focus {
121+
outline: none;
122+
}
107123
`

‎packages/components/src/Avatar/AvatarCombo.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import React, { FC } from 'react'
2828
import styled from 'styled-components'
29+
import { omitStyledProps } from '@looker/design-tokens'
2930
import { IconNames } from '@looker/icons'
3031
import { Icon } from '../Icon'
3132
import { AvatarUser, AvatarUserProps } from './AvatarUser'
@@ -53,10 +54,13 @@ const AvatarLayout: FC<AvatarComboProps> = ({
5354
color,
5455
icon = 'User',
5556
user,
56-
className,
57+
role,
58+
...props
5759
}) => {
60+
const BaseElement = role === 'button' ? 'button' : 'div'
61+
5862
return (
59-
<div className={className}>
63+
<BaseElement {...omitStyledProps(props)}>
6064
{user ? (
6165
<AvatarUser user={user} color={color} />
6266
) : (
@@ -67,7 +71,7 @@ const AvatarLayout: FC<AvatarComboProps> = ({
6771
color={secondaryColor}
6872
icon={secondaryIcon}
6973
/>
70-
</div>
74+
</BaseElement>
7175
)
7276
}
7377

‎packages/components/src/Avatar/AvatarIcon.tsx‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
*/
2626

2727
import React, { FC } from 'react'
28+
import { omitStyledProps } from '@looker/design-tokens'
2829
import { IconNames } from '@looker/icons'
2930
import styled from 'styled-components'
3031
import { variant } from 'styled-system'
@@ -76,14 +77,19 @@ const size = variant({
7677
})
7778

7879
const AvatarLayout: FC<AvatarIconProps> = ({
79-
className,
8080
color,
8181
icon = 'User',
82-
}) => (
83-
<div className={className}>
84-
<Icon name={icon} color={color} />
85-
</div>
86-
)
82+
role,
83+
...props
84+
}) => {
85+
const BaseElement = role === 'button' ? 'button' : 'div'
86+
87+
return (
88+
<BaseElement {...omitStyledProps(props)}>
89+
<Icon name={icon} color={color} />
90+
</BaseElement>
91+
)
92+
}
8793

8894
export const AvatarIcon = styled(AvatarLayout)`
8995
${avatarCSS}

‎packages/components/src/Avatar/AvatarUser.tsx‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import React, { FC } from 'react'
2828
import styled from 'styled-components'
29+
import { omitStyledProps } from '@looker/design-tokens'
2930
import { avatarCSS, AvatarProps } from './Avatar'
3031

3132
export interface AvatarUserProps extends AvatarProps {
@@ -38,24 +39,33 @@ export interface AvatarUserProps extends AvatarProps {
3839

3940
const AvatarLayout: FC<AvatarUserProps> = ({
4041
color,
41-
className,
4242
user,
43+
role,
4344
size,
45+
...props
4446
}) => {
4547
const firstInitial = user && user.first_name && user.first_name[0]
4648
const lastInitial = user && user.last_name && user.last_name[0]
49+
const name = user ? `${user.first_name} ${user.last_name}` : 'Avatar'
50+
51+
const BaseElement = role === 'button' ? 'button' : 'div'
4752

4853
return (
49-
<div className={className}>
50-
<AvatarInitials color={color}>
54+
<BaseElement {...omitStyledProps(props)} aria-label={name}>
55+
<AvatarInitials color={color} aria-hidden>
5156
{size === 'xxsmall'
5257
? `${firstInitial}`
5358
: `${firstInitial}${lastInitial}`}
5459
</AvatarInitials>
5560
{user && user.avatar_url && (
56-
<AvatarPhoto color={color} type="image/png" data={user.avatar_url} />
61+
<AvatarPhoto
62+
aria-hidden
63+
color={color}
64+
type="image/png"
65+
data={user.avatar_url}
66+
/>
5767
)}
58-
</div>
68+
</BaseElement>
5969
)
6070
}
6171

0 commit comments

Comments
 (0)