diff --git a/src/components/ReportActionAvatars/ReportActionAvatar.tsx b/src/components/ReportActionAvatars/ReportActionAvatar.tsx index f24c35f753cb..9e3776670b34 100644 --- a/src/components/ReportActionAvatars/ReportActionAvatar.tsx +++ b/src/components/ReportActionAvatars/ReportActionAvatar.tsx @@ -18,15 +18,14 @@ import useTheme from '@hooks/useTheme'; import useThemeIllustrations from '@hooks/useThemeIllustrations'; import useThemeStyles from '@hooks/useThemeStyles'; import {getCardFeedIcon} from '@libs/CardUtils'; -import localeCompare from '@libs/LocaleCompare'; -import {getUserDetailTooltipText} from '@libs/ReportUtils'; +import {getUserDetailTooltipText, sortIconsByName} from '@libs/ReportUtils'; import type {AvatarSource} from '@libs/UserUtils'; import Navigation from '@navigation/Navigation'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; -import type {CompanyCardFeed, OnyxInputOrEntry, PersonalDetailsList} from '@src/types/onyx'; +import type {CompanyCardFeed} from '@src/types/onyx'; import type {Icon as IconType} from '@src/types/onyx/OnyxCommon'; type SortingOptions = ValueOf; @@ -286,23 +285,6 @@ function ReportActionAvatarSubscript({ ); } -const getIconDisplayName = (icon: IconType, personalDetails: OnyxInputOrEntry) => - icon.id ? (personalDetails?.[icon.id]?.displayName ?? personalDetails?.[icon.id]?.login ?? '') : ''; - -function sortIconsByName(icons: IconType[], personalDetails: OnyxInputOrEntry) { - return icons.sort((first, second) => { - // First sort by displayName/login - const displayNameLoginOrder = localeCompare(getIconDisplayName(first, personalDetails), getIconDisplayName(second, personalDetails)); - if (displayNameLoginOrder !== 0) { - return displayNameLoginOrder; - } - - // Then fallback on accountID as the final sorting criteria. - // This will ensure that the order of avatars with same login/displayName - // stay consistent across all users and devices - return Number(first?.id) - Number(second?.id); - }); -} function ReportActionAvatarMultipleHorizontal({ isHovered = false, @@ -330,6 +312,7 @@ function ReportActionAvatarMultipleHorizontal({ const theme = useTheme(); const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); + const {localeCompare} = useLocalize(); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, { canBeMissing: true, @@ -345,13 +328,13 @@ function ReportActionAvatarMultipleHorizontal({ let avatars: IconType[] = unsortedIcons; if (sortAvatars?.includes(CONST.REPORT_ACTION_AVATARS.SORT_BY.NAME)) { - avatars = sortIconsByName(unsortedIcons, personalDetails); + avatars = sortIconsByName(unsortedIcons, personalDetails, localeCompare); } else if (sortAvatars?.includes(CONST.REPORT_ACTION_AVATARS.SORT_BY.ID)) { avatars = lodashSortBy(unsortedIcons, (icon) => icon.id); } return sortAvatars?.includes(CONST.REPORT_ACTION_AVATARS.SORT_BY.REVERSE) ? avatars.reverse() : avatars; - }, [unsortedIcons, personalDetails, sortAvatars]); + }, [unsortedIcons, personalDetails, sortAvatars, localeCompare]); const avatarRows = useMemo(() => { // If we're not displaying avatars in rows or the number of icons is less than or equal to the max avatars in a row, return a single row diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index b1369a48260e..4e2e1c28abec 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -279,8 +279,6 @@ type SpendBreakdown = { totalDisplaySpend: number; }; -type ParticipantDetails = [number, string, AvatarSource, AvatarSource]; - type OptimisticAddCommentReportAction = Pick< ReportAction, | 'reportActionID' @@ -2739,38 +2737,18 @@ function getDefaultGroupAvatar(reportID?: string): IconAsset { * The Avatar sources can be URLs or Icon components according to the chat type. */ function getIconsForParticipants(participants: number[], personalDetails: OnyxInputOrEntry): Icon[] { - const participantDetails: ParticipantDetails[] = []; const participantsList = participants || []; + const avatars: Icon[] = []; for (const accountID of participantsList) { const avatarSource = personalDetails?.[accountID]?.avatar ?? FallbackAvatar; const displayNameLogin = personalDetails?.[accountID]?.displayName ? personalDetails?.[accountID]?.displayName : personalDetails?.[accountID]?.login; - participantDetails.push([accountID, displayNameLogin ?? '', avatarSource, personalDetails?.[accountID]?.fallbackIcon ?? '']); - } - - const sortedParticipantDetails = participantDetails.sort((first, second) => { - // First sort by displayName/login - const displayNameLoginOrder = localeCompareLibs(first[1], second[1]); - if (displayNameLoginOrder !== 0) { - return displayNameLoginOrder; - } - - // Then fallback on accountID as the final sorting criteria. - // This will ensure that the order of avatars with same login/displayName - // stay consistent across all users and devices - return first[0] - second[0]; - }); - - // Now that things are sorted, gather only the avatars (second element in the array) and return those - const avatars: Icon[] = []; - - for (const sortedParticipantDetail of sortedParticipantDetails) { const userIcon = { - id: sortedParticipantDetail[0], - source: sortedParticipantDetail[2], + id: accountID, + source: avatarSource, type: CONST.ICON_TYPE_AVATAR, - name: sortedParticipantDetail[1], - fallbackIcon: sortedParticipantDetail[3], + name: displayNameLogin ?? '', + fallbackIcon: personalDetails?.[accountID]?.fallbackIcon ?? '', }; avatars.push(userIcon); } @@ -3373,6 +3351,24 @@ function getIcons( return getIconsForParticipants(participantAccountIDs, personalDetails); } +const getIconDisplayName = (icon: Icon, personalDetails: OnyxInputOrEntry) => + icon.id ? (personalDetails?.[icon.id]?.displayName ?? personalDetails?.[icon.id]?.login ?? '') : ''; + +function sortIconsByName(icons: Icon[], personalDetails: OnyxInputOrEntry, localeCompare: LocaleContextProps['localeCompare']) { + return icons.sort((first, second) => { + // First sort by displayName/login + const displayNameLoginOrder = localeCompare(getIconDisplayName(first, personalDetails), getIconDisplayName(second, personalDetails)); + if (displayNameLoginOrder !== 0) { + return displayNameLoginOrder; + } + + // Then fallback on accountID as the final sorting criteria. + // This will ensure that the order of avatars with same login/displayName + // stay consistent across all users and devices + return Number(first?.id) - Number(second?.id); + }); +} + function getDisplayNamesWithTooltips( personalDetailsList: PersonalDetails[] | PersonalDetailsList | OptionData[], shouldUseShortForm: boolean, @@ -11605,6 +11601,7 @@ export { getUpgradeWorkspaceMessage, getDowngradeWorkspaceMessage, getIcons, + sortIconsByName, getIconsForParticipants, getIndicatedMissingPaymentMethod, getLastVisibleMessage, diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 606839bb5e5d..432946572a2f 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -71,6 +71,7 @@ import { shouldReportBeInOptionList, shouldReportShowSubscript, shouldShowFlagComment, + sortIconsByName, sortOutstandingReportsBySelected, temporary_getMoneyRequestOptions, } from '@libs/ReportUtils'; @@ -338,7 +339,7 @@ describe('ReportUtils', () => { '1', ); - expect(title).toBeCalledWith( + expect(title).toHaveBeenCalledWith( expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment testDriveURL: expect.any(String), @@ -367,7 +368,7 @@ describe('ReportUtils', () => { '1', ); - expect(description).toBeCalledWith( + expect(description).toHaveBeenCalledWith( expect.objectContaining({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment testDriveURL: expect.any(String), @@ -377,14 +378,14 @@ describe('ReportUtils', () => { }); describe('getIconsForParticipants', () => { - it('returns sorted avatar source by name, then accountID', () => { + it('returns avatar source', () => { const participants = getIconsForParticipants([1, 2, 3, 4, 5], participantsPersonalDetails); expect(participants).toHaveLength(5); - expect(participants.at(0)?.source).toBeInstanceOf(Function); - expect(participants.at(0)?.name).toBe('(833) 240-3627'); - expect(participants.at(0)?.id).toBe(4); - expect(participants.at(0)?.type).toBe('avatar'); + expect(participants.at(3)?.source).toBeInstanceOf(Function); + expect(participants.at(3)?.name).toBe('(833) 240-3627'); + expect(participants.at(3)?.id).toBe(4); + expect(participants.at(3)?.type).toBe('avatar'); expect(participants.at(1)?.source).toBeInstanceOf(Function); expect(participants.at(1)?.name).toBe('floki@vikings.net'); @@ -393,6 +394,24 @@ describe('ReportUtils', () => { }); }); + describe('sortIconsByName', () => { + it('returns sorted avatar source by name, then accountID', () => { + const participants = getIconsForParticipants([1, 2, 3, 4, 5], participantsPersonalDetails); + const sortedParticipants = sortIconsByName(participants, participantsPersonalDetails, localeCompare); + expect(sortedParticipants).toHaveLength(5); + + expect(sortedParticipants.at(0)?.source).toBeInstanceOf(Function); + expect(sortedParticipants.at(0)?.name).toBe('(833) 240-3627'); + expect(sortedParticipants.at(0)?.id).toBe(4); + expect(sortedParticipants.at(0)?.type).toBe('avatar'); + + expect(sortedParticipants.at(1)?.source).toBeInstanceOf(Function); + expect(sortedParticipants.at(1)?.name).toBe('floki@vikings.net'); + expect(sortedParticipants.at(1)?.id).toBe(2); + expect(sortedParticipants.at(1)?.type).toBe('avatar'); + }); + }); + describe('getWorkspaceIcon', () => { it('should not use cached icon when avatar is updated', () => { // Given a new workspace and a expense chat with undefined `policyAvatar`