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

[$250] Move the Group Chat "Group name" field out of Settings and into Report Details #40262

Closed
marcaaron opened this issue Apr 16, 2024 · 38 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Monthly KSv2 NewFeature Something to build that is a new item. Reviewing Has a PR in review

Comments

@marcaaron
Copy link
Contributor

marcaaron commented Apr 16, 2024

cc @shawnborton @JmillsExpensify

Coming from this Slack conversation

We have decided to move the "Name" edit field out of the "Details > Settings > Name" page and into the Report Details page itself with a much more stylized look.

Deliverables:

  • Update the design to this style

Image

  • Ensure that updating the name works as expected
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f69965c8e23061b7
  • Upwork Job ID: 1780048626647789568
  • Last Price Increase: 2024-04-16
  • Automatic offers:
    • c3024 | Contributor | 102713460
@marcaaron
Copy link
Contributor Author

@shawnborton do we want to go ahead and update this for Workspace Rooms too while we're in here?

We've got this right now...

Image

@marcaaron marcaaron self-assigned this Apr 16, 2024
@marcaaron marcaaron added NewFeature Something to build that is a new item. External Added to denote the issue can be worked on by a contributor labels Apr 16, 2024
Copy link

melvin-bot bot commented Apr 16, 2024

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

@melvin-bot melvin-bot bot changed the title Move the Group Chat "Group name" field out of Settings and into Report Details [$250] Move the Group Chat "Group name" field out of Settings and into Report Details Apr 16, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 16, 2024
Copy link

melvin-bot bot commented Apr 16, 2024

Triggered auto assignment to @muttmuure (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.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Apr 16, 2024
Copy link

melvin-bot bot commented Apr 16, 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 Apr 16, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 16, 2024
@nkdengineer
Copy link
Contributor

Proposal

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

Move the Group Chat "Group name" field out of Settings and into Report Details

What is the root cause of that problem?

New feature

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

  1. Move the room name input here to the report details page here
  2. Update the style to match the new design
    • Change input title from Name to Group name
    • Allow the input to have custom fonts + font size
    • Add padding bottom so the room name will have spaces compared to the Group chat quick buttons below it
  3. We might need to update the navigation structure for the group name edit field so that it will be a sub route of the report details page instead of the report settings page. So navigating to it and go back will go to the correct report details page.
    • Move this to here
    • Move this to here
    • Update here and here so it goes back to the correct REPORT_WITH_ID_DETAILS on clicking back.

I believe this will apply to the room name as well (so similar changes for workspace room name will be needed), if not, we can apply the above change only to the group name

What alternative solutions did you explore? (Optional)

NA

@Nodebrute
Copy link
Contributor

Nodebrute commented Apr 16, 2024

Proposal

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

Move the "Name" edit field out of the "Details > Settings > Name" page

What is the root cause of that problem?

Feature Request

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

  1. Move this to report details page here

  2. In ReportDetailsPage add

import * as ReportActions from '@userActions/Report';
import Text from '@components/Text';
const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(report) && !ReportUtils.isChatThread(report);
const linkedWorkspace = useMemo(() => Object.values(policies ?? {}).find((policy) => policy && policy.id === report?.policyID) ?? null, [policies, report?.policyID]);
const shouldDisableRename = useMemo(() => ReportUtils.shouldDisableRename(report, linkedWorkspace), [report, linkedWorkspace]);
  const roomNameLabel = translate(isMoneyRequestReport ? 'workspace.editor.nameInputLabel' : 'newRoomPage.roomName');
  1. We can change below code to show Group Name
    description={isGroupChat ? translate('common.name') : translate('newRoomPage.roomName')}
    to
    description={isGroupChat ? translate('groupConfirmPage.groupName') : translate('newRoomPage.roomName')}
  2. We can also add another variable to fix reportID issue
 const reportID = report?.reportID ?? '';
  1. Change Font size/style according to design. We also need to change style for disable room name. The quick action buttons in the group are not center-aligned. We should align them in the center as well, creating a gap between the group name and the quick action buttons.

  2. Navigation

     ROOM_NAME: 'Report_Details_Room_Name',
     GROUP_NAME: 'Report_Details_Group_Name',

and remove this

App/src/SCREENS.ts

Lines 174 to 175 in 90412ec

ROOM_NAME: 'Report_Settings_Room_Name',
GROUP_NAME: 'Report_Settings_Group_Name',

   REPORT_DETAILS_ROOM_NAME: {
        route: 'r/:reportID/details/room-name',
        getRoute: (reportID: string) => `r/${reportID}/details/room-name` as const,
    },
    REPORT_DETAILS_GROUP_NAME: {
        route: 'r/:reportID/details/group-name',
        getRoute: (reportID: string) => `r/${reportID}/details/group-name` as const,
    },
   [SCREENS.REPORT_DETAILS.ROOM_NAME]: {
     path: ROUTES.REPORT_DETAILS_ROOM_NAME.route,
       },
       [SCREENS.REPORT_DETAILS.GROUP_NAME]: {
           path: ROUTES.REPORT_DETAILS_GROUP_NAME.route,
          },
[SCREENS.REPORT_DETAILS.ROOM_NAME]: () => require('../../../../pages/settings/Report/RoomNamePage').default as React.ComponentType,
    [SCREENS.REPORT_DETAILS.GROUP_NAME]: () => require('../../../../pages/GroupChatNameEditPage').default as React.ComponentType,

7.BackNavigation
Change this to

                Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID));

also Change this to

 onBackButtonPress={() => Navigation.goBack(isUpdatingExistingReport ? ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID) : ROUTES.NEW_CHAT_CONFIRM)}

In updatePolicyRoomNameAndNavigate here and here change these to

    Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID));

Also change this to

onBackButtonPress={() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? ''))}

Note:If we opt to apply this solely to group names, we can bypass the steps that involve changes with room names. Don't forget to clean up the ReportSettingsPage for unused variables.Changes can be made to the above code as needed.

RESULT

Screen.Recording.2024-04-16.at.9.53.53.AM.mov

What alternative solutions did you explore? (Optional)

If we aim to combine both the group and room name screens into one, we can simplify the navigation flow by removing one screen. By consolidating the logic of both screens into a single one, we can utilize the 'isGroupChat' check to determine which API to call. If also have to remove this check so user is navigated to one screen

@karam122
Copy link

i am willing to start this task immediately if you people allow me

Copy link

melvin-bot bot commented Apr 16, 2024

📣 @karam122! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@karam122
Copy link

Contributor details
Your Expensify account email: karam.pucit@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~015bd108906659a6b4?viewMode=1

Copy link

melvin-bot bot commented Apr 16, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@karam122
Copy link

karam122 commented Apr 16, 2024 via email

@shawnborton
Copy link
Contributor

@shawnborton do we want to go ahead and update this for Workspace Rooms too while we're in here?

We've got this right now... image

Hmm yeah, I guess it would make sense to be consistent between groups and rooms. I could get down with that then, cc @Expensify/design for thoughts too.

@dubielzyk-expensify
Copy link
Contributor

Yeah would be nice to do it for both at the same time 😄

@dragnoir
Copy link
Contributor

dragnoir commented Apr 16, 2024

Proposal

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

Move the Group Chat "Group name" field out of Settings and into Report Details

What is the root cause of that problem?

New feature

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

1- add MenuItemWithTopDescription for name and workspace

as we do for description

<MenuItemWithTopDescription
shouldShowRightIcon={canEditReportDescription}
interactive={canEditReportDescription}
title={report.description}
shouldRenderAsHTML
shouldCheckActionAllowedOnPress={false}
description={translate('reportDescriptionPage.roomDescription')}
onPress={() => Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report.reportID))}
/>

We do the same for adding Room/group name and Workspace.

Also we need to remove the top part to fit the design here #40262 (comment)
image

and here for groups
image

2-remove this part of shouldShowRoomName from ReportSettingsPage

remove this part of shouldShowRoomName. This will remove the name editing from "Details > Settings > Name".

Results for rooms:
image

Results for groups:
image

3- make group/room names larger

Pass the prop titleStyle to MenuItemWithTopDescription related to group/room name, with font ExpensifyNewKansas-Medium and larger font size

Result:
image

4- make Workspace just a text under the Room name

As mentioned by the design team below, for chat rooms, we will display the workspace as simple Text under the room name.
image

@dragnoir
Copy link
Contributor

@shawnborton After we do the updates in #40256 and take the task here in consideration for rooms too #40262 (comment)

We will get a UI like this:
image

I have 2 suggestions to make the UI better:

1- move room/group name editing under the Pin/Share buttons like this:

image

2- Use menu item for room/group name editing

I think the UI will be a bit overwhelmed in suggestion 1, so I would like to suggest this one:
image

@marcaaron
Copy link
Contributor Author

Proposals look pretty 👍 so far.

I think we'd want to have a single route/screen though for both the Room and Group Chat page if possible. The underlying flows are slightly different i.e. different validation and different API calls. But ideally should be consolidated if possible.

@dragnoir great observations - will let @shawnborton respond before we proceed here.

@shawnborton
Copy link
Contributor

shawnborton commented Apr 17, 2024

Maybe we should take an approach like this to harmonize rooms and groups? cc @JmillsExpensify @Expensify/design

CleanShot 2024-04-17 at 06 07 35@2x

@dubielzyk-expensify
Copy link
Contributor

LGTM. Though I'm not sure if we need the Workspace on the room, but we can take that discussion elsewhere (also don't mind it too much)

@dragnoir
Copy link
Contributor

Proposal

updated based on the design team comment #40262 (comment)

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Apr 26, 2024
@muttmuure
Copy link
Contributor

PR in progress

@melvin-bot melvin-bot bot removed the Weekly KSv2 label May 31, 2024
Copy link

melvin-bot bot commented May 31, 2024

This issue has not been updated in over 15 days. @eVoloshchak, @dragnoir, @marcaaron, @muttmuure 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!

@melvin-bot melvin-bot bot added the Monthly KSv2 label May 31, 2024
@mallenexpensify mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Monthly KSv2 labels Jun 13, 2024
@mallenexpensify
Copy link
Contributor

PR is here #40858

Copy link

melvin-bot bot commented Jun 13, 2024

📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@c3024
Copy link
Contributor

c3024 commented Jun 13, 2024

Commenting for taking over as contributor. (Context)[https://expensify.enterprise.slack.com/archives/C02NK2DQWUX/p1718239157243779?thread_ts=1718239157.243779&cid=C02NK2DQWUX]

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jun 13, 2024
@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Jul 8, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

This issue has not been updated in over 15 days. @eVoloshchak, @dragnoir, @mallenexpensify, @marcaaron, @c3024 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!

@mallenexpensify
Copy link
Contributor

Closing, the PR was closed as well because "This has been implemented in the Details Revamp flow during the refactor of ReportDetailsPage"

@c3024 if you believe you're due compensation because you raised the PR, please comment and I'll reopen. Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Monthly KSv2 NewFeature Something to build that is a new item. Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests