This repository has been archived by the owner on Mar 4, 2020. It is now read-only.
fix(Avatar): fixing issues for the height of the element and the initials generation #38
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import React from 'react' | ||
| import { Avatar } from '@stardust-ui/react' | ||
|
|
||
| const AvatarExampleExcludedInitialsShorthand = () => ( | ||
| <div> | ||
| <Avatar name="John Doe (Software Developer)" status="Available" /> | ||
|   | ||
| <Avatar name="John Doe {Software Developer}" status="Available" /> | ||
|   | ||
| <Avatar name="John Doe [Software Developer]" status="Available" /> | ||
|   | ||
| <Avatar name="John A B Doe" status="Available" /> | ||
| </div> | ||
| ) | ||
|
|
||
| export default AvatarExampleExcludedInitialsShorthand |
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,10 @@ | ||
| import React from 'react' | ||
| import { Avatar } from '@stardust-ui/react' | ||
|
|
||
| const getInitials = name => name.split(' ').map(word => `${word[0]}.`) | ||
|
|
||
| const AvatarExampleGetInitialsShorthand = () => ( | ||
| <Avatar status="Available" name="John Doe" getInitials={getInitials} /> | ||
| ) | ||
|
|
||
| export default AvatarExampleGetInitialsShorthand |
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,13 @@ | ||
| import React from 'react' | ||
| import { Avatar } from '@stardust-ui/react' | ||
|
|
||
| const AvatarExamplePresenceIndicatorCustomizationShorthand = () => ( | ||
| <Avatar | ||
| src="/public/images/avatar/small/matt.jpg" | ||
| alt="Profile picture of Matt" | ||
| status="Available" | ||
| variables={{ presenceIndicatorBackground: '#d3d3d3' }} | ||
| /> | ||
| ) | ||
|
|
||
| export default AvatarExamplePresenceIndicatorCustomizationShorthand |
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 |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| import React from 'react' | ||
| import { Avatar } from '@stardust-ui/react' | ||
|
|
||
| const AvatarExampleSrc = () => <Avatar src="public/images/avatar/small/matt.jpg" /> | ||
|
|
||
| export default AvatarExampleSrc |
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 |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ class Avatar extends UIComponent<any, any> { | |
|
|
||
| static displayName = 'Avatar' | ||
|
|
||
| static handledProps = ['alt', 'as', 'className', 'getInitials', 'name', 'size', 'src', 'status'] | ||
|
|
||
| static rules = avatarRules | ||
|
|
||
|
|
@@ -51,10 +51,34 @@ class Avatar extends UIComponent<any, any> { | |
| 'Offline', | ||
| 'PresenceUnknown', | ||
| ]), | ||
|
|
||
| /** Custom method for generating the initials from the name property, shown in the avatar if there is no image provided. */ | ||
| getInitials: PropTypes.func, | ||
| } | ||
|
|
||
| static defaultProps = { | ||
| size: 5, | ||
| getInitials(name: string) { | ||
| if (!name) { | ||
| return '' | ||
| } | ||
|
|
||
| const reducedName = name | ||
| .replace(/\s*\(.*?\)\s*/g, ' ') | ||
| .replace(/\s*{.*?}\s*/g, ' ') | ||
| .replace(/\s*\[.*?]\s*/g, ' ') | ||
|
|
||
| const initials = reducedName | ||
| .split(' ') | ||
| .filter(item => item !== '') | ||
| .map(name => name.charAt(0)) | ||
| .reduce((accumulator, currentValue) => accumulator + currentValue) | ||
|
|
||
| if (initials.length > 2) { | ||
| return initials.charAt(0) + initials.charAt(initials.length - 1) | ||
| } | ||
| return initials | ||
| }, | ||
| } | ||
|
|
||
| static statusToIcon = { | ||
|
|
@@ -89,55 +113,43 @@ class Avatar extends UIComponent<any, any> { | |
| } | ||
|
|
||
| renderComponent({ ElementType, classes, rest }) { | ||
| const { src, alt, name, status, getInitials, size } = this.props | ||
| const { icon = '', color = '' } = Avatar.statusToIcon[status] || {} | ||
|
There was a problem hiding this comment. just to not forget - there is a problem here that colors are defined based on status, and thus
Both these issues should be addressed by means of dedicated PRs - there is a corresponding issue created to track this work: #52 There was a problem hiding this comment. Thanks for the review! Will try to create issues for the other things that are not part of this PR |
||
|
|
||
| const iconVariables = { | ||
| color: 'white', | ||
| backgroundColor: color, | ||
| } | ||
|
|
||
| return ( | ||
| <ElementType {...rest} className={classes.root}> | ||
| {src ? ( | ||
| <Image className={classes.imageAvatar} avatar src={src} alt={alt} title={name} /> | ||
| ) : ( | ||
| <Label | ||
| className={classes.avatarNameContainer} | ||
| as="div" | ||
| content={getInitials(name)} | ||
| variables={{ padding: '0px' }} | ||
| circular | ||
| title={name} | ||
| /> | ||
| )} | ||
| {status && ( | ||
| <div className={classes.presenceIndicatorWrapper}> | ||
| <Icon | ||
| className={classes.presenceIndicator} | ||
| size={size < 4 ? 'micro' : size < 6 ? 'mini' : 'tiny'} | ||
| circular | ||
| name={icon} | ||
| variables={iconVariables} | ||
| title={status} | ||
| /> | ||
| </div> | ||
| )} | ||
| </ElementType> | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| export default Avatar | ||
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.
a bit worrying about performance implications of this - in general, we could have a linear complexity algorithm to achieve the same goal, while currently, because of regexp, we would use an algorithm of polynomial complexity (cubic at best). Taking the following things into account:
I would rather suggest to address this issue by introducing linear algorithm for handling this aspect and not introduce possibility for bottlenecks that could surprise client of the library.
So, maybe we should create an issue and address it by means of dedicated PR
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.
👍 to separate PR. would be good to see data before making changes.