Added persistent Created Room Header to the Chat by sig5 · Pull Request #7445 · Expensify/App

sig5 · 2022-01-27T15:12:47Z

Details

We needed to replace the initial Room header with a persistent room header. I have added a new component that renders when action-type is CREATED.
I have also updated the names of a component and styles to reflect the change (Hope that's fine).

Fixed Issues

$ #7077

Tests

Open a new/existing chat.
Verify the position of the header and scroll-ability within the chat.

Verify that no errors appear in the JS console

QA Steps

Open a new/existing chat.
Verify the position of the header and scroll-ability within the chat.

[X ] Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Screencast.from.27-01-22.08_07_07.PM.IST.mp4

Mobile Web

Desktop

iOS

Android

sig5 · 2022-01-27T15:23:45Z

@parasharrajat I do not have a mac or an iOS device atm, can you test it if it looks okay on those devices? Thanks :D

parasharrajat

Testing now...

parasharrajat · 2022-01-27T15:42:04Z

Apart from it, I think there is more gap between first message and the Created view.

@shawnborton Could you please review the design here? Thank you 🙇 .

Leaving some images for broader view.

When Chat is empty

shawnborton · 2022-01-27T16:08:26Z

Is this just for rooms or does this fix this behavior for all new chat types? Like 1:1, group messages, etc?

From our mockups, it looks like we have 32px padding at the bottom of the chat header:

parasharrajat · 2022-01-27T16:20:51Z

Tested on IOS and Desktop. looks good code-wise. But Should the text in the center extend more on the mobile? It seems shrunk.

shawnborton · 2022-01-27T16:31:17Z

I think we might be able to widen that area a bit. In your screenshot, you can see how "him" falls to a third line because of how skinny that area is currently.

sig5 · 2022-01-28T00:19:14Z

Is this just for rooms or does this fix this behavior for all new chat types? Like 1:1, group messages, etc?

From what I know, this is supposed to be added in the beginning of all Chat Reports (#7077 (comment))

I think we might be able to widen that area a bit. In your screenshot, you can see how "him" falls to a third line because of how skinny that area is currently.

EDIT: My bad, Width is set as 70% in ReportWelcomeText which is causing the wrapping of the text, will fix and push.
I think adding a slack of 4px on each side should be the sweet spot for us. It should cover most of the names and looks almost as good as 20px padding. Opinions?

20px padding	16px padding

cc: @shawnborton , @parasharrajat

parasharrajat · 2022-01-28T00:35:40Z

Not a design expert. but I think it can be widened more. The Left and right side gap should be a minimum of 20px from screen edges. Thus there is enough space around so that history with can fit in a single line.

shawnborton · 2022-01-28T07:29:26Z

I agree, I think we can just do 20px padding on the left and right with no max-width (just let it go full container) and we should be good to go.

sig5 · 2022-01-28T11:19:52Z

Updated Changes, the width specification matches that of the mockup now.Updated screenshots in the PR.

parasharrajat

UI looks good. Just a small tweaks.

parasharrajat

LGTM. leaving a few no blocker comments.

cc: @marcaaron

🎀 👀 🎀 C+ reviewed

parasharrajat · 2022-01-29T00:21:25Z

+    pt5: {
+        paddingTop: 20,
+    },
+


Please only add styles when they are used.

Suggested change

pt5: {

paddingTop: 20,

},

parasharrajat · 2022-01-29T00:23:29Z

+        <View style={[styles.chatContent,
+            styles.pb8,
+            styles.p5,
+        ]}


Suggested change

<View style={[styles.chatContent,

styles.pb8,

styles.p5,

]}

<View style={[

styles.chatContent,

styles.pb8,

styles.p5,

]}

marcaaron · 2022-01-31T18:30:07Z

-            .filter(action => action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU
-                    || action.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT
-                    || action.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED)
+            .filter(action => _.contains(_.values(CONST.REPORT.ACTIONS.TYPE), action.actionName))


This doesn't seem like a safe change. We might have action types that should not be rendered in the future added to this list. Can we just add the new action to this list?

.filter(action => action.actionName === CONST.REPORT.ACTIONS.TYPE.IOU || action.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED || action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED)

Yes, that makes sense. Will do!

Sounds good.

sig5 · 2022-02-02T22:34:28Z

Updated with the requested changes!

OSBotify · 2022-02-02T22:54:27Z

✋ 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-02-04T12:23:40Z

🚀 Deployed to staging by @marcaaron in version: 1.1.36-0 🚀

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

OSBotify · 2022-02-07T18:50:40Z

🚀 Deployed to production by @roryabraham in version: 1.1.36-0 🚀

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

sig5 added 6 commits January 27, 2022 18:46

Renamed EmptyStateAvatar

aeb8eb9

modified styles

da5f317

Added created action type

4fbd6a9

Removed old component

24dc272

Added new Header component

2cc3508

fix errors

1d10942

sig5 requested a review from a team as a code owner January 27, 2022 15:12

MelvinBot requested review from marcaaron and parasharrajat and removed request for a team January 27, 2022 15:12

parasharrajat suggested changes Jan 27, 2022

View reviewed changes

Comment thread src/pages/home/report/ReportActionItemCreated.js Outdated

Comment thread src/pages/home/report/ReportActionItemCreated.js Outdated

Comment thread src/pages/home/report/ReportActionsView.js Outdated

sig5 added 3 commits January 28, 2022 16:04

Added displayname,comments and modified styling

15a86ce

Cleaned up conditionals

773f932

Updated proptype comments

8c43994

sig5 changed the title ~~Added persistent created room header to Chat~~ Added persistent Created Room Header to the Chat Jan 28, 2022

parasharrajat suggested changes Jan 28, 2022

View reviewed changes

Comment thread src/pages/home/report/ReportActionItemCreated.js Outdated

Comment thread src/styles/styles.js Outdated

shawnborton reviewed Jan 28, 2022

View reviewed changes

Comment thread src/styles/styles.js Outdated

Addressed issues

0e203a0

parasharrajat previously approved these changes Jan 28, 2022

View reviewed changes

Comment thread src/pages/home/report/ReportActionItemCreated.js Outdated

Improved css

8a204b8

sig5 dismissed parasharrajat’s stale review via 8a204b8 January 29, 2022 00:04

parasharrajat suggested changes Jan 29, 2022

View reviewed changes

Removed p5 from styles

ce055a5

parasharrajat previously approved these changes Jan 29, 2022

View reviewed changes

shawnborton previously approved these changes Jan 31, 2022

View reviewed changes

marcaaron requested changes Jan 31, 2022

View reviewed changes

Reverted filter Types change for future code quality.

96c6441

sig5 dismissed stale reviews from shawnborton and parasharrajat via 96c6441 January 31, 2022 18:57

marcaaron approved these changes Feb 2, 2022

View reviewed changes

marcaaron merged commit 9d6e07a into Expensify:main Feb 2, 2022

marcaaron assigned sig5 Feb 2, 2022

OSBotify mentioned this pull request Feb 4, 2022

Deploy Checklist: New Expensify 2022-02-04 #7564

Closed

22 tasks

botify mentioned this pull request Feb 7, 2022

[HOLD for payment 2022-02-14] FEATURE REQUEST: Update the empty state of a chat to be center aligned and show persistently #7077

Closed

parasharrajat mentioned this pull request Feb 9, 2022

mWeb - Chat - New Group with long emails display goes beyond borders #7609

Closed

Conversation

sig5 commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Uh oh!

sig5 commented Jan 27, 2022

Uh oh!

parasharrajat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

parasharrajat commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

When Chat is empty

Uh oh!

shawnborton commented Jan 27, 2022

Uh oh!

parasharrajat commented Jan 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shawnborton commented Jan 27, 2022

Uh oh!

sig5 commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parasharrajat commented Jan 28, 2022

Uh oh!

shawnborton commented Jan 28, 2022

Uh oh!

sig5 commented Jan 28, 2022

Uh oh!

parasharrajat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

parasharrajat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

parasharrajat Jan 29, 2022

Choose a reason for hiding this comment

Uh oh!

parasharrajat Jan 29, 2022

Choose a reason for hiding this comment

Uh oh!

marcaaron Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

marcaaron Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

sig5 Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

marcaaron Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

parasharrajat Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

sig5 commented Feb 2, 2022

Uh oh!

OSBotify commented Feb 2, 2022

Uh oh!

OSBotify commented Feb 4, 2022

Uh oh!

sig5 commented Jan 27, 2022 •

edited

Loading

parasharrajat commented Jan 27, 2022 •

edited

Loading

parasharrajat commented Jan 27, 2022 •

edited

Loading

sig5 commented Jan 28, 2022 •

edited

Loading