-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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] Group chat title is different in header than in details. #47803
Comments
Triggered auto assignment to @stephanieelliott ( |
Edited by proposal-police: This proposal was edited at 2024-08-21 16:04:15 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Group chat title is different in header than in details. What is the root cause of that problem?We display header title via DisplayNames by passing up to 5 participants App/src/pages/home/HeaderView.tsx Lines 69 to 74 in a32da40
but in the details page we get the group chat name via getGroupChatName with App/src/pages/ReportDetailsPage.tsx Lines 248 to 249 in a32da40
What changes do you think we should make in order to solve the problem?We need to be consistent in the way we get group chat name App/src/pages/ReportDetailsPage.tsx Lines 248 to 249 in a32da40
or if we want without limit applied we can stop slicing the participants here in header view App/src/pages/home/HeaderView.tsx Lines 69 to 74 in a32da40
We can also update getReportName to include the case for isDeprecatedGroupDM case handle it there and avoid the condition in details page other wise we might also need to consider isDeprecatedGroupDM case in header view as we did in report details page. What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.The header and details page differ and do not include all members of the group. What is the root cause of that problem?In the header, we get the group chat report name with App/src/pages/home/HeaderView.tsx Line 81 in 9757ae2
Line 3739 in 9757ae2
In the report detail page we get the group chat report name with App/src/pages/ReportDetailsPage.tsx Line 250 in 9757ae2
The problem here is we sort the list of participant names after we slice Line 2145 in 9757ae2
What changes do you think we should make in order to solve the problem?I think in the header we slice the group chat report name because it maybe too long and the width of report detail page is small. So in this case we should sort the participant name list before we slice the list
Lines 2139 to 2145 in 9757ae2
What alternative solutions did you explore? (Optional)If we want to display the same, we can remove the check App/src/pages/ReportDetailsPage.tsx Line 250 in 9757ae2
|
Job added to Upwork: https://www.upwork.com/jobs/~0185f6c487fb4d1046 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak ( |
We already have an issue here to make the group name consistent. |
That issue looks different since it's for a thread |
@eVoloshchak, @stephanieelliott Eep! 4 days overdue now. Issues have feelings too... |
@puneetlath Can you confirm the expected about the group chat title If the group has a list of participant names like
|
I'm not sure honestly and I don't really have a preference as long as we're consistent. What does getGroupChatName currently do? |
@puneetlath |
With this example, it's returning option 1 now. |
Ok I say we just do that in all the scenarios then. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@eVoloshchak, @stephanieelliott Still overdue 6 days?! Let's take care of this! |
Hey @eVoloshchak there are a few proposals here, can you review please? |
@eVoloshchak, @stephanieelliott Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it! |
@eVoloshchak, @stephanieelliott 12 days overdue. Walking. Toward. The. Light... |
Bump on this @eVoloshchak
|
I think we need to put this on hold #48046 |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@eVoloshchak @stephanieelliott this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
This issue has not been updated in over 14 days. @eVoloshchak, @stephanieelliott eroding to Weekly issue. |
Thanks @ahmedGaber93, let's hold for #48046 |
PR #48046 was merged to main, should be on staging soon |
I think this is fixed with #48046 🎉 I can't repro this -- anyone able to reproduce? |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
I am going to close since I am still unable to repro, if you are able to repro this feel free to reopen it! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.23-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724245884496689
Action Performed:
Expected Result:
The header and details page should include all the members of group
Actual Result:
The header and details page differ and do not include all members of the group.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Onyx Data:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @eVoloshchakThe text was updated successfully, but these errors were encountered: