This repository has been archived by the owner on May 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 166
[terra-avatar] Changed fallback to initials from user icon for avatar #2621
Merged
Merged
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
9b6a5d4
Changed fallback to initials for avatar
supreethmr 9382491
Merge branch 'master' into Added_Initial_fallback
supreethmr 3e9cb4c
Merge branch 'master' into Added_Initial_fallback
supreethmr 565f141
changes of Code review comment
supreethmr 706b591
Made Initials as Required Prop
supreethmr 84c6038
Merge branch 'master' into Added_Initial_fallback
supreethmr 32352b5
Merge branch 'master' into Added_Initial_fallback
supreethmr 9a25680
added role to initials so that screen reader reads the both intials a…
supreethmr 197e280
Merge branch 'master' into Added_Initial_fallback
supreethmr 919d7f4
Merge branch 'master' into Added_Initial_fallback
supreethmr 4698f86
screenshot update
supreethmr 64b7c72
Update packages/terra-avatar/docs/avatar.md
supreethmr e7b8744
removed obselete snapshots
supreethmr ce1f8fe
Removing Tests which were not required with new changes
supreethmr d8098a5
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel 87fde47
Merge branch 'master' into Added_Initial_fallback
supreethmr 9c50d3a
Update CONTRIBUTORS.md
jeremyfuksa 05df907
Merge remote-tracking branch 'origin/master' into Added_Initial_fallback
rm012685 21fdee0
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel be42bf4
Merge branch 'master' into Added_Initial_fallback
ryanthemanuel 900a3fd
Fix tests
rm012685 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,17 +60,33 @@ const getColorVariant = (hashValue) => { | |
/** | ||
* Render placeholder. | ||
*/ | ||
const generateImagePlaceholder = (alt, isAriaHidden, variant) => { | ||
const generateImagePlaceholder = (avatarParams) => { | ||
const { | ||
alt, variant, isAriaHidden, | ||
} = avatarParams; | ||
const avatarIconClassNames = cx(['icon', variant]); | ||
yuderekyu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return <span className={avatarIconClassNames} role="img" aria-label={alt} alt={alt} aria-hidden={isAriaHidden} />; | ||
}; | ||
|
||
/** | ||
* Render placeholder. | ||
*/ | ||
const generateInitials = (avatarParams) => { | ||
const { | ||
alt, initials, isAriaHidden, | ||
} = avatarParams; | ||
const avatarTextClassNames = cx('initials'); | ||
return <span className={avatarTextClassNames} alt={alt} aria-label={alt} aria-hidden={isAriaHidden}>{initials.toUpperCase()}</span>; | ||
}; | ||
|
||
/** | ||
* Render image with placeholder. | ||
*/ | ||
const generateImage = (image, alt, isAriaHidden, variant, handleFallback) => { | ||
const icon = generateImagePlaceholder(alt, isAriaHidden, variant); | ||
const generateImage = (avatarParams) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated here |
||
const { | ||
alt, image, variant, handleFallback, | ||
} = avatarParams; | ||
const icon = (variant === AVATAR_VARIANTS.USER) ? generateInitials(avatarParams) : generateImagePlaceholder(avatarParams); | ||
return <TerraImage className={cx('image')} src={image} placeholder={icon} alt={alt} onError={handleFallback} fit="cover" />; | ||
}; | ||
|
||
|
@@ -111,6 +127,7 @@ export { | |
getColorVariant, | ||
generateImagePlaceholder, | ||
generateImage, | ||
generateInitials, | ||
validateColor, | ||
setColor, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 0 additions & 4 deletions
4
packages/terra-avatar/src/terra-dev-site/test/avatar/avatar/InvalidImageAvatar.test.jsx
This file was deleted.
Oops, something went wrong.
4 changes: 4 additions & 0 deletions
4
...erra-avatar/src/terra-dev-site/test/avatar/avatar/InvalidImageAvatarWithInitials.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" />; |
2 changes: 1 addition & 1 deletion
2
packages/terra-avatar/src/terra-dev-site/test/avatar/avatar/IsDeceasedAvatar.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import React from 'react'; | ||
import Avatar from '../../../../index'; | ||
|
||
export default () => <Avatar id="is-deceased-avatar" alt="user" isDeceased />; | ||
export default () => <Avatar id="is-deceased-avatar" alt="user" isDeceased initials="JS" />; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
packages/terra-avatar/src/terra-dev-site/test/avatar/avatar/TwoInitialsAvatar.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import React from 'react'; | ||
import Avatar from '../../../../index'; | ||
|
||
export default () => <Avatar id="two-initials-avatar" initials="JS" alt="user" color="two" />; | ||
export default () => <Avatar id="two-initials-avatar" initials="JSS" color="two" />; |
2 changes: 1 addition & 1 deletion
2
packages/terra-avatar/src/terra-dev-site/test/avatar/avatar/UserAvatar.test.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import React from 'react'; | ||
import Avatar from '../../../../index'; | ||
|
||
export default () => <Avatar id="user-avatar" alt="user" color="one" />; | ||
export default () => <Avatar id="user-avatar" alt="user" color="one" initials="JS" />; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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..?
There was a problem hiding this comment.
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.