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

Add <Avatar> to display an image along with the text #208

Merged
merged 15 commits into from
May 3, 2019
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [Core] Add `verticalOrder` prop to `<Text>` so you can swap the position of `basic` and `aside`. Also applied to `rowComp()` mixin.
- [Core] Rewrite `<TextInput>` to match latest design, offering single-line `<input>`, multi-line `<textarea>` and supports custom rendering via render prop.
- [Form] `<TextInputRow>` now renders the new `<TextInput>` and forwards almost every prop to it, **without** a ref to its inner input.
- [Core] Add `<Avatar>` to display an image.

### Changed
- [Core] Refactored `closable()` mixin to detect inside/outside clicks via React SyntheticEvent mechanism instead of listening native events from DOM.
- [Storybook] Fix mangled component name in storybook build. (#203)
- [Storybook] Update examples for core `<TextInput>` and form `<TextInputRow>`. (#203)
- [Core] Change `rowComp()` to allow the appearance of `<Avatar>` alongside the text. (#208)
- [Core] Change `<Checkbox>` to display `<Avatar>`. (#208)
- [Form] Change `<SelectRow>` and `<Checkbox>` to display `<Avatar>`. (#208)
- [Storybook] Add examples for `<Avatar>` and the list components with `<Avatar>`s. (#208)

## [2.1.0]
### Changed
Expand Down
46 changes: 46 additions & 0 deletions packages/core/src/Avatar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';

import './styles/Avatar.scss';

import icBEM from './utils/icBEM';
import prefixClass from './utils/prefixClass';

const COMPONENT_NAME = prefixClass('avatar');
const ROOT_BEM = icBEM(COMPONENT_NAME);

const SQUARE = 'square';
const ROUNDED = 'rounded';
const CIRCLE = 'circle';
export const AVATAR_TYPE = { SQUARE, ROUNDED, CIRCLE };

function Avatar({
className,
src,
alt,
type,
...otherProps
}) {
const bemClass = ROOT_BEM.modifier(type);

const rootClassName = classNames(className, `${bemClass}`);

return (
<div className={rootClassName} {...otherProps}>
<img alt={alt} src={src} />
</div>
);
}

Avatar.propTypes = {
src: PropTypes.string.isRequired,
alt: PropTypes.string.isRequired,
type: PropTypes.oneOf(Object.values(AVATAR_TYPE)),
};

Avatar.defaultProps = {
type: SQUARE,
};

export default Avatar;
28 changes: 28 additions & 0 deletions packages/core/src/__tests__/Avatar.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React from 'react';
import ReactDOM from 'react-dom';
import { shallow } from 'enzyme';

import Avatar from '../Avatar';

describe('<Avatar>', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
const element = <Avatar src="LINK" alt="ALT" />;

ReactDOM.render(element, div);
});

it('handles type modifiers', () => {
let wrapper = shallow(<Avatar src="LINK" alt="ALT" />);
expect(wrapper.hasClass('gyp-avatar--square')).toBeTruthy();

wrapper = shallow(<Avatar type="square" src="LINK" alt="ALT" />);
expect(wrapper.hasClass('gyp-avatar--square')).toBeTruthy();

wrapper = shallow(<Avatar type="rounded" src="LINK" alt="ALT" />);
expect(wrapper.hasClass('gyp-avatar--rounded')).toBeTruthy();

wrapper = shallow(<Avatar type="circle" src="LINK" alt="ALT" />);
expect(wrapper.hasClass('gyp-avatar--circle')).toBeTruthy();
});
});
2 changes: 2 additions & 0 deletions packages/core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import EditableBasicRow from './EditableBasicRow';
import Icon from './Icon';
import StatusIcon from './StatusIcon';
import Tag from './Tag';
import Avatar from './Avatar';
import Text from './Text';
import EditableText from './EditableText';
import Tooltip from './Tooltip';
Expand Down Expand Up @@ -49,6 +50,7 @@ export {
StatusIcon,
Tag,
Text,
Avatar,
EditableText,
Tooltip,
SwitchIcon,
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/mixins/rowComp.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const rowComp = ({
PropTypes.element
]),
basic: PropTypes.node,
avatar: PropTypes.node,
aside: PropTypes.node,
tag: PropTypes.node,
bold: PropTypes.bool,
Expand All @@ -155,6 +156,7 @@ const rowComp = ({
verticalOrder: defaultVerticalOrder,
icon: null,
basic: null,
avatar: null,
aside: null,
tag: null,
bold: false,
Expand Down Expand Up @@ -237,7 +239,7 @@ const rowComp = ({
render() {
const {
minified,

avatar,
align,
verticalOrder,
icon,
Expand Down Expand Up @@ -276,6 +278,7 @@ const rowComp = ({

return (
<WrappedComponent className={wrapperClassName} {...otherProps}>
{avatar}
{children || this.renderContent()}
</WrappedComponent>
);
Expand Down
51 changes: 51 additions & 0 deletions packages/core/src/styles/Avatar.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
@import "./mixins";

// -------------------------------------
// Component Block
// -------------------------------------
.#{$prefix}-avatar {
width: 2.2rem;
height: 2.2rem;
overflow: hidden;
margin: .2rem .45rem .2rem .25rem;
background-color: $c-gray;
flex-shrink: 0;

// ----------------------
// <Avatar> variants
// ----------------------
&--square {
border-radius: 0;
}

&--rounded {
border-radius: 6px;
}

&--circle {
border-radius: 50%;
}

// ----------------------
// Inner image
// ----------------------
> img {
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably need a & here?

Suggested change
> img {
& > img {

width: 100%;
height: 100%;
text-align: center;
object-fit: cover;
}
}

// Override the default left margin on <ListRow>
.#{$prefix}-list-row > .#{$prefix}-avatar {
&:first-child {
margin-left: 0;
}
}

.#{$prefix}-list-row__body > .#{$prefix}-avatar {
&:first-child {
margin-left: 0;
}
}
2 changes: 2 additions & 0 deletions packages/form/src/SelectOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const TYPE_SYMBOL = Symbol('SelectOption');
function SelectOption({
label,
value,
avatar,
Copy link
Contributor

Choose a reason for hiding this comment

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

missing type check here?

readOnly,
checked,
onChange,
Expand All @@ -32,6 +33,7 @@ function SelectOption({
checked={checked}
disabled={readOnly}
basic={label}
avatar={avatar}
onChange={handleCheckboxChange}
{...checkboxProps} />
</ListRow>
Expand Down
30 changes: 30 additions & 0 deletions packages/form/src/SelectRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,22 @@ function getValueLabelMap(fromChildren = []) {
return resultMap;
}

/**
* Generate a value-avatar map from all `<SelectOption>`s.
*
* @param {array} fromOptions
* @return {Map}
*/
function getValueAvatarMap(fromChildren = []) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be merged with getValueLabelMap so we don't need to iterate the Array twice, only for deriving different data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion. Thanks!

const resultMap = new Map();
const options = parseSelectOptions(fromChildren);

options.forEach(
option => resultMap.set(option.value, option.avatar)
);
return resultMap;
}

class SelectRow extends PureComponent {
static propTypes = {
label: PropTypes.node.isRequired,
Expand Down Expand Up @@ -85,12 +101,14 @@ class SelectRow extends PureComponent {
state = {
popoverOpen: false,
valueLabelMap: getValueLabelMap(this.props.children),
avatarLabelMap: getValueAvatarMap(this.props.children),
cachedValues: this.props.values || this.props.defaultValues,
};

componentWillReceiveProps(nextProps) {
this.setState({
valueLabelMap: getValueLabelMap(nextProps.children),
avatarLabelMap: getValueAvatarMap(nextProps.children),
});

if (this.getIsControlled(nextProps)) {
Expand Down Expand Up @@ -156,6 +174,17 @@ class SelectRow extends PureComponent {
.join(asideSeparator);
}

renderAvatar() {
const { cachedValues, avatarLabelMap } = this.state;

if (cachedValues.length === 0) {
return null;
}

return cachedValues
.map(value => avatarLabelMap.get(value));
}

render() {
const {
label,
Expand Down Expand Up @@ -189,6 +218,7 @@ class SelectRow extends PureComponent {

return (
<ListRow className={wrapperClassName} {...rowProps}>
{this.renderAvatar()}
<Content minified={false} disabled={disabled} {...contentProps}>
<Text
bold={!ineditable}
Expand Down
18 changes: 18 additions & 0 deletions packages/form/src/__tests__/SelectRow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import ReactDOM from 'react-dom';
import { shallow } from 'enzyme';

import {
Avatar,
Button,
Popover,
Text,
Expand Down Expand Up @@ -182,6 +183,23 @@ describe('Pure <SelectRow>: Data', () => {
expect(wrapper.find(Text).prop('aside')).toBe('All');
});

it('renders the avatar', () => {
const fooAvatar = <Avatar alt="foo" src="FOO_SRC" />;
const barAvatar = <Avatar alt="bar" src="BAR_SRC" />;

const wrapper = shallow(
<PureSelectRow label="Select" values={['foo']}>
<Option label="foo" value="foo" avatar={fooAvatar} />
<Option label="bar" value="bar" avatar={barAvatar} />
</PureSelectRow>
);

expect(wrapper.find(Avatar).prop('src')).toEqual('FOO_SRC');

wrapper.setProps({ values: ['bar'] });
expect(wrapper.find(Avatar).prop('src')).toEqual('BAR_SRC');
});

it('can customize aside labels', () => {
const wrapper = shallow(
<PureSelectRow multiple label="Select" values={[]} asideNone="None">
Expand Down
17 changes: 17 additions & 0 deletions packages/storybook/examples/core/Avatar/BasicAvatar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';

import Avatar from '@ichef/gypcrete/src/Avatar';
import FlexRow from 'utils/FlexRow';

function BasicAvatarExample() {
return (
<FlexRow>
<Avatar alt="Avatar of Design" src="https://api.adorable.io/avatars/285/design@ichef.tw" />
<Avatar type="square" alt="Avatar of RD" src="https://api.adorable.io/avatars/285/rd@ichef.tw" />
<Avatar type="rounded" alt="Avatar of Marketing" src="https://api.adorable.io/avatars/285/marketing@ichef.tw" />
<Avatar type="circle" alt="Avatar of Customer Service" src="https://api.adorable.io/avatars/285/customer_service@ichef.tw" />
</FlexRow>
);
}

export default BasicAvatarExample;
12 changes: 12 additions & 0 deletions packages/storybook/examples/core/Avatar/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { storiesOf } from '@storybook/react';
import { withInfo } from '@storybook/addon-info';

import Avatar from '@ichef/gypcrete/src/Avatar';
import getPropTables from 'utils/getPropTables';

import BasicAvatar from './BasicAvatar';

storiesOf('@ichef/gypcrete|Avatar', module)
.add('basic usage', withInfo()(BasicAvatar))
// Props table
.add('props', getPropTables([Avatar]));
10 changes: 10 additions & 0 deletions packages/storybook/examples/core/Checkbox/BasicCheckbox.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React from 'react';

import Checkbox from '@ichef/gypcrete/src/Checkbox';
import Avatar from '@ichef/gypcrete/src/Avatar';
import DebugBox from 'utils/DebugBox';

function BasicCheckboxExample() {
const rdAvatar = <Avatar type="square" alt="Avatar of RD" src="https://api.adorable.io/avatars/285/rd@ichef.tw" />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the email here to more generic one, maybe johndoe@example.com?


return (
<div>
<DebugBox>
Expand All @@ -18,6 +21,13 @@ function BasicCheckboxExample() {
tag="New" />
</DebugBox>

<DebugBox>
<Checkbox
defaultChecked
basic="Join pilot program"
avatar={rdAvatar} />
</DebugBox>

<DebugBox>
<Checkbox
defaultChecked
Expand Down
6 changes: 6 additions & 0 deletions packages/storybook/examples/core/List/NormalList.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';

import Avatar from '@ichef/gypcrete/src/Avatar';
import List from '@ichef/gypcrete/src/List';
import ListRow from '@ichef/gypcrete/src/ListRow';

Expand All @@ -13,6 +14,11 @@ function NormalList() {
return (
<DebugBox width="30rem">
<List variant="normal" title="List title" desc="Help text here">
<ListRow>
<Avatar alt="iCHEF" src="https://api.adorable.io/avatars/285/hello@ichef.tw" />
<TextLabel basic="Hello World" />
</ListRow>

<ListRow>
<TextLabel icon="tickets" basic="Hello World" />
</ListRow>
Expand Down
Loading