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

[HOLD for payment 2024-10-11] Add the workspace name to the title of the workspace chats #49624

Closed
davidcardoza opened this issue Sep 24, 2024 · 36 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@davidcardoza
Copy link
Contributor

davidcardoza commented Sep 24, 2024

Background

We’ve been discussing making the workspace name appear first in all workspace rooms to improve efficiency when identifying which company we are communicating with. The current process requires multiple clicks (#admins > #admins details > members) to see the domain of a new lead, which is inefficient. More background can be found in this Slack conversation - https://expensify.slack.com/archives/C07HPDRELLD/p1726677008605379

Design
The plan is to adda workspace name to the front of the workspca rooms, for example, "My Workspace • #admins". This will allow the workspace name to be visible without needing to click through multiple steps.

Example:
image

Issue OwnerCurrent Issue Owner: @puneetlath
@davidcardoza davidcardoza added External Added to denote the issue can be worked on by a contributor Daily KSv2 Design labels Sep 24, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 24, 2024
Copy link

melvin-bot bot commented Sep 24, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

Copy link

melvin-bot bot commented Sep 24, 2024

Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.

@nyomanjyotisa
Copy link
Contributor

nyomanjyotisa commented Sep 24, 2024

Edited by proposal-police: This proposal was edited at 2024-09-24 02:22:51 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add the workspace name to the title of the workspace chats

What is the root cause of that problem?

Changes request

What changes do you think we should make in order to solve the problem?

Add the following code here to add workspace name to the report title if the chatType is policyRoom policyAnnounce policyAdmins or domainAll

    if (isUserCreatedPolicyRoom(report) || isDefaultRoom(report)){
        formattedName = report?.policyName + " • " +  report?.reportName;
    }

Additionally we can return empty string on getChatRoomSubtitle function here if we don't need to display any subtitle on the Room screen for report with chatType equals to policyRoom policyAnnounce policyAdmins or domainAll

What alternative solutions did you explore? (Optional)

@nkdengineer
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

New feature request

What changes do you think we should make in order to solve the problem?

  1. We need to add a new condition here to add the workspace name in the title base on chatType.
    if (isUserCreatedPolicyRoom(report) || isDefaultRoom(report)) {
        formattedName = `${report?.policyName ?? ''} • ${report?.reportName ?? ''}`;
    }

  1. As I see in the design, we will not show the subtitle as the workspace name so we need to add early return empty string here
    if (isChatThread(report) || isUserCreatedPolicyRoom(report) || isDefaultRoom(report)) {
        return '';
    }

if (isChatThread(report)) {

  1. We also don't need the additional text in HeaderView here
    if (shouldShowSubtitle() || isPersonalExpenseChat || !policyName || !isEmptyObject(parentNavigationSubtitleData) || isSelfDM || ReportUtils.isUserCreatedPolicyRoom(report) || ReportUtils.isDefaultRoom(report)) {
            return null;
    }

const renderAdditionalText = () => {

What alternative solutions did you explore? (Optional)

@mollfpr
Copy link
Contributor

mollfpr commented Sep 27, 2024

@nkdengineer, your first step does not seem to be working well. It got overridden from the next condition.

App/src/libs/ReportUtils.ts

Lines 3850 to 3852 in 05ca35c

if (isChatRoom(report) || isTaskReport(report)) {
formattedName = report?.reportName;
}


@nyomanjyotisa Almost looks good. There are a couple of things that we need to work on:

  1. There's still additional text in the header view
  2. Looking at the design, there's no workspace name in the welcome text
    Screenshot 2024-09-27 at 19 15 28
  3. The workspace name is undefined when creating a room offline
    Screenshot 2024-09-27 at 19 13 57

I think we can go with @nyomanjyotisa proposal and fix the above issue while working on the PR.

🎀 👀 🎀 C+ reviewed!

Copy link

melvin-bot bot commented Sep 27, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 27, 2024
@nkdengineer
Copy link
Contributor

@mollfpr your first step does not seem to be working well. It got overridden from the next condition.

@mollfpr I see @nyomanjyotisa's proposal also uses the same condition as mine. Why did you only inform me?
And nyomanjyotisa's proposal does not cover all cases

@mollfpr
Copy link
Contributor

mollfpr commented Sep 30, 2024

I see @nyomanjyotisa's proposal also uses the same condition as mine. Why did you only inform me?

Their latest update proposal is come first from your proposal. Yours 03:11 UTC and theirs 02:22 UTC, I don't see anything wrong with that.

And nyomanjyotisa's proposal does not cover all cases

Screenshot 2024-09-30 at 11 24 47

You're correct, but the result from your proposal wasn't close to the design either. So I don't have anything to asses.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Sep 30, 2024
@dubielzyk-expensify
Copy link
Contributor

dubielzyk-expensify commented Oct 1, 2024

Default workspace names or long workspace names will clip the room name on mobile phones:

CleanShot 2024-10-01 at 20 13 04@2x

cc @Expensify/design

@dannymcclain
Copy link
Contributor

Default workspace names or long workspace names will clip the room name on mobile phones

I brought this up more than once and advocated for putting the room name first, but it didn't seem like anyone outside the design team was concerned about this inevitability... Definitely feel free to bring it up again in Slack! Or we can let this play out and let people experience the issue for themselves.

@dubielzyk-expensify
Copy link
Contributor

Yeah, I did the same. Let's push through then and see if this becomes a more widespread problem or not 👍

@davidcardoza
Copy link
Contributor Author

Side note: I’d love to see the room name as a header in the LHN, with the chat preview as a subheader. I feel like this would keep the LHN way more organized and give more space for the chat preview, especially when the room name is too long.

Example:

Workspace123 - #admins
What up, this my admins chat

Workspace456 - #admins
Do you like peanut butter on your expenses?

I think a similar idea was discussed before in a thread somewhere, but nothing came of it.

@trjExpensify
Copy link
Contributor

Default workspace names or long workspace names will clip the room name on mobile phones:

CleanShot 2024-10-01 at 20 13 04@2x

cc @Expensify/design

Dang, that's really not great IMO. To further illustrate the point, add a bunch of messages in those chats and you won't be able to see the name at all anywhere. Super disorientating.

@JmillsExpensify
Copy link

Woof, I'm in the same spot. I think this creates more problems than it solves.

@mountiny
Copy link
Contributor

mountiny commented Oct 2, 2024

I agree that we should ensure the room name is visible at the header at all times.

@trjExpensify
Copy link
Contributor

There's a thread here.

@JmillsExpensify
Copy link

Maybe we should create a new discussion thread and also revert this change until we have a holistic solution?

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 4, 2024
@melvin-bot melvin-bot bot changed the title Add the workspace name to the title of the workspace chats [HOLD for payment 2024-10-11] Add the workspace name to the title of the workspace chats Oct 4, 2024
Copy link

melvin-bot bot commented Oct 4, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 4, 2024
Copy link

melvin-bot bot commented Oct 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.44-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-10-11. 🎊

For reference, here are some details about the assignees on this issue:

  • @mollfpr requires payment through NewDot Manual Requests
  • @nyomanjyotisa requires payment (Needs manual offer from BZ)

@mountiny
Copy link
Contributor

mountiny commented Oct 7, 2024

We ended up reverting this PR, but I think its fair we will pay the contributors on this one as it was product decision down the line

@mountiny mountiny added the NewFeature Something to build that is a new item. label Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

Triggered auto assignment to @puneetlath (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Oct 7, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Oct 7, 2024

Auto-assign attempt failed, all eligible assignees are OOO.

@mountiny
Copy link
Contributor

mountiny commented Oct 7, 2024

$250 to @mollfpr and @nyomanjyotisa

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 11, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

Payment Summary

Upwork Job

BugZero Checklist (@puneetlath)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

@puneetlath, @mollfpr, @mountiny, @nyomanjyotisa, @dubielzyk-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@puneetlath
Copy link
Contributor

@nyomanjyotisa can you link me your upwork profile?

@melvin-bot melvin-bot bot removed the Overdue label Oct 14, 2024
@puneetlath
Copy link
Contributor

puneetlath commented Oct 14, 2024

Payment summary:

Thanks everyone!

@nyomanjyotisa
Copy link
Contributor

@puneetlath
Copy link
Contributor

@nyomanjyotisa offer is here. Please ping me on this issue when you've accepted.

https://www.upwork.com/nx/wm/offer/104447593

@nyomanjyotisa
Copy link
Contributor

@puneetlath offer accepted

@puneetlath
Copy link
Contributor

@nyomanjyotisa has been paid. @mollfpr please request on NewDot. Thanks y'all!

@garrettmknight
Copy link
Contributor

$250 approved for @mollfpr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests