Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[HOLD for payment 2021-12-06] [HOLD for payment 2021-11-25] Update "Be the first person to comment!" screen when you start a new DM #4678

Closed mallenexpensify closed 2 years ago

mallenexpensify commented 3 years ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

Sent DM message to someone I hadn't messaged before

Expected Result:

The blank screen should have avatars, a message for "This is the beginning of your chat history with XXXX (pronouns), as well as showing the timezone above the message input when messaging someone 1:1.

Actual Result:

Only see "Be the first person to comment!"

Solution

Let's update this state/area to include the users icon, name, preferred pronouns and the time in their time zone (only if 1:1 DM).
image

Design files are here in Figma

Platform:

Where is this issue occurring?

Expensify/Expensify Issue URL: link

View all open jobs on Upwork

Upwork job: https://www.upwork.com/jobs/~0159561fdd76ccd036

MelvinBot commented 3 years ago

Triggered auto assignment to @michaelhaxhiu (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

Santhosh-Sellavel commented 3 years ago

@mallenexpensify

What's it gonna be for the new group chat?

Screenshot 2021-08-17 at 12 37 42 AM
mallenexpensify commented 3 years ago

Great point @Santhosh-Sellavel ! I addressed your question in an old internal issue we had been working on, I instructed folks to post comments/feedback to this issue. We have two choices

  1. Address group chats in this GH and figure out what we want before posting the job.
  2. Only focus on the single DM usecase for now and address groups later.
Santhosh-Sellavel commented 3 years ago

Is this going to be external? @mallenexpensify If it's I'm interest then.

An overview of implementation details,

mallenexpensify commented 3 years ago

It will very likely be external @Santhosh-Sellavel , @michaelhaxhiu needs to triage the issue first, part of that is figuring out what we want to do about group messages. My hunch is it's best to fix this issue specifically for the 1:1 DM usecase now then create a new issue in the future to address both group messages and possibly rooms.

@michaelhaxhiu, this shouldn't require a designer, an engineer should be able to take care of everything

rosegrech commented 3 years ago

Great question about the group. I want to say lets work on how the 1:1 looks like and then probably handle the group in a new GH to keep things neat and tidy. I'd love to say whatever we use for 1:1 will work for the group but ard to say that as of now :)

shawnborton commented 3 years ago

@megankelso @michelle-thompson I vaguely recall that we explored what this screen would look like for various use cases... actually I think it was @michelle-thompson that did it for Concierge Travel.

So basically we could do something like this? image

And maybe before we have those features ready to go, we could just use a generic message for the group situation like so: image

And then in terms of scalability, looks like Michelle had mocked up something like this for when a group has lots of participants: image

michelle-thompson commented 3 years ago

Cool, happy to move forward with the generic messaging you've mocked up above!

shawnborton commented 3 years ago

Sweet, so I think maybe we want to make the avatars in my mockup a bit smaller like your original mocks (the one at the bottom) for scalability purposes. Otherwise, then I think we just need to decide if we're going to add Group chats to the exported Upwork issue?

michelle-thompson commented 3 years ago

Ahh, you mean so the size of the avatar never has to change? And I figure might as well add group chats to this issue.

shawnborton commented 3 years ago

Yeah, I think I like the size you had here: image

...better than the larger size I used. So that gives us something like this: image

michelle-thompson commented 3 years ago

Agreed, I think that looks good too

megankelso commented 3 years ago

I agree, this looks good! I do wonder if we should make this a bit more consistent. What comes to mind for me is using the existing UI pattern to show local time (above the chat input) and including the same "this is the beginning for your chat" message, but perhaps adding the pronouns. Curious for thoughts?

Screen Shot 2021-08-17 at 11 19 20 AM
michelle-thompson commented 3 years ago

I'm into it! Though curious what this would look like if you were to DM someone in the same time zone?

megankelso commented 3 years ago

I'm suggesting it still show the time for the first chat, regardless of what time zone. Curious if you had something else in mind?

michelle-thompson commented 3 years ago

Ahhh, I see. I think that would be fine too, I'm keen with either direction!

shawnborton commented 3 years ago

Oh, I really like that idea about the timezone. Actually taking this a step further, perhaps the name above the message for the 1:1 case is redundant as well? So that leaves us with this:

image

megankelso commented 3 years ago

yes, agreed! we can drop the name. I like the addition of pronouns to the group but I'm not sure how scaleable it is for larger group chats. For example, if we added them to the next screen:

Screen Shot 2021-08-18 at 11 48 15 AM

Curious for thoughts. would it be weird to make a rule that pronouns are included only for group chats of 3 people and under or so?

michelle-thompson commented 3 years ago

I think you can have up to eight people in DMs, so it's possible to include pronouns for everyone! It looks a little busy, but I don't particularly mind that since this screen is temporary.

image

shawnborton commented 3 years ago

I agree, I think it will be okay. It's probably not very likely that all 8 members of the group will have their pronouns set anyways.

MelvinBot commented 3 years ago

@michaelhaxhiu Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mallenexpensify commented 3 years ago

@shawnborton @michelle-thompson will let you @michaelhaxhiu know when he should review it for triage? Also.. huge, double plus, bonus points if you can update the OP with the deliverables for the contributors.

shawnborton commented 3 years ago

Cool, I updated the original comment with the new screenshots, I think this is ready for @michaelhaxhiu

MelvinBot commented 3 years ago

Triggered auto assignment to @puneetlath (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

michaelhaxhiu commented 3 years ago

Posted to upwork just now, and added job link in GH body.

MirFahad58 commented 3 years ago
Screenshot 2021-08-24 at 12 48 30 AM

As shown in the picture, I suggest we can fix this by placing it above the text input not at the centre in a fixed position. reasons: 1 - on smaller screens fixed positions (which is the centre) will be under the keyboard. so it will not be visible. above the keyboard, we can use keyboard avoiding view and will be in the best position, not mater user have a smaller or larger device 2 - by default keyboard should be open (auto-focused) because the user is here to start the chat. this will reduce one more step 3 - In a group chat, we should see only two in that group and with others tag if he clicks other we can show other people in that chat as details, otherwise the user could be overwhelmed. 4 - I recommend "write a message" over "say hello" because it is not a social app or fun app, a professional app should use professional words.

shawnborton commented 3 years ago

I like the feedback that the keyboard should be open by default on mobile devices. However, let's please stick with our mockups where the empty state is vertically centered in the available empty space. We also want to keep "Say hello" as it's in line with our brand tone.

michaelhaxhiu commented 3 years ago

That's a good start @MirFahad58, nice!

I agree with Shawn's feedback regarding sticking to vertical centering, as shown in the mockups.

I like the feedback that the keyboard should be open by default on mobile devices.

Good call, one less step for the user. Can you show a quick screenshot of this screen with the keyboard expanded?

MirFahad58 commented 3 years ago

Ok i will share in while.

michaelhaxhiu commented 3 years ago

Sounds good. @puneetlath is assigned to review proposals here.

MirFahad58 commented 3 years ago

this one is with sticking to vertical centring.

Screenshot 2021-08-24 at 7 54 25 PM

and this one is for what I was suggesting.

Screenshot 2021-08-24 at 7 53 32 PM
shawnborton commented 3 years ago

Please stick with the style from our mockups. It looks like your mockups have a different font as well as the timezone being centered.

MirFahad58 commented 3 years ago

yeah, I just wanted to share the idea, just created my separate mock to get ideas and work around it.

MirFahad58 commented 3 years ago

the development will be based on styles from your mockups.

puneetlath commented 3 years ago

@michaelhaxhiu FYI I'm focused on allocations and then OOO the rest of the week, so going to unassign myself.

MelvinBot commented 3 years ago

Triggered auto assignment to @roryabraham (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

roryabraham commented 3 years ago

Okay, despite not making a formal proposal, it seems like @MirFahad58 has a pretty good grip on this task. @MirFahad58 If you're interested in working on this, can you please write up a proposal for how you would implement this in the code?

MirFahad58 commented 3 years ago

yeah sure @roryabraham

MirFahad58 commented 3 years ago

First, I will understand the flow in the app, and what you guys had used (like a UI library) or used custom components, so text inputs and other components must be already there, I will reuse with my purpose and styling, And again I will look in the code structure, how flexes are implemented.. etc I will start from the build a basic structure of the screen by adding flexes for each section, after that the header part and make a separate one so we can reuse it for different chats like group chats etc. After that will create a custom avatar as a separate component so we can pass the source and height width as props; Conditionally if there is no chat history we will show the "be the first person to comment" screen. and Input will have autoFocus so the keyboard should auto open. but for all this, I have to go through the code base and see which components are prebuilt and I can reuse those.

roryabraham commented 3 years ago

Okay, good enough for me @MirFahad58. @michaelhaxhiu let's hire @MirFahad58 on Upwork.

@MirFahad58 Since I believe this will be your first pull request in this repo, let me give you some pointers to help you succeed and to help ensure that the PR review moves quickly. I strongly recommend that you read over these resources before submitting your PR and refer to them as needed while you work:

  1. Please read our JavaScript style guide.
    • If necessary, prefer the use of Underscore.js over built-in JavaScript array methods. Underscore has lots of useful utilities and generally has better null-checking.
    • Also use lodashGet instead of any optional chaining.
  2. Read over the stylng guide. This provides guidance on creating styles in this repo. We want to keep our styles as consistent, generic, and reusable as possible.
  3. Read over the structure of the app. In particular, the directory structure, file naming/structure, and platform-specific style extensions will be relevant in this PR. We do not use the react-native Platform library (i.e: no Platform.select).
  4. Read the contributing guidelines.

Thanks! If you have any questions, refer to "asking questions" in the contributing guidelines.

MirFahad58 commented 3 years ago

Ok thanks

shawnborton commented 3 years ago

In terms of building the empty state component, keep in mind that this component will be reused for rooms as well. So we want to make sure we can extend this component to support a custom room avatar and a custom message below the avatar as well: image

MirFahad58 commented 3 years ago

Yeah sure

michaelhaxhiu commented 3 years ago

Hired on upwork!

arielgreen commented 3 years ago

@michaelhaxhiu @roryabraham please be sure you're also assigning the issue to the contributor upon hire.

MirFahad58 commented 3 years ago

Started working on the issue, linted the code, but was unable to commit. need someone to help.

MirFahad58 commented 3 years ago
Screenshot 2021-08-28 at 11 07 48 PM

getting verified tag for commits on other repos but unable to commit on this one, getting gpg error and already done with gpg step by step process.

Santhosh-Sellavel commented 3 years ago

@MirFahad58

commit on this one?

Are you trying to push your changes directly in this repo

MirFahad58 commented 3 years ago

@Santhosh-Sellavel no, forked and created a new branch from "main".

MirFahad58 commented 3 years ago
Screenshot 2021-08-28 at 11 32 53 PM

getting this error