-
Notifications
You must be signed in to change notification settings - Fork 4k
ReportActionAvatars -> UserAvatar for single avatars migration #94906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f87c022
540e4b0
41e29ac
9f6f6a9
84fc1a8
30a84c8
fbb170e
735390f
f8904b2
5342cf3
dc3daaa
92d2816
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import SingleAvatar from '@components/Avatar/layouts/SingleAvatar'; | ||
| import {usePersonalDetails} from '@components/OnyxListItemProvider'; | ||
|
|
||
| import useDefaultAvatars from '@hooks/useDefaultAvatars'; | ||
| import useStyleUtils from '@hooks/useStyleUtils'; | ||
|
|
||
| import {buildUserIcon} from '@libs/UserAvatarUtils'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
|
|
||
| import type {StyleProp, ViewStyle} from 'react-native'; | ||
| import type {ValueOf} from 'type-fest'; | ||
|
|
||
| import React from 'react'; | ||
|
|
||
| type AccountAvatarProps = { | ||
| /** Account ID of the user to display the avatar for */ | ||
| accountID: number; | ||
|
|
||
| /** Size of the avatar */ | ||
| size?: ValueOf<typeof CONST.AVATAR_SIZE>; | ||
|
|
||
| /** Whether to show the tooltip on hover */ | ||
| shouldShowTooltip?: boolean; | ||
|
Contributor
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. I think it's been an oversight of our efforts so far that NAB for this PR, but I think it's something we should address separately
Contributor
Author
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. Agreed, but I'd rather do it in separate PR -> Draft stacked on this one
Contributor
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. @jmusial Use GH stack PRs system they recently introduced.
Contributor
Author
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. would love to but AFAIK they don't work from forks :( |
||
|
|
||
| /** Display name used as a fallback for the avatar tooltip */ | ||
| fallbackDisplayName?: string; | ||
|
|
||
| /** Container styles for the avatar. Replaces the size-derived default container styles when provided */ | ||
| containerStyle?: StyleProp<ViewStyle>; | ||
| }; | ||
|
|
||
| /** | ||
| * Renders a single account's avatar, resolving the icon from the personal-details context (zero Onyx subscriptions). | ||
| * Use whenever exactly one account is rendered. Pass `Avatar/UserAvatar` a `source` instead when the avatar is already resolved. | ||
| */ | ||
| function AccountAvatar({accountID, size = CONST.AVATAR_SIZE.DEFAULT, shouldShowTooltip = true, fallbackDisplayName, containerStyle}: AccountAvatarProps) { | ||
| const personalDetails = usePersonalDetails(); | ||
|
Contributor
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. @jmusial Do we need to subscribe to entire personal details object for a single one? Maybe use
Contributor
Author
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. @shubham1206agra |
||
| const defaultAvatars = useDefaultAvatars(); | ||
| const StyleUtils = useStyleUtils(); | ||
|
|
||
| const icon = buildUserIcon({accountID, personalDetails, defaultAvatars}); | ||
|
|
||
| return ( | ||
| <SingleAvatar | ||
| avatar={icon} | ||
| size={size} | ||
| containerStyles={containerStyle ?? StyleUtils.getContainerStyles(size)} | ||
| shouldShowTooltip={shouldShowTooltip} | ||
| fallbackDisplayName={fallbackDisplayName} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| export default AccountAvatar; | ||
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.
Just clarifying, I think the intention is that
src/Avatar/connectedcontains the level-two wrappers around the level-one primitives for common use-cases. And they're "connected" because they actually hook up data, rather than just displaying what they're given via props?Uh oh!
There was an error while loading. Please reload this page.
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.
yup, that was my reasoning behind the name. Open to other suggestions if you feel like naming should be highlighting leveled structure more