-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[NoQA] Add AvatarSelector component #72214
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
168b52d
e0b3c9d
4b9016a
41f0025
200a573
cadd490
ae8206b
f8c629d
de14fe4
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 |
|---|---|---|
|
|
@@ -139,4 +139,5 @@ function Avatar({ | |
|
|
||
| Avatar.displayName = 'Avatar'; | ||
|
|
||
| export type {AvatarProps}; | ||
| export default Avatar; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import React, {useState} from 'react'; | ||
| import {View} from 'react-native'; | ||
| import useStyleUtils from '@hooks/useStyleUtils'; | ||
| import useTheme from '@hooks/useTheme'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import type {ALL_CUSTOM_AVATARS} from '@libs/Avatars/CustomAvatarCatalog'; | ||
| import {CUSTOM_AVATAR_CATALOG} from '@libs/Avatars/CustomAvatarCatalog'; | ||
| import type {AvatarSizeName} from '@styles/utils'; | ||
| import CONST from '@src/CONST'; | ||
| import Avatar from './Avatar'; | ||
| import {PressableWithFeedback} from './Pressable'; | ||
| import Text from './Text'; | ||
|
|
||
| type AvatarSelectorProps = { | ||
| /** Currently selected avatar ID */ | ||
| selectedID?: keyof typeof ALL_CUSTOM_AVATARS; | ||
|
|
||
| /** Called when an avatar is selected */ | ||
| onSelect: (id: keyof typeof ALL_CUSTOM_AVATARS) => void; | ||
|
|
||
| /** Optional: size of avatars in grid */ | ||
| size?: AvatarSizeName; | ||
|
|
||
| /** Optional label to display above the grid */ | ||
| label?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * AvatarSelector — renders a grid of selectable avatars. | ||
| * Note: This component should be placed inside a ScrollView. | ||
| */ | ||
| function AvatarSelector({selectedID, onSelect, label, size = CONST.AVATAR_SIZE.MEDIUM}: AvatarSelectorProps) { | ||
| const theme = useTheme(); | ||
| const styles = useThemeStyles(); | ||
| const StyleUtils = useStyleUtils(); | ||
| const [selected, setSelected] = useState(selectedID); | ||
|
|
||
| const handleSelect = (id: keyof typeof ALL_CUSTOM_AVATARS) => { | ||
| setSelected(id); | ||
| onSelect(id); | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| {!!label && ( | ||
| <View style={[styles.pt5, styles.ph2]}> | ||
| <Text style={StyleUtils.combineStyles([styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre])}>{label}</Text> | ||
|
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. ❌ PERF-4
Suggested fix: const labelStyle = useMemo(
() => StyleUtils.combineStyles([styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre]),
[StyleUtils, styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre]
);
// Then use it:
<Text style={labelStyle}>{label}</Text>Or simply use array syntax directly without <Text style={[styles.sidebarLinkText, styles.optionAlternateText, styles.textLabelSupporting, styles.pre]}>{label}</Text> |
||
| </View> | ||
| )} | ||
| <View style={styles.avatarSelectorListContainer}> | ||
| {CUSTOM_AVATAR_CATALOG.map(({id, local}) => { | ||
| const isSelected = selected === id; | ||
|
|
||
| return ( | ||
| <PressableWithFeedback | ||
| key={id} | ||
| accessible | ||
| accessibilityRole="button" | ||
| accessibilityLabel="Select Avatar" | ||
| onPress={() => handleSelect(id)} | ||
| style={[styles.avatarSelectorWrapper, isSelected && {borderColor: theme.success, borderWidth: 2}]} | ||
| > | ||
| <Avatar | ||
| type={CONST.ICON_TYPE_AVATAR} | ||
| source={local} | ||
| size={size} | ||
| containerStyles={styles.avatarSelectorContainer} | ||
| testID={`AvatarSelector_${id}`} | ||
| /> | ||
| </PressableWithFeedback> | ||
| ); | ||
| })} | ||
| </View> | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| AvatarSelector.displayName = 'AvatarSelector'; | ||
|
|
||
| export type {AvatarSelectorProps}; | ||
| export default AvatarSelector; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,13 +116,78 @@ const SEASON_F1: Record<SeasonF1AvatarIDs, AvatarEntry> = { | |
| 'wrenches-pink600': {local: SeasonF1.WrenchesPink600, url: `${CDN_SEASON_F1}/wrenches-pink600.png`}, | ||
| }; | ||
|
|
||
| const DISPLAY_ORDER = [ | ||
|
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. https://github.com/Expensify/App/pull/72331/files I made some slight improvements in this PR- want to integrate those?
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. updated |
||
| 'car-blue100', | ||
| 'default-avatar_1', | ||
| 'helmet-blue400', | ||
| 'default-avatar_13', | ||
| 'default-avatar_7', | ||
| 'podium-blue400', | ||
| 'flag-blue600', | ||
| 'default-avatar_19', | ||
| 'car-green100', | ||
| 'default-avatar_2', | ||
| 'helmet-green400', | ||
| 'default-avatar_14', | ||
| 'default-avatar_8', | ||
| 'tire-green400', | ||
| 'champagne-green400', | ||
| 'default-avatar_20', | ||
| 'car-yellow100', | ||
| 'default-avatar_3', | ||
| 'helmet-yellow400', | ||
| 'default-avatar_15', | ||
| 'default-avatar_9', | ||
| 'medal-yellow400', | ||
| 'trophy-yellow600', | ||
| 'default-avatar_21', | ||
| 'car-tangerine100', | ||
| 'default-avatar_4', | ||
| 'helmet-tangerine400', | ||
| 'default-avatar_16', | ||
| 'default-avatar_10', | ||
| 'gasoline-tangerine400', | ||
| 'cone-tangerine700', | ||
| 'default-avatar_22', | ||
| 'car-pink100', | ||
| 'default-avatar_5', | ||
| 'helmet-pink400', | ||
| 'default-avatar_17', | ||
| 'default-avatar_11', | ||
| 'steeringwheel-pink400', | ||
| 'wrenches-pink600', | ||
| 'default-avatar_23', | ||
| 'car-ice100', | ||
| 'default-avatar_6', | ||
| 'helmet-ice400', | ||
| 'default-avatar_18', | ||
| 'default-avatar_12', | ||
| 'speedometer-ice400', | ||
| 'stopwatch-ice600', | ||
| 'default-avatar_24', | ||
| ] as const satisfies readonly CustomAvatarID[]; | ||
|
|
||
| const ALL_CUSTOM_AVATARS: Record<CustomAvatarID, AvatarEntry> = { | ||
| ...DEFAULTS, | ||
| ...SEASON_F1, | ||
| }; | ||
|
|
||
| const buildOrderedAvatars = (): Array<{id: CustomAvatarID} & AvatarEntry> => { | ||
| const allIDS = Object.keys(ALL_CUSTOM_AVATARS) as CustomAvatarID[]; | ||
| const explicit = DISPLAY_ORDER.filter((id) => id in ALL_CUSTOM_AVATARS); | ||
| const explicitSet = new Set(explicit); | ||
| const leftovers = allIDS.filter((id) => !explicitSet.has(id)).sort(); | ||
| const finalIDOrder = [...explicit, ...leftovers]; | ||
| return finalIDOrder.map((id) => ({ | ||
| id, | ||
| ...ALL_CUSTOM_AVATARS[id], | ||
| })); | ||
| }; | ||
|
|
||
| const CUSTOM_AVATAR_CATALOG = buildOrderedAvatars(); | ||
|
|
||
| const getAvatarLocal = (id: CustomAvatarID) => ALL_CUSTOM_AVATARS[id].local; | ||
| const getAvatarURL = (id: CustomAvatarID) => ALL_CUSTOM_AVATARS[id].url; | ||
|
|
||
| export {ALL_CUSTOM_AVATARS, getAvatarLocal, getAvatarURL}; | ||
| export {ALL_CUSTOM_AVATARS, CUSTOM_AVATAR_CATALOG, getAvatarLocal, getAvatarURL}; | ||
| export type {DefaultAvatarIDs, SeasonF1AvatarIDs, CustomAvatarID}; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| /* eslint-disable react/jsx-props-no-spreading */ | ||
| import type {Meta, StoryFn} from '@storybook/react'; | ||
| import React from 'react'; | ||
| import {View} from 'react-native'; | ||
| import type {AvatarProps} from '@components/Avatar'; | ||
| import Avatar from '@components/Avatar'; | ||
| import * as Expensicons from '@components/Icon/Expensicons'; | ||
| import {ALL_CUSTOM_AVATARS} from '@libs/Avatars/CustomAvatarCatalog'; | ||
| import CONST from '@src/CONST'; | ||
|
|
||
| const AVATAR_URL = ALL_CUSTOM_AVATARS['car-blue100'].url; | ||
|
|
||
| type AvatarStory = StoryFn<typeof Avatar>; | ||
|
|
||
| const story: Meta<typeof Avatar> = { | ||
| title: 'Components/Avatar', | ||
| component: Avatar, | ||
| }; | ||
|
|
||
| function Template(props: AvatarProps) { | ||
| return ( | ||
| <View style={{flexDirection: 'row', padding: 10}}> | ||
| <Avatar {...props} /> | ||
| </View> | ||
| ); | ||
| } | ||
|
|
||
| const Default: AvatarStory = Template.bind({}); | ||
| Default.args = { | ||
| type: CONST.ICON_TYPE_AVATAR, | ||
| source: AVATAR_URL, | ||
| name: 'John Doe', | ||
| size: CONST.AVATAR_SIZE.DEFAULT, | ||
| }; | ||
|
|
||
| const WorkspaceAvatar: AvatarStory = Template.bind({}); | ||
| WorkspaceAvatar.args = { | ||
| type: CONST.ICON_TYPE_WORKSPACE, | ||
| name: 'Cathy’s Croissants', | ||
| avatarID: 'policy_123', | ||
| size: CONST.AVATAR_SIZE.LARGE, | ||
| }; | ||
|
|
||
| const FallbackAvatar: AvatarStory = Template.bind({}); | ||
| FallbackAvatar.args = { | ||
| type: CONST.ICON_TYPE_AVATAR, | ||
| fallbackIcon: Expensicons.FallbackAvatar, | ||
| name: 'Offline User', | ||
| size: CONST.AVATAR_SIZE.DEFAULT, | ||
| }; | ||
|
|
||
| const SmallAvatar: AvatarStory = Template.bind({}); | ||
| SmallAvatar.args = { | ||
| type: CONST.ICON_TYPE_AVATAR, | ||
| source: AVATAR_URL, | ||
| name: 'Jane', | ||
| size: CONST.AVATAR_SIZE.SMALL, | ||
| }; | ||
|
|
||
| export default story; | ||
| export {Default, WorkspaceAvatar, FallbackAvatar, SmallAvatar}; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /* eslint-disable react/jsx-props-no-spreading */ | ||
| import type {Meta, StoryFn} from '@storybook/react'; | ||
| import React, {useState} from 'react'; | ||
| import type {AvatarSelectorProps} from '@components/AvatarSelector'; | ||
| import AvatarSelector from '@components/AvatarSelector'; | ||
| import CONST from '@src/CONST'; | ||
|
|
||
| /** | ||
| * We use the Component Story Format for writing stories. Follow the docs here: | ||
| * | ||
| * https://storybook.js.org/docs/react/writing-stories/introduction#component-story-format | ||
| */ | ||
|
|
||
| type AvatarSelectorStory = StoryFn<typeof AvatarSelector>; | ||
|
|
||
| const story: Meta<typeof AvatarSelector> = { | ||
| title: 'Components/AvatarSelector', | ||
| component: AvatarSelector, | ||
| }; | ||
|
|
||
| function Template(props: AvatarSelectorProps) { | ||
| const [selected, setSelected] = useState(props.selectedID); | ||
|
|
||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| return ( | ||
| <AvatarSelector | ||
| {...props} | ||
| selectedID={selected} | ||
| onSelect={setSelected} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| const Default: AvatarSelectorStory = Template.bind({}); | ||
| Default.args = { | ||
| selectedID: undefined, | ||
| label: 'Or choose an avatar', | ||
| }; | ||
|
|
||
| const WithPreselectedAvatar: AvatarSelectorStory = Template.bind({}); | ||
| WithPreselectedAvatar.args = { | ||
| selectedID: 'default-avatar_3', | ||
| label: 'With preselected avatar', | ||
| }; | ||
|
|
||
| const LargeAvatars: AvatarSelectorStory = Template.bind({}); | ||
| LargeAvatars.args = { | ||
| selectedID: 'helmet-blue400', | ||
| size: CONST.AVATAR_SIZE.LARGE, | ||
| label: 'Large avatars', | ||
| }; | ||
|
|
||
| const SmallAvatars: AvatarSelectorStory = Template.bind({}); | ||
| SmallAvatars.args = { | ||
| size: CONST.AVATAR_SIZE.SMALL, | ||
| label: 'Small avatars', | ||
| }; | ||
|
|
||
| export default story; | ||
| export {Default, WithPreselectedAvatar, LargeAvatars, SmallAvatars}; |
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.
❌ PERF-4
Object.entries(ALL_CUSTOM_AVATARS)creates a new array instance on every render. Since this array is used in the render loop below, React cannot optimize re-renders effectively as the reference changes every time.Suggested fix:
Since
ALL_CUSTOM_AVATARSis a constant catalog, the dependency array can be empty.