From 204af4167f61641c0b6f035fc25f75c350755bba Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 12 Apr 2022 14:57:33 +0200 Subject: [PATCH 1/5] Refactor Avatar to use Box --- src/Avatar.tsx | 39 ++++++++----------- .../__snapshots__/Avatar.test.tsx.snap | 5 +-- .../__snapshots__/Token.test.tsx.snap | 35 ++++------------- src/stories/Avatar.stories.tsx | 36 +++++++++++++++++ 4 files changed, 61 insertions(+), 54 deletions(-) create mode 100644 src/stories/Avatar.stories.tsx diff --git a/src/Avatar.tsx b/src/Avatar.tsx index 857ad7d0984..e3d8c8965a9 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -1,9 +1,8 @@ -import styled from 'styled-components' -import {get} from './constants' -import sx, {SxProp} from './sx' -import {ComponentProps} from './utils/types' +import React from 'react' +import Box from './Box' +import {SxProp, merge} from './sx' -type StyledAvatarProps = { +export type AvatarProps = { /** Sets the width and height of the avatar. */ size?: number /** Sets the shape of the avatar to a square if true. If false, the avatar will be circular. */ @@ -14,7 +13,7 @@ type StyledAvatarProps = { alt?: string } & SxProp -function getBorderRadius({size, square}: StyledAvatarProps) { +function getBorderRadius({size, square}: Pick) { if (square) { return size && size <= 24 ? '4px' : '6px' } else { @@ -22,23 +21,17 @@ function getBorderRadius({size, square}: StyledAvatarProps) { } } -const Avatar = styled.img.attrs(props => ({ - height: props.size, - width: props.size -}))` - display: inline-block; - overflow: hidden; // Ensure page layout in Firefox should images fail to load - line-height: ${get('lineHeights.condensedUltra')}; - vertical-align: middle; - border-radius: ${props => getBorderRadius(props)}; - ${sx} -` - -Avatar.defaultProps = { - size: 20, - alt: '', - square: false +export const Avatar: React.FC = ({size = 20, alt = '', square = false, sx = {}, ...props}) => { + const styles = { + display: 'inline-block', + overflow: 'hidden', + lineHeight: 'condensedUltra', + verticalAlign: 'middle', + width: size, + height: size, + borderRadius: getBorderRadius({size, square}) + } + return } -export type AvatarProps = ComponentProps export default Avatar diff --git a/src/__tests__/__snapshots__/Avatar.test.tsx.snap b/src/__tests__/__snapshots__/Avatar.test.tsx.snap index 6c84752e4de..5eb4cdf3a9e 100644 --- a/src/__tests__/__snapshots__/Avatar.test.tsx.snap +++ b/src/__tests__/__snapshots__/Avatar.test.tsx.snap @@ -6,14 +6,13 @@ exports[`Avatar renders consistently 1`] = ` overflow: hidden; line-height: 1; vertical-align: middle; + width: 20px; + height: 20px; border-radius: 50%; } `; diff --git a/src/__tests__/__snapshots__/Token.test.tsx.snap b/src/__tests__/__snapshots__/Token.test.tsx.snap index f16b0d27a3a..7df83c10b86 100644 --- a/src/__tests__/__snapshots__/Token.test.tsx.snap +++ b/src/__tests__/__snapshots__/Token.test.tsx.snap @@ -6,9 +6,9 @@ exports[`Token components AvatarToken renders all sizes 1`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -166,10 +166,7 @@ exports[`Token components AvatarToken renders all sizes 1`] = ` @@ -217,9 +214,9 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -377,10 +374,7 @@ exports[`Token components AvatarToken renders all sizes 2`] = ` @@ -428,9 +422,9 @@ exports[`Token components AvatarToken renders all sizes 3`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -588,10 +582,7 @@ exports[`Token components AvatarToken renders all sizes 3`] = ` @@ -639,9 +630,9 @@ exports[`Token components AvatarToken renders all sizes 4`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -799,10 +790,7 @@ exports[`Token components AvatarToken renders all sizes 4`] = ` @@ -850,9 +838,9 @@ exports[`Token components AvatarToken renders all sizes 5`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -1010,10 +998,7 @@ exports[`Token components AvatarToken renders all sizes 5`] = ` @@ -1144,9 +1129,9 @@ exports[`Token components AvatarToken renders isSelected 1`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c4 { @@ -1252,10 +1237,7 @@ exports[`Token components AvatarToken renders isSelected 1`] = ` @@ -1273,9 +1255,9 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` overflow: hidden; line-height: 1; vertical-align: middle; - border-radius: 50%; width: 100%; height: 100%; + border-radius: 50%; } .c5 { @@ -1433,10 +1415,7 @@ exports[`Token components AvatarToken renders with a remove button 1`] = ` diff --git a/src/stories/Avatar.stories.tsx b/src/stories/Avatar.stories.tsx new file mode 100644 index 00000000000..10042dbbb76 --- /dev/null +++ b/src/stories/Avatar.stories.tsx @@ -0,0 +1,36 @@ +import Avatar from '../Avatar' +import {Meta} from '@storybook/react' +import React from 'react' +import {ThemeProvider} from '..' +import BaseStyles from '../BaseStyles' + +const meta: Meta = { + title: 'Building blocks/Avatar/fixtures', + component: Avatar, + decorators: [ + (Story: React.ComponentType): JSX.Element => ( + + + + + + ) + ] +} +export default meta + +export function SimpleAvatar(): JSX.Element { + return +} + +export function CustomSize(): JSX.Element { + return +} + +export function SquareAvatar(): JSX.Element { + return +} + +export function DefaultAltText(): JSX.Element { + return +} From 07cc791e8bbb8ef532b53fb2dba72e6efec382bd Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 12 Apr 2022 15:06:12 +0200 Subject: [PATCH 2/5] support ref --- src/Avatar.tsx | 8 +++++--- src/stories/Avatar.stories.tsx | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index e3d8c8965a9..4a122bfa5e7 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -21,7 +21,9 @@ function getBorderRadius({size, square}: Pick) { } } -export const Avatar: React.FC = ({size = 20, alt = '', square = false, sx = {}, ...props}) => { +export const Avatar = React.forwardRef((props, ref) => { + const {size = 20, alt = '', square = false, sx = {}, ...rest} = props + const styles = { display: 'inline-block', overflow: 'hidden', @@ -31,7 +33,7 @@ export const Avatar: React.FC = ({size = 20, alt = '', square = fal height: size, borderRadius: getBorderRadius({size, square}) } - return -} + return +}) export default Avatar diff --git a/src/stories/Avatar.stories.tsx b/src/stories/Avatar.stories.tsx index 10042dbbb76..9b6e06bf806 100644 --- a/src/stories/Avatar.stories.tsx +++ b/src/stories/Avatar.stories.tsx @@ -34,3 +34,9 @@ export function SquareAvatar(): JSX.Element { export function DefaultAltText(): JSX.Element { return } + +export function AcceptsRef(): JSX.Element { + const ref = React.useRef(null) + + return +} From 0d8aee445112ece5d985fd616a25327d286dfdaa Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 14 Apr 2022 12:46:50 +0200 Subject: [PATCH 3/5] passing tests --- src/Avatar.tsx | 3 ++- src/__tests__/Avatar.test.tsx | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index 4a122bfa5e7..e9e8c7e58c8 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -21,7 +21,7 @@ function getBorderRadius({size, square}: Pick) { } } -export const Avatar = React.forwardRef((props, ref) => { +const Avatar = React.forwardRef((props, ref) => { const {size = 20, alt = '', square = false, sx = {}, ...rest} = props const styles = { @@ -36,4 +36,5 @@ export const Avatar = React.forwardRef((props, re return }) +Avatar.displayName = 'Avatar' export default Avatar diff --git a/src/__tests__/Avatar.test.tsx b/src/__tests__/Avatar.test.tsx index 24ce518a3be..3030bb0bc82 100644 --- a/src/__tests__/Avatar.test.tsx +++ b/src/__tests__/Avatar.test.tsx @@ -24,14 +24,16 @@ describe('Avatar', () => { it('renders small by default', () => { const size = 20 const result = render() - expect(result.props.width).toEqual(size) - expect(result.props.height).toEqual(size) + + expect(result).toHaveStyleRule('width', px(size)) + expect(result).toHaveStyleRule('height', px(size)) }) it('respects the size prop', () => { const result = render() - expect(result.props.width).toEqual(40) - expect(result.props.height).toEqual(40) + + expect(result).toHaveStyleRule('width', '40px') + expect(result).toHaveStyleRule('height', '40px') }) it('passes through the src prop', () => { From e41e849bba800e4068246d4d86bdf5a7d1bb766a Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Apr 2022 14:43:30 +0200 Subject: [PATCH 4/5] use BetterSystemStyleObject for types --- src/Avatar.tsx | 6 +++--- src/stories/Avatar.stories.tsx | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index e9e8c7e58c8..f9efc785125 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -1,6 +1,6 @@ import React from 'react' import Box from './Box' -import {SxProp, merge} from './sx' +import {SxProp, merge, BetterSystemStyleObject} from './sx' export type AvatarProps = { /** Sets the width and height of the avatar. */ @@ -24,7 +24,7 @@ function getBorderRadius({size, square}: Pick) { const Avatar = React.forwardRef((props, ref) => { const {size = 20, alt = '', square = false, sx = {}, ...rest} = props - const styles = { + const styles: BetterSystemStyleObject = { display: 'inline-block', overflow: 'hidden', lineHeight: 'condensedUltra', @@ -33,7 +33,7 @@ const Avatar = React.forwardRef((props, ref) => { height: size, borderRadius: getBorderRadius({size, square}) } - return + return (styles, sx)} {...rest} /> }) Avatar.displayName = 'Avatar' diff --git a/src/stories/Avatar.stories.tsx b/src/stories/Avatar.stories.tsx index 9b6e06bf806..add34efc614 100644 --- a/src/stories/Avatar.stories.tsx +++ b/src/stories/Avatar.stories.tsx @@ -40,3 +40,12 @@ export function AcceptsRef(): JSX.Element { return } + +export function AcceptsSxProp(): JSX.Element { + return ( + <> + + text pushed to right because avatar has margin + + ) +} From f1fcc62618bb02e837a5ec44f349d0ee5d8c5252 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 18 May 2022 12:20:41 +0200 Subject: [PATCH 5/5] add img props to Avatar --- src/Avatar.tsx | 3 ++- src/stories/Avatar.stories.tsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Avatar.tsx b/src/Avatar.tsx index f9efc785125..f6018d0213a 100644 --- a/src/Avatar.tsx +++ b/src/Avatar.tsx @@ -11,7 +11,8 @@ export type AvatarProps = { src: string /** Provide alt text when the Avatar is used without the user's name next to it. */ alt?: string -} & SxProp +} & SxProp & + Omit, 'ref'> function getBorderRadius({size, square}: Pick) { if (square) { diff --git a/src/stories/Avatar.stories.tsx b/src/stories/Avatar.stories.tsx index add34efc614..23760d9b050 100644 --- a/src/stories/Avatar.stories.tsx +++ b/src/stories/Avatar.stories.tsx @@ -38,7 +38,7 @@ export function DefaultAltText(): JSX.Element { export function AcceptsRef(): JSX.Element { const ref = React.useRef(null) - return + return } export function AcceptsSxProp(): JSX.Element {