Implement Archived Workspace Chat UI #7831

roryabraham · 2022-02-19T17:47:41Z

Details

Implements the Archived Workspace Chat UI.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/195474

Tests

Storybook

Run storybook, verify that you can see the new stories for the Banner component.

There are a number of issues with Storybook at present, none having to do with this PR or the new stories.

Setup

Create and validate 5 accounts – 1 policy admin and 4 employees.
Sign in as the policy admin in NewDot.
Create a new workspace.
Go to Settings > <Your workspace> > Manage members and invite all four employees to the workspace.

Account Closed

Open the workspace chat for employee A. Verify that it's in the open state.
- BUG: I had to sign out and sign back in for the workspace chat(s) to appear.
Sign into OldDot as employee A. Go to Settings > Account > Account Details and close the account.
Go back to NewDot. Verify that the workspace chat for employee is archived, and that you see the following:
- BUG: I had to refresh the page for the UI to update.
Go back to the workspace chat for the employee w/ the closed account, and verify that you see the following:

Account Merged

Sign in to NewDot as employee B.
Go to Settings > Profile and set a first and last name
Sign in to NewDot as employee C.
Go to Settings > Profile and set a first and last name
Sign in to NewDot as the admin and find the workspace chat for employee C.
Sign in to OldDot as employee B.
Go to Settings > Account > Account Details and merge employee C's account into employee B's account.
Go back to NewDot and employee C's workspace chat should now look like this:
Go to NewDot Settings > Preferences and switch the language to Spanish.
Go back to the workspace chat for employee C and you should see the following copy:

Removed from Policy

Testing blocked on https://github.com/Expensify/Auth/pull/6456

Policy Deleted

Testing blocked on https://github.com/Expensify/Auth/pull/6456

Verify that no errors appear in the JS console

QA Steps

Most of this will be tested internally before we add a large suite of regressions to cover workspace chats. However, there is something that needs to be QA'd here (Chrome web only) to prevent performance regressions.

Sign out.
Open the JS console. Make sure you have verbose logs enabled. This dropdown should say All Levels:
Sign into a high traffic account with very large policies.
Search the logs for Timing:expensify.cash.personal_details_formatted
Verify that the number is very low (ideally less than 1000ms).
Repeat these same steps on production with the same account, and post the difference in timing between staging and production in this PR.

Tested On

Screenshots

Web

All of the screenshots for the above steps were taken on web.

Mobile Web

Desktop

iOS

Android

roryabraham · 2022-02-23T01:11:10Z

src/styles/styles.js

@@ -1329,6 +1320,15 @@ const styles = {
        minHeight: variables.componentSizeNormal,
    },

+    chatFooter: {


This is the same as chatItemCompose, just renamed. I just moved it so that it wasn't in between a number of other chatItem* styles.

# Conflicts: # src/pages/home/report/ReportActionCompose.js

src/libs/reportUtils.js

src/pages/home/ReportScreen.js

src/pages/home/report/ReportActionCompose.js

src/CONST/index.js

roryabraham · 2022-02-23T21:48:18Z

Updated. @TomatoToaster let me know if this is what you had in mind, not sure i completely understood your last comment.

src/CONST/index.js

# Conflicts: # src/pages/home/report/ReportActionCompose.js

roryabraham · 2022-03-23T05:05:20Z

This is ready for review. The welcome messages are still weird for archived workspace chats but I'd prefer to handle that in a follow-up if that's okay with everyone. Created an issue here: #8272

marcaaron

Looks great - had a few ideas for improvement.

src/components/ArchivedReportFooter.js

src/components/Banner.js

src/libs/LoginUtils.js

src/libs/ValidationUtils.js

marcaaron · 2022-03-23T16:24:35Z

src/libs/actions/PersonalDetails.js

@@ -87,20 +88,20 @@ function formatPersonalDetails(personalDetailsList) {

    // This method needs to be SUPER PERFORMANT because it can be called with a massive list of logins depending on the policies that someone belongs to
    // eslint-disable-next-line rulesdir/prefer-underscore-method
-    Object.keys(personalDetailsList).forEach((login) => {
-        const personalDetailsResponse = personalDetailsList[login];
+    Object.entries(personalDetailsList).forEach(([login, details]) => {


The terrifying comment above this line seems to imply that Object.keys() + forEach() is more performant than underscore. I can't really think of why that would be, but at the same time my default is to trust whoever wrote it.

Since we are changing this to Object.entries() is there some way we can verify there are no performance regressions. Maybe testing against production with an account that will have many personal details (someone who has been at Expensify for a while perhaps)?

Since we are changing this to Object.entries() is there some way we can verify there are no performance regressions. Maybe testing against production with an account that will have many personal details (someone who has been at Expensify for a while perhaps)?

Yes, I can add QA steps here.

FWIW here is the PR that introduced that, and the problem with the code before wasn't necessarily that we were using underscore instead of vanilla JS, just that we were using the spread operator in a reduce, which is an O(n^2) operation because it copies the whole object for every iteration.

So it would probably be safe to just use _.each(personalDetailsList, (details, login) => {...}) because it doesn't have that spread inside a reduce.

Added QA steps. Applause has access to some high-volume accounts with large policies (Expensify.org policies I think) that will give them thousands of personalDetails.

A bit of extra context here: #7649 (comment)

Ok cool. It's a weird comment then and sounds like we could update this to use underscore, but NAB.

src/libs/reportUtils.js

src/pages/home/report/ReportActionCompose.js

src/stories/Banner.stories.js

marcaaron · 2022-03-23T20:55:21Z

src/components/Banner.js

+                    />
+                </View>
+                {
+                    props.shouldRenderHTML


NAB, alternatively just always render HTML because plain text run through an HTML renderer is just text anyway

marcaaron · 2022-03-23T20:56:31Z

src/libs/actions/PersonalDetails.js

@@ -87,20 +88,20 @@ function formatPersonalDetails(personalDetailsList) {

    // This method needs to be SUPER PERFORMANT because it can be called with a massive list of logins depending on the policies that someone belongs to
    // eslint-disable-next-line rulesdir/prefer-underscore-method
-    Object.keys(personalDetailsList).forEach((login) => {
-        const personalDetailsResponse = personalDetailsList[login];
+    Object.entries(personalDetailsList).forEach(([login, details]) => {


Ok cool. It's a weird comment then and sounds like we could update this to use underscore, but NAB.

TomatoToaster · 2022-03-24T03:49:15Z

I think we can merge this because the remaining NAB is minor. We can open a separate PR if we want to remove the renderHTML and test it works the same on all platforms.

OSBotify · 2022-03-24T03:58:44Z

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

OSBotify · 2022-03-24T21:00:31Z

🚀 Deployed to staging by @TomatoToaster in version: 1.1.46-0 🚀

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

mvtglobally · 2022-03-25T18:19:59Z

@roryabraham We are seeing the following numbers in Console. Response is missing time variant. Let us know if this looks good

roryabraham · 2022-03-25T19:19:26Z

Thanks. That's in ms so I think this is performant enough to be a QA-pass

OSBotify · 2022-03-30T19:26:50Z

🚀 Deployed to production by @timszot in version: 1.1.46-3 🚀

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

puneetlath · 2023-01-06T19:54:15Z

@roryabraham @marcaaron FYI I believe this PR caused this bug: #13452

puneetlath · 2023-01-06T19:55:57Z

Actually nvm. I think it was this PR actually: #11363

roryabraham added 4 commits February 19, 2022 07:57

Create report action const for ARCHIVE_REASON report action

2681e03

Update isArchivedRoom to account for policy expense chats

a9a454b

Create ArchivedReportFooter component

2faf3d2

Look for last archived reason in ReportActionCompose

dbb991d

roryabraham self-assigned this Feb 19, 2022

roryabraham added 9 commits February 22, 2022 16:22

Use CLOSED reportAction instead of ARCHIVE_REASON

40ac799

Simplify component to only include the one hoverable element

20715e2

Use reportArchiveReasons translation key

ab38779

Pre-emptively add translations for other archive reasons

efa7db0

Add known spanish translations

a53c4ea

Move chat footer rendering logic to ReportScreen

36b80d3

Get rid of unnecessary arrow function in render

844cc86

Remove outdate use of isArchivedRoom from ReportActionCompose

7731013

Simplify diff for reviewers

1b45be4

roryabraham commented Feb 23, 2022

View reviewed changes

roryabraham added 4 commits February 22, 2022 17:47

Clean up component style

7308254

Add support for bold text in footer

b6e2a5d

Merge branch 'main' into Rory-ArchiveUI

fb4ff5b

# Conflicts: # src/pages/home/report/ReportActionCompose.js

Fix JS style

f9b6a66

TomatoToaster reviewed Feb 23, 2022

View reviewed changes

src/libs/reportUtils.js Show resolved Hide resolved

TomatoToaster reviewed Feb 23, 2022

View reviewed changes

src/pages/home/ReportScreen.js Outdated Show resolved Hide resolved

TomatoToaster suggested changes Feb 23, 2022

View reviewed changes

src/pages/home/report/ReportActionCompose.js Show resolved Hide resolved

TomatoToaster suggested changes Feb 23, 2022

View reviewed changes

src/CONST/index.js Outdated Show resolved Hide resolved

roryabraham added 2 commits February 23, 2022 13:37

Fix JS style (again)

facde6a

Rename MANUALLY_ARCHIVED to DEFAULT

2bcb7a1

roryabraham changed the title ~~[WIP] Create report action const for ARCHIVE_REASON report action~~ [WIP] Implement Archived Chat Room UI Feb 24, 2022

TomatoToaster reviewed Feb 25, 2022

View reviewed changes

src/CONST/index.js Outdated Show resolved Hide resolved

roryabraham added 2 commits March 1, 2022 15:50

Merge branch 'main' into Rory-ArchiveUI

7bbb79a

# Conflicts: # src/pages/home/report/ReportActionCompose.js

Update translations

11a5fc1

roryabraham marked this pull request as ready for review March 23, 2022 05:04

roryabraham requested a review from a team as a code owner March 23, 2022 05:04

melvin-bot bot requested review from timszot and removed request for a team March 23, 2022 05:04

roryabraham mentioned this pull request Mar 23, 2022

Fix welcome message for archived workspace chats #8272

Closed

Remove unused props

d067f19

marcaaron requested changes Mar 23, 2022

View reviewed changes

roryabraham added 2 commits March 23, 2022 11:23

Fix originalMessage.reason default prop

117241b

Use onyx collection key constant

8d5d53a

roryabraham changed the title ~~[No QA] Implement Archived Workspace Chat UI~~ Implement Archived Workspace Chat UI Mar 23, 2022

Use state and status constants

8e5442c

roryabraham requested a review from marcaaron March 23, 2022 19:13

Take HTML out of translation files

811379b

marcaaron approved these changes Mar 23, 2022

View reviewed changes

timszot approved these changes Mar 23, 2022

View reviewed changes

TomatoToaster approved these changes Mar 24, 2022

View reviewed changes

TomatoToaster merged commit eb3b18e into main Mar 24, 2022

TomatoToaster deleted the Rory-ArchiveUI branch March 24, 2022 03:49

OSBotify mentioned this pull request Mar 24, 2022

Deploy Checklist: New Expensify 2022-03-24 #8305

Closed

69 tasks

NikkiWines mentioned this pull request Apr 7, 2022

Workspace chat - Attachments hides under the footer text when scrolling #8532

Closed

puneetlath mentioned this pull request Jan 6, 2023

[$1000] The cursor is displayed as pointer on the archived room banner text #13452

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Archived Workspace Chat UI #7831

Implement Archived Workspace Chat UI #7831

roryabraham commented Feb 19, 2022 •

edited

Loading

roryabraham Feb 23, 2022

roryabraham commented Feb 23, 2022

roryabraham commented Mar 23, 2022 •

edited

Loading

marcaaron left a comment

marcaaron Mar 23, 2022

roryabraham Mar 23, 2022

roryabraham Mar 23, 2022

roryabraham Mar 23, 2022

roryabraham Mar 23, 2022

marcaaron Mar 23, 2022

marcaaron Mar 23, 2022

marcaaron Mar 23, 2022

TomatoToaster commented Mar 24, 2022

OSBotify commented Mar 24, 2022

OSBotify commented Mar 24, 2022

mvtglobally commented Mar 25, 2022

roryabraham commented Mar 25, 2022

OSBotify commented Mar 30, 2022

puneetlath commented Jan 6, 2023

puneetlath commented Jan 6, 2023

Implement Archived Workspace Chat UI #7831

Implement Archived Workspace Chat UI #7831

Conversation

roryabraham commented Feb 19, 2022 • edited Loading

Details

Fixed Issues

Tests

Storybook

Setup

Account Closed

Account Merged

Removed from Policy

Policy Deleted

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Choose a reason for hiding this comment

roryabraham commented Feb 23, 2022

roryabraham commented Mar 23, 2022 • edited Loading

marcaaron left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomatoToaster commented Mar 24, 2022

OSBotify commented Mar 24, 2022

OSBotify commented Mar 24, 2022

mvtglobally commented Mar 25, 2022

roryabraham commented Mar 25, 2022

OSBotify commented Mar 30, 2022

puneetlath commented Jan 6, 2023

puneetlath commented Jan 6, 2023

roryabraham commented Feb 19, 2022 •

edited

Loading

roryabraham commented Mar 23, 2022 •

edited

Loading