Skip to content
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

Sort group member avatar icons correctly to avoid avatar rearranging upon receiving backend response #25416

Merged
merged 8 commits into from
Sep 22, 2023
28 changes: 18 additions & 10 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -943,23 +943,31 @@ function getIconsForParticipants(participants, personalDetails) {
for (let i = 0; i < participantsList.length; i++) {
const accountID = participantsList[i];
const avatarSource = UserUtils.getAvatar(lodashGet(personalDetails, [accountID, 'avatar'], ''), accountID);
participantDetails.push([
accountID,
lodashGet(personalDetails, [accountID, 'displayName']) || lodashGet(personalDetails, [accountID, 'login'], ''),
lodashGet(personalDetails, [accountID, 'firstName'], ''),
avatarSource,
]);
const displayNameLogin = lodashGet(personalDetails, [accountID, 'displayName']) || lodashGet(personalDetails, [accountID, 'login'], '');
participantDetails.push([accountID, displayNameLogin, avatarSource]);
}

// Sort all logins by first name (which is the second element in the array)
const sortedParticipantDetails = participantDetails.sort((a, b) => a[2] - b[2]);
const sortedParticipantDetails = _.chain(participantDetails)
.sort((first, second) => {
// First sort by displayName/login
const displayNameLoginOrder = first[1].localeCompare(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];
})
.value();

// Now that things are sorted, gather only the avatars (third element in the array) and return those
// Now that things are sorted, gather only the avatars (second element in the array) and return those
const avatars = [];
for (let i = 0; i < sortedParticipantDetails.length; i++) {
const userIcon = {
id: sortedParticipantDetails[i][0],
source: sortedParticipantDetails[i][3],
source: sortedParticipantDetails[i][2],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated from index 3 to 2 for avatar source, because the original index 2 element (firstName) is deemed unnecessary and removed.

type: CONST.ICON_TYPE_AVATAR,
name: sortedParticipantDetails[i][1],
};
Expand Down
72 changes: 72 additions & 0 deletions tests/unit/ReportUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ const participantsPersonalDetails = {
login: '+18332403627@expensify.sms',
displayName: '(833) 240-3627',
},
5: {
accountID: 5,
displayName: 'Lagertha Lothbrok',
firstName: 'Lagertha',
login: 'lagertha2@vikings.net',
pronouns: 'She/her',
},
};
const policy = {
policyID: 1,
Expand All @@ -55,6 +62,53 @@ describe('ReportUtils', () => {
});
beforeEach(() => Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.DEFAULT).then(waitForPromisesToResolve));

describe('getIconsForParticipants', () => {
it('returns sorted avatar source by name, then accountID', () => {
expect(ReportUtils.getIconsForParticipants([1, 2, 3, 4, 5], participantsPersonalDetails)).toEqual([
{
id: 4,
name: '(833) 240-3627',
source: {
testUri: '../../../assets/images/avatars/user/default-avatar_5.svg',
},
type: 'avatar',
},
{
id: 2,
name: 'floki@vikings.net',
source: {
testUri: '../../../assets/images/avatars/user/default-avatar_3.svg',
},
type: 'avatar',
},
{
id: 3,
name: 'Lagertha Lothbrok',
source: {
testUri: '../../../assets/images/avatars/user/default-avatar_4.svg',
},
type: 'avatar',
},
{
id: 5,
name: 'Lagertha Lothbrok',
source: {
testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
},
type: 'avatar',
},
{
id: 1,
name: 'Ragnar Lothbrok',
source: {
testUri: '../../../assets/images/avatars/user/default-avatar_2.svg',
},
type: 'avatar',
},
]);
});
});

describe('getDisplayNamesWithTooltips', () => {
test('withSingleParticipantReport', () => {
expect(ReportUtils.getDisplayNamesWithTooltips(participantsPersonalDetails, false)).toStrictEqual([
Expand Down Expand Up @@ -94,6 +148,15 @@ describe('ReportUtils', () => {
accountID: 4,
pronouns: undefined,
},
{
displayName: 'Lagertha Lothbrok',
avatar: {
testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
},
login: 'lagertha2@vikings.net',
accountID: 5,
pronouns: 'She/her',
},
]);
});

Expand Down Expand Up @@ -135,6 +198,15 @@ describe('ReportUtils', () => {
accountID: 4,
pronouns: undefined,
},
{
displayName: 'Lagertha',
avatar: {
testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
},
login: 'lagertha2@vikings.net',
accountID: 5,
pronouns: 'She/her',
},
]);
});
});
Expand Down