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

Conversation

supreethmr
Copy link
Contributor

@supreethmr supreethmr commented Sep 3, 2019

Summary

Resolves #2559

Additional Details

generateImage has been modified to fallback to initials when provided for invalid image.

Deployment Links

https://terra-core-deployed-pr-2621.herokuapp.com/components/terra-avatar/avatar/avatar
https://terra-core-deployed-pr-2621.herokuapp.com/raw/tests/terra-avatar/avatar/avatar/invalid-image-avatar-with-initials

@@ -3,6 +3,8 @@ ChangeLog

Unreleased
----------
### Changed
* Changed generateImage function to Fallback to Initials if Provided.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe saying 'Changed the load behavior such that if the image fails to load and initials are provided, the avatar falls back to the initials display.' is more clear to a consumer.

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

avatarContent = generateImage(image, alt, isAriaHidden, AVATAR_VARIANTS.USER, this.handleFallback);
} else if (initials && (initials.length === 1 || initials.length === 2)) {
avatarContent = generateImage(image, alt, isAriaHidden, AVATAR_VARIANTS.USER, this.handleFallback, initials);
} else if (initials && initials.length <= 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, could these redundant checks be probably avoided if the default value of initials is set to an empty string instead of undefined and if the length of the initials prop is custom-validated during datatype validation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redundant checks have been removed and validating this only at generateImagePlaceholder function : here

@nramamurth
Copy link
Contributor

@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 September 5, 2019 08:50 Inactive
@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 September 9, 2019 04:30 Inactive
@mmalaker
Copy link

mmalaker commented Sep 9, 2019

During functional verification the initials fallback is working fine, but on JAWS on Chrome and on VO in iOS, the screen reader reads initials only and not the name, which should be available to the screen reader. VO on Safari desktop reads the avatar name correctly.

@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 September 10, 2019 11:52 Inactive
@supreethmr
Copy link
Contributor Author

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

@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 September 16, 2019 18:05 Inactive
@mmalaker
Copy link

Functional verification completed. The avatar fallback is working as designed. Confirmed that using the VO commands: control + option + R/L arrow, will enable VoiceOver on desktop Safari to read out the name of the avatar image. Some accessibility issues remain, however. VoiceOver on iOS navigation still skips over the avatar. Also, we need to determine user expectations regarding when the VO commands are needed. Most page elements are accessible by using the arrow keys only. These accessibility issues will be addressed separately.

Co-Authored-By: Matt Henkes <mjhenkes@gmail.com>
@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 October 15, 2019 09:25 Inactive
@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 October 16, 2019 10:03 Inactive
@bjankord bjankord temporarily deployed to terra-core-deployed-pr-2621 October 16, 2019 11:42 Inactive
@ryanthemanuel ryanthemanuel self-assigned this Oct 28, 2019
@jeremyfuksa jeremyfuksa self-requested a review November 4, 2019 21:43
Copy link
Contributor

@jeremyfuksa jeremyfuksa left a comment

Choose a reason for hiding this comment

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

Tested: PR #2621
Resolves: Issue #2559

Reviewed:

Pass
UX Review

@StephenEsser StephenEsser changed the title Changed fallback to initials from user icon for avatar [terra-avatar] Changed fallback to initials from user icon for avatar Jan 24, 2020
rm012685 and others added 4 commits March 27, 2020 10:43
@ryanthemanuel ryanthemanuel merged commit ef3c65e into master Mar 28, 2020
@ryanthemanuel ryanthemanuel deleted the Added_Initial_fallback branch March 28, 2020 23:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[terra-avatar] Allow Fallback to Initials Display