fix: use the same sort order for header display names and avatars in a group chat view #28218

akinwale · 2023-09-26T05:19:19Z

Details

Sorts the display names being displayed in the header view of a group chat using the same logic used to sort the avatars displayed in the chat view.

Fixed Issues

$ #28048
PROPOSAL: #28048 (comment)

Tests

Launch Expensify
Create a group chat or open an existing group chat with at least 3 members
Verify that the display names in the header view are in the same order as the avatars in the chat view.
Verify that the display names in the LHN are in the same order as the avatars in the chat view.

Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Same as tests.

Verify that no errors appear in the JS console

PR Author Checklist

I linked the correct issue in the ### Fixed Issues section above
I wrote clear testing steps that cover the changes made in this PR
- I added steps for local testing in the Tests section
- I added steps for the expected offline behavior in the Offline steps section
- I added steps for Staging and/or Production testing in the QA steps section
- I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
- I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
I included screenshots or videos for tests on all platforms
I ran the tests on all platforms & verified they passed on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
I followed proper code patterns (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
  - If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I followed the guidelines as stated in the Review Guidelines
I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
I verified that if a function's arguments changed that all usages have also been updated correctly
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

28048-web.mp4

Mobile Web - Chrome

Mobile Web - Safari

Desktop

28048-desktop.mp4

iOS

Android

melvin-bot · 2023-09-26T05:19:25Z

@allroundexperts Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

akinwale · 2023-09-28T04:40:24Z

@allroundexperts Bump on review, please.

allroundexperts · 2023-09-28T23:30:38Z

@akinwale Can you please resolve conflicts?

akinwale · 2023-09-29T02:25:18Z

@akinwale Can you please resolve conflicts?

@allroundexperts Done.

akinwale · 2023-10-02T05:11:17Z

@allroundexperts Bump on review, please. The PR has been up for almost a week now.

allroundexperts · 2023-10-02T05:12:34Z

Sorry, I'm on it. This has quite some file changes so I need time to test it completely.

allroundexperts · 2023-10-02T07:09:39Z

src/components/LHNOptionsList/OptionRowLHN.js

+    const sortedDisplayNames =
+        optionItem.type === CONST.REPORT.TYPE.CHAT && !optionItem.isArchivedRoom && optionItem.displayNamesWithTooltips && optionItem.displayNamesWithTooltips.length > 1
+            ? _.filter(
+                  _.map(optionItem.displayNamesWithTooltips, ({displayName}) => displayName),
+                  (displayName) => !_.isEmpty(displayName),
+              ).join(', ')
+            : '';


@akinwale Can you please explain why this change is needed?

Specifically, I'm curious about this line:

optionItem.type === CONST.REPORT.TYPE.CHAT && !optionItem.isArchivedRoom && optionItem.displayNamesWithTooltips && optionItem.displayNamesWithTooltips.length > 1

Also, this name sortedDisplayNames seems to be confusing. optionItem.displayNamesWithTooltips are already sorted and you're just joining the array here. I think something like below makes more sense.

const fullTitle = condition ? join the display names : optionItem.text;

Doing this will also prevent the need for duplicate conditional when specifying fullTittle prop.

In the LHN for small screen devices, for group chats, the names are not sorted by default in ascending order. The component used to display these names is DisplayNamesWithoutTooltips which makes use of the value specified for the fullTitle prop.

The conditions checked are for the following reasons:

optionItem.type === CONST.REPORT.TYPE.CHAT to ensure that it is actually a group chat, not an IOU or task which uses a different string value for the title. For instance, IOU titles look like x owes y $5.00, which is different from a list of names in a group chat.

!optionItem.isArchivedRoom to make sure that the room is not archived. Archived rooms use a different title format (eg. Report (archived)).

The remaining two conditions are to check to make sure that there is more than 1 participant in the report, which most likely indicates that it is a group chat.

The _.filter was added to make sure that there are no empty string display names. I added this due to one of the tests failing.

To summarise, if the report is a group chat, and not an IOU or task, and not archived, and there is more than one participant, then the list of sorted display names will be displayed as the title for devices where the DisplayNameWithoutTooltip component is rendered (usually mobile / touchscreen or small screen devices).

Also, this name sortedDisplayNames seems to be confusing. optionItem.displayNamesWithTooltips are already sorted and you're just joining the array here. I think something like below makes more sense.

const fullTitle = condition ? join the display names : optionItem.text;

Doing this will also prevent the need for duplicate conditional when specifying fullTittle prop.

This won't work. displayNamesWithTooltips is a list of objects, which is the reason for the map. Also, one of the tests was failing with empty display names, which is why I added the filter.

I mean't something else. Please read it again 😄

Oh, right, I see it now. Assigning to a fullTitle value instead of using sortedDisplayNames.

allroundexperts · 2023-10-02T07:20:03Z

src/libs/ReportUtils.js

@@ -1192,6 +1192,19 @@ function getDisplayNamesWithTooltips(personalDetailsList, isMultipleParticipantR
            pronouns,
        };
    });
+
+    return _.chain(displayNames)


Doesn't chain support .map? If it does, then we should chain on personalDetailsList instead.

allroundexperts

Changes needed.

akinwale · 2023-10-02T07:31:53Z

Changes needed.

@allroundexperts Pushed a new commit.

allroundexperts · 2023-10-02T07:54:54Z

src/pages/home/HeaderView.js

@@ -86,6 +86,8 @@ function HeaderView(props) {
    const isAutomatedExpensifyAccount = ReportUtils.hasSingleParticipant(props.report) && ReportUtils.hasAutomatedExpensifyAccountIDs(participants);
    const parentReportAction = ReportActionsUtils.getParentReportAction(props.report);
    const isCanceledTaskReport = ReportUtils.isCanceledTaskReport(props.report, parentReportAction);
+    // Use sorted display names for the fullTitle instead of title for group chats on native small screen widths
+    const sortedDisplayNames = isMultipleParticipant ? _.map(displayNamesWithTooltips, ({displayName}) => displayName).join(', ') : '';


Why don't we need to filter here like we did above? I think its better to create two utility functions, one called isActiveGroupChat and the other one called displayNamesFromTooltips.

I guess in the case of the LHN, the related test specifically did not set a displayName, and there are no tests for the header view.

We only need to check if it's an active group chat in the LHN, which is done in the OptionRowLHN component, so I don't think a utility method is necessary here.

For the sorted display names joined as a string, I can add a utility method to ReportUtils for this which returns the value of:

_.filter( _.map(displayNamesWithTooltips, ({displayName}) => displayName), (displayName) => !_.isEmpty(displayName), ).join(', ')

Then we can use this in both HeaderView and OptionRowLHN. Agree?

I still feel like the function to check if its a group chat would come in handy sometime later down the road. Abstracting it into a utility function doesn't hurt. Does it?

That makes sense but my reservation was since optionItem is not actually a properly defined type (the prop type doesn't list its properties), it may be only specific to the OptionRow* components.

Why didn't I notice that before 😵‍💫

You're right. It doesn't make sense to add this.

allroundexperts · 2023-10-02T08:43:49Z

src/components/LHNOptionsList/OptionRowLHN.js

@@ -104,6 +104,11 @@ function OptionRowLHN(props) {
    const shouldShowGreenDotIndicator =
        !hasBrickError && (optionItem.isUnreadWithMention || optionItem.isWaitingForTaskCompleteFromAssignee || ReportUtils.isWaitingForIOUActionFromCurrentUser(optionItem));

+    const fullTitle =
+        optionItem.type === CONST.REPORT.TYPE.CHAT && !optionItem.isArchivedRoom && optionItem.displayNamesWithTooltips && optionItem.displayNamesWithTooltips.length > 1


lodashGet(optionItem, 'displayNamesWithTooltips', 0) > 1 doesn't work?

Nice tip. It works. Just needed to add .length in the second parameter.

allroundexperts · 2023-10-02T10:23:26Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Mobile Web - Chrome

Screen.Recording.2023-10-02.at.2.35.41.PM.mov

Mobile Web - Safari

Screen.Recording.2023-10-02.at.2.33.01.PM.mov

Desktop

iOS

Screen.Recording.2023-10-02.at.2.34.38.PM.mov

Android

allroundexperts

Looks good!

stitesExpensify

lgtm

OSBotify · 2023-10-04T00:05:40Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2023-10-04T00:47:03Z

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
App start TTI	1191.186 ms → 1274.114 ms (+82.928 ms, +7.0%) 🔴
App start runJsBundle	827.000 ms → 891.063 ms (+64.063 ms, +7.7%) 🔴

Show details

Name	Duration
App start TTI	Baseline Mean: 1191.186 ms Stdev: 39.463 ms (3.3%) Runs: 1098.5204949998297 1119.6601459998637 1128.765982999932 1140.5005640001036 1152.3817480001599 1158.4563469998538 1164.493702000007 1164.8058919999748 1171.0132160000503 1173.7511970000342 1173.946585000027 1176.57151899999 1183.6346410000697 1186.6807450000197 1186.8287129998207 1188.3926209998317 1192.7962369997986 1198.4645899999887 1198.5228720000014 1199.5083070001565 1204.8888630000874 1206.0677100000903 1211.4656119998544 1216.9081660001539 1217.5991259999573 1226.8744450001977 1228.1756819998845 1228.811315999832 1235.5882370001636 1237.7995600001886 1258.9906489998102 1287.0828840001486 Current Mean: 1274.114 ms Stdev: 35.590 ms (2.8%) Runs: 1214.4628750002012 1215.508574000094 1219.8235989999957 1224.4545760001056 1230.2288580001332 1238.2502299998887 1244.9786510001868 1245.8778880001046 1249.603819000069 1252.2377610001713 1253.5847740001045 1258.9012799998745 1263.998960999772 1264.0382940000854 1268.634908999782 1277.0541059998795 1278.144685999956 1279.667843000032 1281.2115079998039 1283.4386049997993 1287.3966290000826 1297.5418279999867 1297.6216879999265 1298.5446290001273 1301.26601199992 1301.5142160002142 1305.5351999998093 1306.9835520000197 1309.9796250001527 1318.2172409999184 1336.0221299999394 1366.9306870000437
App start runJsBundle	Baseline Mean: 827.000 ms Stdev: 35.878 ms (4.3%) Runs: 741 760 763 771 796 797 804 808 810 811 816 818 821 822 824 825 826 833 833 836 836 843 845 847 854 866 866 873 875 875 875 894 Current Mean: 891.063 ms Stdev: 26.431 ms (3.0%) Runs: 845 851 856 858 859 860 861 865 868 878 878 879 881 888 888 891 894 895 896 897 898 899 904 910 914 915 920 927 933 934 935 937

Meaningless Changes To Duration

Show entries

Name	Duration
Open Search Page TTI	662.379 ms → 664.606 ms (+2.228 ms, ±0.0%)
App start nativeLaunch	20.226 ms → 21.645 ms (+1.419 ms, +7.0%)
App start regularAppStart	0.018 ms → 0.016 ms (-0.003 ms, -14.0%)

Show details

Name	Duration
Open Search Page TTI	Baseline Mean: 662.379 ms Stdev: 44.286 ms (6.7%) Runs: 596.3782139997929 604.3501789998263 609.0503739998676 609.6276050000452 613.9129229998216 615.1086830003187 623.431233999785 629.5472009996884 631.5530189997517 638.1389570003375 638.1740319998935 638.4069419996813 640.4166670003906 642.0327559998259 642.2799490001053 645.2939460002817 647.1811529998668 655.8922529998235 656.8850099998526 658.0575359999202 660.1553960000165 669.975424000062 670.3733729999512 682.2482910002582 706.4556879997253 706.4791259998456 717.1802989998832 720.1056320001371 726.2738859998062 732.5853679999709 741.9984130002558 742.5597330001183 746.3861489999108 Current Mean: 664.606 ms Stdev: 42.703 ms (6.4%) Runs: 615.4627279997803 616.6004229998216 617.0012619998306 617.6864419998601 620.4443369996734 622.071248000022 625.7834469997324 625.7858890001662 625.8146159998141 628.1507170000114 628.4126789998263 630.7208259999752 630.8535569999367 640.6065679998137 641.8999840002507 646.0887449998409 647.7287189997733 662.4719650000334 663.552612000145 675.4107670001686 676.3731689997949 687.6141760000028 688.996826000046 695.8269050000235 696.3651939998381 699.3960369997658 705.8784179999493 711.9043379998766 720.0624190000817 728.1944989999756 736.8096110001206 739.2425129995681 762.8028159998357
App start nativeLaunch	Baseline Mean: 20.226 ms Stdev: 1.640 ms (8.1%) Runs: 17 18 18 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 20 21 22 22 23 23 23 23 24 Current Mean: 21.645 ms Stdev: 1.976 ms (9.1%) Runs: 19 19 19 19 19 19 20 20 20 20 20 21 21 21 21 21 22 22 23 23 23 23 23 23 23 24 24 24 25 25 25
App start regularAppStart	Baseline Mean: 0.018 ms Stdev: 0.001 ms (8.0%) Runs: 0.015217999927699566 0.01607199991121888 0.016235999763011932 0.016561000142246485 0.016805000137537718 0.016805000137537718 0.016846000216901302 0.01696800021454692 0.01717099966481328 0.0172520000487566 0.017374999821186066 0.017456000205129385 0.017578000202775 0.01761800004169345 0.017821999732404947 0.01798499980941415 0.01806599972769618 0.018066000193357468 0.018350999802350998 0.018757999874651432 0.01883900025859475 0.0188400000333786 0.01912399986758828 0.019450000021606684 0.019857000093907118 0.01993800001218915 0.020345000084489584 0.020508000161498785 0.02075199969112873 0.020752000156790018 Current Mean: 0.016 ms Stdev: 0.001 ms (5.5%) Runs: 0.0138349998742342 0.01436399994418025 0.014485000167042017 0.014527000021189451 0.014607999939471483 0.014769999776035547 0.014851999934762716 0.01509599993005395 0.015135999768972397 0.015137000009417534 0.015218000393360853 0.01525900000706315 0.015420999843627214 0.015503000002354383 0.015585000161081553 0.015666000079363585 0.015666000079363585 0.015746999997645617 0.01582799991592765 0.015868999995291233 0.015868999995291233 0.015868999995291233 0.01599099999293685 0.01607199991121888 0.01607300015166402 0.016559999901801348 0.016561000142246485 0.0167239997535944 0.017537999898195267 0.017700000200420618

github-actions · 2023-10-04T00:47:04Z

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

OSBotify · 2023-10-05T00:39:48Z

🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.3.78-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-06T07:54:45Z

🚀 Deployed to production by https://github.com/Beamanator in version: 1.3.78-4 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-10-06T08:27:00Z

🚀 Deployed to staging by https://github.com/stitesExpensify in version: 1.3.79-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-10-09T22:35:34Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-10-09T22:44:27Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	cancelled 🔪
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-10-09T23:20:21Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

display sorted names for group chats in the header view and LHN

a66f4e1

akinwale requested a review from a team as a code owner September 26, 2023 05:19

melvin-bot bot removed the request for review from a team September 26, 2023 05:19

melvin-bot bot requested a review from allroundexperts September 26, 2023 05:19

akinwale mentioned this pull request Sep 26, 2023

[HOLD for payment 2023-11-01] [HOLD for payment 2023-10-13] [$500] Web - The order of avatars on the header sometimes matches the avatars on chat view #28048

Closed

6 tasks

akinwale added 3 commits September 26, 2023 07:38

modify tests to handle display name sorting logic

85083c2

use displayNamesWithTooltips list to set the sortedDisplayNames value

46a7ac0

use sorted display names as title in LHN only when the type is chat

c6a7553

fix merge conflict

c43a812

allroundexperts reviewed Oct 2, 2023

View reviewed changes

allroundexperts suggested changes Oct 2, 2023

View reviewed changes

use chain on personalDetailsList

dab8610

assign full title value based on certain conditions

80ebbcf

akinwale requested a review from allroundexperts October 2, 2023 07:52

allroundexperts reviewed Oct 2, 2023

View reviewed changes

refactor shared code

2e3812a

akinwale requested a review from allroundexperts October 2, 2023 08:33

allroundexperts reviewed Oct 2, 2023

View reviewed changes

cleanup fullTitle condition check

fa70ee0

akinwale requested a review from allroundexperts October 2, 2023 09:04

allroundexperts approved these changes Oct 2, 2023

View reviewed changes

melvin-bot bot requested a review from stitesExpensify October 2, 2023 10:28

stitesExpensify approved these changes Oct 3, 2023

View reviewed changes

stitesExpensify merged commit 299e054 into Expensify:main Oct 4, 2023
14 checks passed

github-actions bot added the DeployBlockerCash This issue or pull request should block deployment label Oct 4, 2023

OSBotify mentioned this pull request Oct 5, 2023

Deploy Checklist: New Expensify 2023-10-05 #28883

Closed

57 tasks

mountiny mentioned this pull request Oct 5, 2023

[CP Staging] Revert "fix: use the same sort order for header display names and avatars in a group chat view" #28929

Merged

OSBotify mentioned this pull request Oct 6, 2023

Deploy Checklist: New Expensify 2023-10-06 #28983

Closed

akinwale mentioned this pull request Oct 6, 2023

fix: sort header display names and avatars in a group chat view #28991

Merged

59 tasks

fix: use the same sort order for header display names and avatars in a group chat view #28218

fix: use the same sort order for header display names and avatars in a group chat view #28218

Conversation

akinwale commented Sep 26, 2023

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Sep 26, 2023

akinwale commented Sep 28, 2023

allroundexperts commented Sep 28, 2023

akinwale commented Sep 29, 2023

akinwale commented Oct 2, 2023

allroundexperts commented Oct 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allroundexperts left a comment

Choose a reason for hiding this comment

akinwale commented Oct 2, 2023

Choose a reason for hiding this comment

akinwale Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allroundexperts commented Oct 2, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

allroundexperts left a comment

Choose a reason for hiding this comment

stitesExpensify left a comment

Choose a reason for hiding this comment

OSBotify commented Oct 4, 2023

github-actions bot commented Oct 4, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

github-actions bot commented Oct 4, 2023

OSBotify commented Oct 5, 2023

OSBotify commented Oct 6, 2023

OSBotify commented Oct 6, 2023

OSBotify commented Oct 9, 2023

OSBotify commented Oct 9, 2023

OSBotify commented Oct 9, 2023

akinwale Oct 2, 2023 •

edited

Loading

allroundexperts commented Oct 2, 2023 •

edited

Loading