From 990932d49607b97105ea7873674d57f5032b1e1f Mon Sep 17 00:00:00 2001 From: honnamkuan Date: Thu, 17 Aug 2023 21:37:38 +0800 Subject: [PATCH 01/16] sort group member avatar icons correctly --- src/libs/ReportUtils.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 1baf6eacb9da..a2663c0cf30d 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -874,8 +874,14 @@ function getIconsForParticipants(participants, personalDetails) { ]); } - // Sort all logins by first name (which is the second element in the array) - const sortedParticipantDetails = participantDetails.sort((a, b) => a[2] - b[2]); + // Sort all logins by first name (position 2 element in array) + // if multiple participants have the same first name, sub-sort them by login/displayName (position 1 element in array) + // if that still clashes, sub-sort by accountID (position 0 element in array) + const sortedParticipantDetails = _.chain(participantDetails) + .sortBy((detail) => detail[0]) + .sortBy((detail) => detail[1]) + .sortBy((detail) => detail[2]) + .value(); // Now that things are sorted, gather only the avatars (third element in the array) and return those const avatars = []; From fd1f6595d9032c8d506d7958195548b8472e5842 Mon Sep 17 00:00:00 2001 From: honnamkuan Date: Wed, 23 Aug 2023 23:18:47 +0800 Subject: [PATCH 02/16] sort avatar icons by user displayName/login followed by accountID efficiently add unit tests --- src/libs/ReportUtils.js | 27 ++++++------- tests/unit/ReportUtilsTest.js | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index f64221f74e6b..028d88d4260c 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -884,21 +884,22 @@ 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, - ]); + participantDetails.push([accountID, lodashGet(personalDetails, [accountID, 'displayName']) || lodashGet(personalDetails, [accountID, 'login'], ''), avatarSource]); } - // Sort all logins by first name (position 2 element in array) - // if multiple participants have the same first name, sub-sort them by login/displayName (position 1 element in array) - // if that still clashes, sub-sort by accountID (position 0 element in array) const sortedParticipantDetails = _.chain(participantDetails) - .sortBy((detail) => detail[0]) - .sortBy((detail) => detail[1]) - .sortBy((detail) => detail[2]) + .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 @@ -906,7 +907,7 @@ function getIconsForParticipants(participants, personalDetails) { for (let i = 0; i < sortedParticipantDetails.length; i++) { const userIcon = { id: sortedParticipantDetails[i][0], - source: sortedParticipantDetails[i][3], + source: sortedParticipantDetails[i][2], type: CONST.ICON_TYPE_AVATAR, name: sortedParticipantDetails[i][1], }; diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js index e97e9147c328..015121a203a0 100644 --- a/tests/unit/ReportUtilsTest.js +++ b/tests/unit/ReportUtilsTest.js @@ -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, @@ -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([ @@ -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', + }, ]); }); @@ -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', + }, ]); }); }); From e70c1e9d723ec348bf86dcbd2b99dbd25bb8629b Mon Sep 17 00:00:00 2001 From: honnamkuan Date: Wed, 23 Aug 2023 23:29:10 +0800 Subject: [PATCH 03/16] update comment --- src/libs/ReportUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 028d88d4260c..53a0b69b54f2 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -902,7 +902,7 @@ function getIconsForParticipants(participants, personalDetails) { }) .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 = { From da5635cb42d6982ad82a5fb6b12fb9213b4829d3 Mon Sep 17 00:00:00 2001 From: Akinwale Ariwodola Date: Tue, 19 Sep 2023 07:49:15 +0100 Subject: [PATCH 04/16] consolidate 'I am a Teacher' and 'Intro to School Principal' pages --- src/ROUTES.ts | 2 +- .../AppNavigator/ModalStackNavigators.js | 2 +- src/pages/TeachersUnite/ImTeacherPage.js | 70 +++++++------------ .../TeachersUnite/ImTeacherUpdateEmailPage.js | 54 ++++++++++++++ .../TeachersUnite/IntroSchoolPrincipalPage.js | 1 + src/pages/TeachersUnite/SaveTheWorldPage.js | 23 +----- 6 files changed, 84 insertions(+), 68 deletions(-) create mode 100644 src/pages/TeachersUnite/ImTeacherUpdateEmailPage.js diff --git a/src/ROUTES.ts b/src/ROUTES.ts index 2c37116db395..feead4890114 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -139,8 +139,8 @@ export default { SEARCH: 'search', TEACHERS_UNITE: 'teachersunite', I_KNOW_A_TEACHER: 'teachersunite/i-know-a-teacher', - INTRO_SCHOOL_PRINCIPAL: 'teachersunite/intro-school-principal', I_AM_A_TEACHER: 'teachersunite/i-am-a-teacher', + INTRO_SCHOOL_PRINCIPAL: 'teachersunite/intro-school-principal', DETAILS: 'details', getDetailsRoute: (login: string) => `details?login=${encodeURIComponent(login)}`, PROFILE: 'a/:accountID', diff --git a/src/libs/Navigation/AppNavigator/ModalStackNavigators.js b/src/libs/Navigation/AppNavigator/ModalStackNavigators.js index 392781a777db..16d2e0733224 100644 --- a/src/libs/Navigation/AppNavigator/ModalStackNavigators.js +++ b/src/libs/Navigation/AppNavigator/ModalStackNavigators.js @@ -345,7 +345,7 @@ const NewTeachersUniteNavigator = createModalStackNavigator([ }, { getComponent: () => { - const IntroSchoolPrincipalPage = require('../../../pages/TeachersUnite/IntroSchoolPrincipalPage').default; + const IntroSchoolPrincipalPage = require('../../../pages/TeachersUnite/ImTeacherPage').default; return IntroSchoolPrincipalPage; }, name: 'Intro_School_Principal', diff --git a/src/pages/TeachersUnite/ImTeacherPage.js b/src/pages/TeachersUnite/ImTeacherPage.js index dbeba700d208..a938abf81b79 100644 --- a/src/pages/TeachersUnite/ImTeacherPage.js +++ b/src/pages/TeachersUnite/ImTeacherPage.js @@ -1,54 +1,36 @@ import React from 'react'; -import ScreenWrapper from '../../components/ScreenWrapper'; -import HeaderWithBackButton from '../../components/HeaderWithBackButton'; -import ROUTES from '../../ROUTES'; -import Navigation from '../../libs/Navigation/Navigation'; -import FixedFooter from '../../components/FixedFooter'; -import styles from '../../styles/styles'; -import Button from '../../components/Button'; -import * as Illustrations from '../../components/Icon/Illustrations'; -import variables from '../../styles/variables'; -import useLocalize from '../../hooks/useLocalize'; -import BlockingView from '../../components/BlockingViews/BlockingView'; +import PropTypes from 'prop-types'; +import {withOnyx} from 'react-native-onyx'; +import ONYXKEYS from '../../ONYXKEYS'; +import * as LoginUtils from '../../libs/LoginUtils'; +import ImTeacherUpdateEmailPage from './ImTeacherUpdateEmailPage'; +import IntroSchoolPrincipalPage from './IntroSchoolPrincipalPage'; -const propTypes = {}; +const propTypes = { + /** Current user session */ + session: PropTypes.shape({ + /** Current user primary login */ + email: PropTypes.string.isRequired, + }), +}; -const defaultProps = {}; +const defaultProps = { + session: { + email: null, + }, +}; -function ImTeacherPage() { - const {translate} = useLocalize(); - - return ( - - Navigation.goBack(ROUTES.TEACHERS_UNITE)} - /> - Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS)} - iconWidth={variables.signInLogoWidthLargeScreen} - iconHeight={variables.lhnLogoWidth} - /> - -