-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Workspace rooms details page display user list in place of room name #23416
Comments
Triggered auto assignment to @maddylewis ( |
Bug0 Triage Checklist (Main S/O)
|
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
|
Triggered auto assignment to @cristipaval ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace rooms details page display user list in place of room name What is the root cause of that problem?This is caused by - #23255 For showing full title we are using App/src/pages/ReportDetailsPage.js Lines 152 to 159 in b7d17e0
For calculating that we are using !isPolicyExpenseChat(report) && !isChatRoom(report) && !isChatThread(report) Before this we used - What changes do you think we should make in order to solve the problem?We shall change - App/src/pages/ReportDetailsPage.js Line 158 in b7d17e0
to - or we can change - App/src/pages/ReportDetailsPage.js Line 64 in b7d17e0
to -
Both are same thing What alternative solutions did you explore? (Optional)We can also change it to - I can make a PR right away to fix this. |
ProposalPlease re-state the problem that we are trying to solve in this issue.On clicking policy room header, the right hand panel shows usernames at room name What is the root cause of that problem?App/src/components/DisplayNames/index.js Lines 88 to 90 in cd851d0
Here in DIsplayNames we show reportName or participants details with tooltips depending on the boolean shouldUseFullTitle It should be true in cases of isChatRoom which is the case here. What changes do you think we should make in order to solve the problem?Change this here App/src/pages/ReportDetailsPage.js Line 64 in cd851d0
to const shouldUseFullTitle = !shouldDisableSettings; What alternative solutions did you explore? (Optional) |
This PR is open to resolve the regression from #22997. cc - @cristipaval @maddylewis |
I just requested to CP the fix |
The fix for this here was deployed yesterday so I think the blocker is resolved here. |
@arosiclair quick ques - at this point, should i have a C+ added to this issue to review the existing proposals? |
This issue was already fixed by this PR. The bug was a regression from a contributor trying to fix another issue. The same contributor and C+ (@ginsuma and @robertKozik) pushed out this fix so, in this case, I think there's no extra payment necessary. That sounds right to you @maddylewis? |
Hi @arosiclair, @maddylewis, If this issue helped to change that PR for fix, will it be eligible for reporting bouns? |
Not in this case. The issue was already known before you reported it. (PR was posted Jul 21 and you posted on slack Jul 22) |
@arosiclair - got it! thanks for the breakdown. are we now waiting for the 7-day regression period to pass before closing this one out? |
Don't think that's needed either we can just use the main issue #22997. Closing this out in favor of that one |
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:
Expected Result:
App should display room name below room profile on room details page
Actual Result:
App displays user list instead of room name below room profile on room details page
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.44-0
Reproducible in staging?: y
Reproducible in production?: n
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
Notes/Photos/Videos: Any additional supporting documentation
user.list.in.place.of.room.name.mp4
Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690038296569129
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: