Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-avatar] Changed fallback to initials from user icon for avatar #2621

Merged
merged 21 commits into from
Mar 28, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
9b6a5d4
Changed fallback to initials for avatar
supreethmr Sep 3, 2019
9382491
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 5, 2019
3e9cb4c
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 9, 2019
565f141
changes of Code review comment
supreethmr Sep 10, 2019
706b591
Made Initials as Required Prop
supreethmr Sep 16, 2019
84c6038
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 19, 2019
32352b5
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 20, 2019
9a25680
added role to initials so that screen reader reads the both intials a…
supreethmr Sep 26, 2019
197e280
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 26, 2019
919d7f4
Merge branch 'master' into Added_Initial_fallback
supreethmr Sep 27, 2019
4698f86
screenshot update
supreethmr Sep 27, 2019
64b7c72
Update packages/terra-avatar/docs/avatar.md
supreethmr Oct 15, 2019
e7b8744
removed obselete snapshots
supreethmr Oct 16, 2019
ce1f8fe
Removing Tests which were not required with new changes
supreethmr Oct 16, 2019
d8098a5
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel Oct 28, 2019
87fde47
Merge branch 'master' into Added_Initial_fallback
supreethmr Nov 4, 2019
9c50d3a
Update CONTRIBUTORS.md
jeremyfuksa Nov 4, 2019
05df907
Merge remote-tracking branch 'origin/master' into Added_Initial_fallback
rm012685 Mar 27, 2020
21fdee0
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel Mar 27, 2020
be42bf4
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel Mar 28, 2020
900a3fd
Fix tests
rm012685 Mar 28, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/terra-avatar/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ ChangeLog

Unreleased
----------
### Changed
* Changed the fallback behavior of Avatar. Such that if the image fails to load when initials are provided, the avatar falls back to the initials display instead of default user icon.

2.26.0 - (September 6, 2019)
------------------
Expand Down
2 changes: 1 addition & 1 deletion packages/terra-avatar/docs/avatar.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Terra Avatar

The `Avatar` variant represents a person - it displays an image or initials in a circular frame. If neither are provided, a fallback user icon displays. This is the default export of the `terra-avatar` package.
The `Avatar` variant represents a person - it displays an image or initials in a circular frame. If valid image is not provided and initials is provided, then fallback initial displays. If neither are provided, a fallback user icon displays. This is the default export of the `terra-avatar` package.
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on this description, is this still a breaking change? Initials are not required and if not supplied the component still has the default behaviour?

Copy link
Contributor Author

@supreethmr supreethmr Oct 10, 2019

Choose a reason for hiding this comment

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

Yes but only if initials are supplied fallback will happen to initials instead of user Icon which was not the behaviour before these code changes. So I Think it is a breaking change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see that as breaking at all. The api does not change and consumers would get the behavior we expect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see you are setting initials to be required. This paragraph needs to be updated.

Suggested change
The `Avatar` variant represents a person - it displays an image or initials in a circular frame. If valid image is not provided and initials is provided, then fallback initial displays. If neither are provided, a fallback user icon displays. This is the default export of the `terra-avatar` package.
The `Avatar` variant represents a person - it displays an image or initials in a circular frame. If valid image is not provided then the avatar falls back to displaying initials. This is the default export of the `terra-avatar` package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see that as breaking at all. The api does not change and consumers would get the behavior we expect.

In scenario : user would be expecting avatar to display user-icon when image fails to load but with this code changes it will display initials when image fails to load instead of displaying user-Icon. (this was something which was not happening before) so will these not be considered as breaking change..?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this paragraph just confused me. We're setting initials to required which is breaking so the fallback is simply if no image, then initials.


## Getting Started

Expand Down
16 changes: 10 additions & 6 deletions packages/terra-avatar/src/common/AvatarUtils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,22 @@ const getColorVariant = (hashValue) => {
/**
* Render placeholder.
*/
const generateImagePlaceholder = (alt, isAriaHidden, variant) => {
const avatarIconClassNames = cx(['icon', variant]);
const generateImagePlaceholder = (avatarParams) => {
if (avatarParams.initials && avatarParams.initials.length <= 2) {
const avatarTextClassNames = cx('initials');
return <span className={avatarTextClassNames} alt={avatarParams.alt} aria-label={avatarParams.alt} aria-hidden={avatarParams.isAriaHidden}>{avatarParams.initials.toUpperCase()}</span>;
}

return <span className={avatarIconClassNames} role="img" aria-label={alt} alt={alt} aria-hidden={isAriaHidden} />;
const avatarIconClassNames = cx(['icon', avatarParams.variant]);
return <span className={avatarIconClassNames} role="img" aria-label={avatarParams.alt} alt={avatarParams.alt} aria-hidden={avatarParams.isAriaHidden} />;
};

/**
* Render image with placeholder.
*/
const generateImage = (image, alt, isAriaHidden, variant, handleFallback) => {
const icon = generateImagePlaceholder(alt, isAriaHidden, variant);
return <TerraImage className={cx('image')} src={image} placeholder={icon} alt={alt} onError={handleFallback} fit="cover" />;
const generateImage = (avatarParams) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated here

const icon = generateImagePlaceholder(avatarParams);
return <TerraImage className={cx('image')} src={avatarParams.image} placeholder={icon} alt={avatarParams.alt} onError={avatarParams.handleFallback} fit="cover" />;
};


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import React from 'react';
import Avatar from '../../../../index';

export default () => <Avatar image="invalid-image-url" initials="JD" alt="User" id="invalid-image-avatar" />;
16 changes: 11 additions & 5 deletions packages/terra-avatar/src/variants/Avatar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,19 @@ class Avatar extends React.Component {

let avatarContent;

const avatarParams = {
image,
alt,
isAriaHidden,
variant: AVATAR_VARIANTS.USER,
handleFallback: this.handleFallback,
initials,
};

if (image) {
avatarContent = generateImage(image, alt, isAriaHidden, AVATAR_VARIANTS.USER, this.handleFallback);
} else if (initials && (initials.length === 1 || initials.length === 2)) {
const avatarTextClassNames = cx('initials');
avatarContent = <span className={avatarTextClassNames} alt={alt} aria-label={alt} aria-hidden={isAriaHidden}>{initials.toUpperCase()}</span>;
avatarContent = generateImage(avatarParams);
} else {
avatarContent = generateImagePlaceholder(alt, isAriaHidden, AVATAR_VARIANTS.USER);
avatarContent = generateImagePlaceholder(avatarParams);
}

const attributes = { ...customProps };
Expand Down
12 changes: 10 additions & 2 deletions packages/terra-avatar/src/variants/Facility.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,18 @@ class Facility extends React.Component {

let facilityContent;

const facilityParams = {
image,
alt,
isAriaHidden,
variant: AVATAR_VARIANTS.FACILITY,
handleFallback: this.handleFallback,
};

if (image) {
facilityContent = generateImage(image, alt, isAriaHidden, AVATAR_VARIANTS.FACILITY, this.handleFallback);
facilityContent = generateImage(facilityParams);
} else {
facilityContent = generateImagePlaceholder(alt, isAriaHidden, AVATAR_VARIANTS.FACILITY);
facilityContent = generateImagePlaceholder(facilityParams);
}
const attributes = { ...customProps };
const customStyles = size ? ({ fontSize: size, ...attributes.style }) : attributes.style;
Expand Down
8 changes: 7 additions & 1 deletion packages/terra-avatar/tests/jest/Avatar.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ describe('Avatar', () => {
expect(wrapper).toMatchSnapshot();
});

it('should render fallback user avatar when invalid image is passed in', () => {
it('should render fallback initials when invalid image is passed in with initials', () => {
const avatar = <Avatar image="https://path/to/invalid_image.jpg" initials="JD" alt="placeholder" />;
const wrapper = render(avatar);
expect(wrapper).toMatchSnapshot();
});

it('should render fallback user avatar when invalid image is passed in without initials', () => {
const avatar = <Avatar image="https://path/to/invalid_image.jpg" alt="placeholder" />;
const wrapper = render(avatar);
expect(wrapper).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,35 @@ exports[`Avatar should render an isDeceased avatar 1`] = `
</div>
`;

exports[`Avatar should render fallback user avatar when invalid image is passed in 1`] = `
exports[`Avatar should render fallback initials when invalid image is passed in with initials 1`] = `
<div
class="avatar five image"
>
<div>
<div
class="hidden"
>
<img
alt="placeholder"
class="image cover default image"
src="https://path/to/invalid_image.jpg"
/>
</div>
<div>
<span
alt="placeholder"
aria-hidden="false"
aria-label="placeholder"
class="initials"
>
JD
</span>
</div>
</div>
</div>
`;

exports[`Avatar should render fallback user avatar when invalid image is passed in without initials 1`] = `
<div
class="avatar five image"
>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 8 additions & 2 deletions packages/terra-avatar/tests/wdio/avatar-common-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,14 @@ Terra.describeViewports('Avatar', ['huge'], () => {
Terra.it.matchesScreenshot({ selector: '#image-avatar' });
});

describe('Invalid Image Avatar', () => {
before(() => browser.url('/#/raw/tests/terra-avatar/avatar/avatar/invalid-image-avatar'));
describe('Invalid Image Avatar With Initials', () => {
before(() => browser.url('/#/raw/tests/terra-avatar/avatar/avatar/invalid-image-avatar-with-initials'));

Terra.it.validatesElement({ selector: '#invalid-image-avatar' });
});

describe('Invalid Image Avatar Without Initials', () => {
before(() => browser.url('/#/raw/tests/terra-avatar/avatar/avatar/invalid-image-avatar-without-initials'));

Terra.it.validatesElement({ selector: '#invalid-image-avatar' });
});
Expand Down