Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: LHN, avatar and header updates to reports, workspace chats and chat rooms tied to workspaces. #20486

Closed
4 tasks done
JmillsExpensify opened this issue Jun 8, 2023 · 55 comments
Assignees
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Jun 8, 2023

While we recently implemented threads, tasks, and manual requests, we realized that threads followed one set of design patterns, while manual requests and tasks followed a different set of patterns. In an effort to clean this up for manual requests and IOUs – threads are already being implemented and tasks are being handled separately – this doc outlines a new, unified approach to ensure business use cases match the rest of the app.

LHN & Avatars
Screenshot 2023-06-08 at 21 18 23

Notes for completeness:

  • The workspace chat now uses a consistent avatar for members and admins, with the overarching goal being consistency and less confusion. Workspace chats use a 40x40 dual-avatar, with a larger workspace avatar and a smaller member avatar overlaid in the bottom-right. That said, the name of the workspace chat will continue to differ for admins, taking the form %memberName%, so that they’re able to find workspace chats for specific employees.
  • Expense reports use a 24x24 dual-avatar, with the rounded member avatar in the top-left and a rounded square workspace avatar in the bottom right. This dual avatar emphasizes the collaboration and shared nature of sending a request and eventually getting paid back. The title takes the form “%workspaceName% owes %amount%” (and updates to %workspaceName paid %amount% when settled).
  • IOU reports will similarly use a 24x24 dual-avatar, though both members have a rounded avatar in contrast. The title is the same as expense reports.
  • Requests made to the workspace will feature a larger 32x32 rounded member avatar, as well as a smaller 20x20 rounded square workspace avatar in the case of expense requests. The workspace overlay helps distinguish requests to a workspace vs a DM, which has no extra overlay. The title takes the form “%amount% request for %merchant% | %comment%”.
  • Threads in workspace rooms/reports will use a distinct “overlay” avatar from requests. A larger 40x40 circular member avatar as well as a smaller 20x20 rounded square workspace avatar overlaid in the bottom right. This similarly helps distinguish workspace threads from DMs, and emphasizes the member who made the parent comment. The title of the thread is the parent comment.
  • Threads in DMs/GroupDMs will simply use the avatar of the parent message sender.
  • All chat rooms tied to workspaces will feature the workspace avatar. This deprecates the chair and similar icons for user-created rooms.

Headers
image

Notes for completeness:

  • For the first line, the avatars and header follow the same convention as the LHN.
  • For the second line, we use “From %parentChat% in %policyName%”.
  • The parent report is always hyperlinked, displayed in blue and can be clicked to return to the parent report.

*iou/expenseReport subheaders

  • Remove the To: and amount
  • reposition the Pay button
  • modify the pay button copy, so it includes the report amount. i.e Pay $50 with Expensify

Mobile (Old | New):
image

Desktop (Old | New):
image

Rollout Plan

  • Update the subheader and parent navigation on all report types (PR) - @chiragsalian
  • Update the avatars in all places they feature (LHN, Chat Headers, Report Previews) (GH) - @grgia
  • Update the iou/expenseReport header logic. - @grgia - (GH)
    • Remove the To: and amount
    • reposition the Pay button
    • modify the pay button copy, so it includes the report amount. i.e Pay $50 with Expensify
    • Add the "wide" reportTotal to the expanded scrollable header
  • Update the iou/expense request header logic. - @grgia (GH)
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d3518465643567ea
  • Upwork Job ID: 1666939604347977728
  • Last Price Increase: 2023-06-08
@JmillsExpensify JmillsExpensify added Weekly KSv2 Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. labels Jun 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 8, 2023

Current assignees @JmillsExpensify and @trjExpensify are eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Jun 8, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01d3518465643567ea

@melvin-bot
Copy link

melvin-bot bot commented Jun 8, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @s77rt (Internal)

@JmillsExpensify JmillsExpensify changed the title Feature: LHN, avatar and header updates to IOU/Expenses, workspace chats and workspace chat rooms. Feature: LHN, avatar and header updates to IOU/Expenses, workspace chats and chat rooms tied to workspaces. Jun 8, 2023
@JmillsExpensify
Copy link
Author

Also @shawnborton did we decide one way or the other on all blue for the second line of the header rows? I still like the differentiated From but I think you had some doubts. Chime in now, because we're going to start implementing this soon!

@shawnborton
Copy link
Contributor

I'm cool with leaving From in gray to differentiate it, I agree it does look a bit nicer.

@grgia
Copy link
Contributor

grgia commented Jun 9, 2023

@shawnborton are we good to combine this issue with #19264 ?

@trjExpensify
Copy link
Contributor

I'm cool with leaving From in gray to differentiate it, I agree it does look a bit nicer.

I agree. @chiragsalian, want to add that as a commit to your PR before it gets reviewed and merged?

@trjExpensify
Copy link
Contributor

@shawnborton are we good to combine this issue with #19264 ?

I don't see why not. We can probably use this issue as a bit of an overarching tracker, and we can add those to the OP.

Breaking that down, is this what we need to complete the refresh across all report types?

  • Update the subheader and parent navigation on all report types (GH) - @chiragsalian
  • Update the avatars in all places they feature (LHN, Chat Headers, Report Previews) (GH) - @grgia
  • Deprecate the iou/expenseReport header logic.
    • Remove the To: and amount
    • reposition the Pay button
    • modify the pay button copy, so it includes the report amount. i.e Pay $50 with Expensify

On that third one, that's these updates in the header side-by-side:

Mobile (Old | New):
image

Desktop (Old | New):
image

Note: I've purposefully left out the new sub-total header stuff, I think that only needs to come first when we bring in card transactions and hold.

Does that sound about right to everyone? Missed anything, anything to add?

@melvin-bot

This comment was marked as off-topic.

@grgia
Copy link
Contributor

grgia commented Jun 9, 2023

Avatar portion should be done monday, just debugging a failing Jest test

@grgia
Copy link
Contributor

grgia commented Jun 9, 2023

On that, Code is done otherwise! PR draft here- #20512

@shawnborton
Copy link
Contributor

Amazing! And thank you Tom for breaking it down like that.

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jun 12, 2023
@grgia
Copy link
Contributor

grgia commented Jun 12, 2023

Could I get clarification on QA steps for creating an expense report vs expense request?
I'm able to make an expense request in LHN, but I'm not sure how we differentiate a normal workspace chat vs an expense report chat

@trjExpensify
Copy link
Contributor

pre-requisite: Have the account(s) on the policyExpenseChat beta to get access to a workspace chat.

I'm able to make an expense request in LHN, but I'm not sure how we differentiate a normal workspace chat vs an expense report chat

In the LHN:

  1. The workspace chat report will be called Shawn Borton
  2. The expenseReport will be called Duraflame owes $100.00
  3. The request will be called $100.00 request for Thursday Lunch

So as suggested QA steps, maybe like so...

  1. Make sure you have a couple of accounts on the policyExpenseChat beta (or use expensifail.com)
  2. Create a free plan workspace > set a custom workspace avatar
  3. Add a member to the workspace
  4. Now log-in as the member, set a custom avatar
  5. Head to your workspace chat in the LHN
  6. Verify the avatar is the custom one you set with the custom workspace avatar overlaid in the bottom right
  7. Navigate to the workspace chat > click the + icon > request money > create and request $100.00 from the workspace > land in the workspace chat
  8. Verify the report preview uses a 24x24 dual-avatar, with the rounded member avatar in the top-;eft and a rounded square workspace avatar in the bottom right
  9. Click the report preview that was added to the workspace chat
  10. Verify in the LHN the expenseReport avatar use a 24x24 dual-avatar, with the rounded member avatar in the top-left and a rounded square workspace avatar in the bottom right.
  11. On the expenseReport, click into the request to access the request view
  12. Verify the request in the LHN uses a larger 32x32 rounded member avatar, as well as a smaller 20x20 rounded square workspace avatar overlaid in the bottom right.

@0xmiros
Copy link
Contributor

0xmiros commented Jun 12, 2023

Here's video to see expense report and expense request

Account A, B should have policyExpenseChat beta permission. (In video, desktop app is A, iOS app is B)

  1. Login A and invite B to workspace
  2. Login B on another device, go to A's workspace chat and request money
  3. Click report preview
Screen.Recording.2023-06-12.at.5.09.13.PM.mov

I am not sure expense request vs expense report.
@trjExpensify can you please guide me which page is expense request, which page is expense report in video? Since I will be reviewing PR.

@trjExpensify
Copy link
Contributor

@trjExpensify can you please guide me which page is expense request, which page is expense report in video? Since I will be reviewing PR.

The expenseReport is the one with the pay button in the header seen at 0:03.
The request is the one with the details (amount, description, date) seen at 0.15.

@JmillsExpensify JmillsExpensify changed the title Feature: LHN, avatar and header updates to IOU/Expenses, workspace chats and chat rooms tied to workspaces. Feature: LHN, avatar and header updates to reports, workspace chats and chat rooms tied to workspaces. Jun 15, 2023
@grgia
Copy link
Contributor

grgia commented Jun 16, 2023

I have the avatar PR up, will start working on all of the headers next

@grgia
Copy link
Contributor

grgia commented Jun 20, 2023

Notes for titles:

  • if workspace chat
    isAdmin?
    %memberName%
    else
    %workspaceName%
  • if Expense reports
    isSettled?
    %workspaceName paid %amount%
    else
    %workspaceName% owes %amount%
  • if IOU reports
    isSettled?
    %payer% paid %amount%
    else
    %payer% owes %amount%
  • if Requests made to the workspace
    %amount% request for %merchant% | %comment%
  • if Threads in workspace rooms/reports
    %parent comment%
  • if Threads in DMs/GroupDMs
    %parent comment%
  • if Chat room
    %roomName%

@grgia
Copy link
Contributor

grgia commented Jun 20, 2023

@trjExpensify / @JmillsExpensify do we have a mockup that features the Request with header pattern %amount% request for %merchant% | %comment% ? I want to clarify what the comment is in this case

@trjExpensify
Copy link
Contributor

Nice, that makes sense to me. 👍

@grgia
Copy link
Contributor

grgia commented Jul 6, 2023

@trjExpensify @shawnborton great, that makes sense with the total in the button! Could I get a mockup for the checkmark place on web/mobile? I want to make sure I'm on the same page there

@grgia
Copy link
Contributor

grgia commented Jul 6, 2023

@shawnborton
image
Is it intentional to remove the divider line under the header only on mobile?

@shawnborton
Copy link
Contributor

Definitely not, that was probably an accident. Let's keep the divider line there.

@trjExpensify
Copy link
Contributor

This whatcha' thinking for the total yeah?

image

@shawnborton
Copy link
Contributor

Yeah I think that works!

@grgia
Copy link
Contributor

grgia commented Jul 10, 2023

OVERALL UPDATE:

I made a few PRs to get started on the headers this weekend:

Both are nearly done and just need screenshots/QA steps and I think I should be able to get most of the Expense REQUEST view (#22333) done today as well.


DROP DOWN BUTTON QUESTION:


@shawnborton - We modify the dropdown button on this page, can you confirm whether this design will be used everywhere or just on this page? (changed to medium sized button (smaller height), remove dark divider line)

before:
image

after (in mockup):
image

I have a few follow-up questions about hover and press actions, so my plan is to create an issue and tackle this button separately. Here's the issue- #22540


THREE DOTS QUESTION:


@trjExpensify - What happens when you click the three dots? We currently omit the three dots on report pages and only show on the individual transactions. Is it intentional to add it here? (The default option is delete)

Current Expense / IOU reports:

New, from the mockup:

@trjExpensify
Copy link
Contributor

@trjExpensify - What happens when you click the three dots? We currently omit the three dots on report pages and only show on the individual transactions. Is it intentional to add it here? (The default option is delete)

Oh hm, you're right. @shawnborton are we moving the pin action into that overflow menu here, yeah? I believe that was the intention, in which case we should add it as a menu item within both the expense/iouReport view and request view.

@grgia
Copy link
Contributor

grgia commented Jul 10, 2023

are we moving the pin action into that overflow menu here, yeah? I believe that was the intention, in which case we should add it as a menu item within both the expense/iouReport view and request view.

@trjExpensify why not just show the pin icon instead of the three dots?

@shawnborton
Copy link
Contributor

Yeah, the intention was to move the pin action into the overflow menu. I like keeping things consistent and training the user that on basically every type of view (chat, task, expense, report, etc) you have an overflow menu that has additional options. So even if the pin is the only option available in some cases, I do like keeping things consistent by using the overflow menu.

@grgia
Copy link
Contributor

grgia commented Jul 10, 2023

I have both PRs MVP done for these two views. @shawnborton @trjExpensify could we sync up and make sure I'm not missing any steps in these PRs? (Note: I still need to clean code up)

the intention was to move the pin action into the overflow menu

Is there a separate issue or design doc section for the pin changes?

Report
#22484

Request
#22562

@shawnborton
Copy link
Contributor

I think we missed putting this into an issue or design doc. That being said, happy to sync up and make sure we're all on the same page!

@shawnborton
Copy link
Contributor

But yeah, the idea is that we basically give every top chat header (the thing with the avatar + user/room/task/expense/report name) an overflow menu. The pin icon will live behind the overflow menu, and sometimes we'll also have a call icon, a delete icon, etc.

@grgia
Copy link
Contributor

grgia commented Jul 11, 2023

EOD Update:

Major PRs Ready For Review:
🟢 Match LHN Avatars to Header Avatars #22467
🟢 Expense Report Headers #22484
🟢 Expense Request Headers #22562

Follow-Up Issues I will be tackling separately as they affect more views: (Note, I may make some of these external)
🟡 Move Pin Logic into Context Menu - #22635
🟡 Update DropDown button style - #22540
🟡 Update Header Link Style - #22641

@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

This issue has not been updated in over 15 days. @JmillsExpensify, @trjExpensify, @s77rt, @grgia eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@trjExpensify
Copy link
Contributor

I think this should be back on weekly until we can close it out.

@trjExpensify trjExpensify added Weekly KSv2 and removed Monthly KSv2 labels Aug 7, 2023
@JmillsExpensify
Copy link
Author

Yeah, agreed. We're down to clean-up now.

@JmillsExpensify
Copy link
Author

Given that we have an issue for everything that remains from here, and related because all those are already included in the wave3 project, I'm inclined to close this larger issue out. In fact, I'm going to go ahead and do that, though please re-open if I'm missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants