Avatar Issue #3627
Avatar Issue #3627
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
@moulichan99 great job.
First off, make sure to complete the PR original post in full. It needs the original issue ID, a brief description, and the checkboxes ticked please. You will also need to include a Changeset as this modifies component code. Instructions are provided above.
Minus these small adjustments detailed below, the only other thing missing is documentation to show users how to implement and utilize this new feature. This will need to be present for both React and Svelte docs.
https://www.skeleton.dev/docs/components/avatar/react
https://www.skeleton.dev/docs/components/avatar/svelte
I'd recommend a new section above "Fallback" like so:
## Initials
Provide an `initials` array to define what part of the name is used to generate initials. Use index values to identify each desired name, and `-1` to represent the last possible name.
(example)Per the example, feel free remove the initials showcase in the Fallback example and description. Then showcase 2-3 avatars with mixed usage in this new example block, including:
- Default value (no prop defined) - moved from Fallback
- All initials for a three word name, ex:
[0, 1, 2] - Two initials from a three word name,
[0, -1]
As per the name, something like "Jane Michelle Doe" will work.
| .split(' ') | ||
| .map((word) => word[0]) | ||
| .join(''); | ||
| const getInitials = (name: string, initials: number[] = [0, -1]) => { |
There was a problem hiding this comment.
There's a few things here:
1.initials should not need to be passed in as params - and technically not name either. Feel free to remove both and inherit those from the parent scope.
2. The default values for initials should be defined on the prop list (line 30)
3. name is a required prop, so we don't need the early return
4. namePart can be exchanged for chaining at/charAt/toUpperCase and drop the ternary conditional
With all modifications it should condense down to something like this:
NOTE: this is untested, so let me know if it gives you any trouble.
const getInitials = () => {
return initials.map(index => (name
.split(' ')
.at(index)
.charAt(0)
.toUpperCase()
.join()
))
}| @@ -27,6 +27,7 @@ export const Avatar: FC<AvatarProps> = ({ | |||
| fallbackClasses = '', | |||
| // Children | |||
| children, | |||
| initials, | |||
There was a problem hiding this comment.
Move this to match the suggested types.ts position and go ahead and set [0, -1] as the default here.
| @@ -64,7 +69,7 @@ export const Avatar: FC<AvatarProps> = ({ | |||
| )} | |||
| {/* Fallback */} | |||
| <span {...api.getFallbackProps()} className={`${fallbackBase} ${fallbackClasses}`} data-testid="avatar-fallback"> | |||
| {children ? children : getInitials(name)} | |||
| {children ? children : getInitials(name, initials)} | |||
There was a problem hiding this comment.
Update: getInitials(...) -> getInitials
| @@ -38,4 +38,5 @@ export interface AvatarProps extends Omit<avatar.Props, 'id'>, React.PropsWithCh | |||
| fallbackBase?: string; | |||
| /** Provide arbitrary CSS classes to fallback element. */ | |||
| fallbackClasses?: string; | |||
| initials?: number[]; | |||
There was a problem hiding this comment.
initials is a "functional prop" per our concerns, so feel free to bump this up under name and give it a JSDocs comment like so:
Initials will be generated based on the name and this index array.
| @@ -25,6 +25,7 @@ | |||
| fallbackClasses = '', | |||
There was a problem hiding this comment.
Same changes as React please!
| @@ -43,4 +43,5 @@ export interface AvatarProps extends Omit<avatar.Props, 'id'>, Pick<HTMLAttribut | |||
| // Snippets --- | |||
There was a problem hiding this comment.
Same changes as React please!
Linked Issue
Closes #{issueNumber}
Description
{description}
Checklist
Please read and apply all contribution requirements.
docs/,feature/,chore/,bugfix/mainbranchpnpm checkin the root of the monorepopnpm formatin the root of the monorepopnpm lintin the root of the monorepopnpm testin the root of the monorepo/packageprojects, please supply a ChangesetChangesets
View our documentation to learn more about Changesets. To create a Changeset:
pnpm changesetand follow the prompts