-
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
[$500] Task - Chat is not scrolled to bottom when edit a task #38833
Comments
Triggered auto assignment to @Christinadobrzyn ( |
@Christinadobrzyn FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
We think that this bug might be related to #vip-vsp |
ProposalPlease re-state the problem that we are trying to solve in this issue.Chat is not scrolled to bottom when edit a task What is the root cause of that problem?Whenever there's any new report action in the report, we should call the App/src/libs/actions/Report.ts Line 342 in d482a25
to trigger
That's what we've been doing for IOU requests (for eg here). But for edit task flow ( What changes do you think we should make in order to solve the problem?Call We should also do it for whatever actions create new report action/message in the chat. For example: What alternative solutions did you explore? (Optional)NA |
This comment was marked as outdated.
This comment was marked as outdated.
That's not correct @bernhardoj. This is a new behavior not on production and has different RCA with #38855. |
Hum, I'm not able to reproduce this, it looks like the chat is at the bottom and the cursor is in the chat field when making edits to an assigned task. 2024-03-25_11-36-13.mp4Asking QA for some clarification on the expected result vs actual result - https://expensify.slack.com/archives/C9YU7BX5M/p1711337939301019 |
@Christinadobrzyn you need to scroll the task report up then make changes there. They expect that when there're new changes in that task report, it should scroll to bottom (just like regular chat report). |
Ah okay, I think I'm able to reproduce, I updated the OP so it's clearer how to reproduce. I think this can be external and part of vip-vsb |
Job added to Upwork: https://www.upwork.com/jobs/~01d64def74796adc96 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.After making multiple edits to a task, the chat thread isn't scrolled to the bottom. What is the root cause of that problem?We don't call App/src/libs/actions/Report.ts Lines 344 to 345 in d8361ee
If it's currently a bug, it also happens when we edit a money request. What changes do you think we should make in order to solve the problem?After edit a task or edit a money request, we should call Line 299 in d8361ee
Line 365 in d8361ee
Line 429 in d8361ee
Line 504 in d8361ee
Line 795 in d8361ee
Line 2053 in d8361ee
Line 2030 in d8361ee
Line 2014 in d8361ee
Line 1975 in d8361ee
Line 1952 in d8361ee
Line 1936 in d8361ee
Line 1913 in d8361ee
Line 3551 in d8361ee
Line 5049 in d8361ee
Line 5102 in d8361ee
What alternative solutions did you explore? (Optional)NA |
Contributor details I'm on Windows OS, Below is the video comparing both staging and the new website. Scrollbar.Problem.mp4I found this third-party cookies warning in the staging website this might be causing the issue(scrolling). |
📣 @0018akhil! 📣
|
Contributor details Try pointing staging.new.expensify.com(staging website) to new.expensify.com URL temporarily, third party cookies warning will disappear and test it (scrollbar problem). If you use Cloudflair this can be done easily. |
|
@nkdengineer Can you replicate this bug with the money request editing flow? I tried, but couldn't reproduce it. |
@fedirjh The reason is here App/src/pages/home/ReportScreen.tsx Lines 436 to 440 in 9d9757f
The transaction thread report is a chat thread and If we don't add any comment in transaction thread report, the notification of this report is hidden. When we edit the money request, the report is unfocused and then it's focused again after we submit the edit. That makes this useEffect above is triggered when we edit the money request and then the chat scrolls to bottom. I think it's another bug and we should add To replicate this bug, we can send a message in this report and edit the money request again. Screen.Recording.2024-03-27.at.13.12.19.mov |
@nkdengineer |
@0018akhil This appears to be happening in production too at https://new.expensify.com/ 2024-03-28_12-59-29.mp4 |
Hi @Christinadobrzyn . Okay So your in Mac. Can you send me the screenshot of your console(any warnings). |
@nkdengineer Thanks for confirming this. I was not able to replicate the bug with the money request edit flow. Hence, I don't think we should extend the fix to cover that flow. |
I think we should proceed with this proposal from @gijoe0295. the solution looks good to me. 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @madmax330, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@fedirjh As I mentioned above, we can replicate this bug on latest main or staging without any code change by sending a message in transaction thread report before we edit the money request. I think we should fix all of them in an issue to verify that this bug doesn't happen in the further. |
Sorry for being late here.
Whichever approach we proceed, they were already covered in my proposal. I think that finding these places are not difficult and not significant enough to count as a completely new proposal. It took more time to find the RCA than finding every places as the implementation is straightforward. Doing this would discourage contributors from investigating the issues and create precedence for tricks like extending former proposals to include "similar" cases. We have many jobs that finding all specific places to fix does not make a difference. We can always handle those in PR implementation. |
Not overdue, @madmax330 can you review the proposal here - #38833 (comment) |
📣 @fedirjh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @gijoe0295 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@Christinadobrzyn I think we're good for payment here because PR has been on production for nearly 10 days. |
Thanks @gijoe0295 - not sure why the payment auto-message didn't trigger for this. Payouts due:
Upwork job is here. @fedirjh or @gijoe0295 do we need a regression test for this? |
@Christinadobrzyn It seems like this bug was discovered using a regression test. Please review the OP for more information:
|
Ah thanks @fedirjh, if it was caught during a regression test then no payments should be made. I just requested a refund through Upwork for the payments sent earlier today. Let me know if I'm missing anything! |
@Christinadobrzyn Sorry for the confusion. My above answer was related to this question :
We don't need a regression test as we already have one listed in the OP.
This is not the case for this issue. it is not caught during the regression period. |
Oh thank you for clarifying! Okay, gotcha! Please ignore my refund requests. Closing since we don't need a regression. Let me know if I'm missing anything. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
**Version Number:**1.4.56-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: https://expensify.testrail.io/index.php?/runs/view/20318&group_by=cases:section_id&group_order=asc&group_id=291935
Email or phone of affected tester (no customers): shussain+web@applausemail.com
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Chat should always scroll to the bottom of the last message ( you should see the last message after making a change)
Actual Result:
After making multiple edits to a task, the chat thread isn't scrolled to the bottom.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6423531_1711133132795.2024-03-22_23-44-10.mp4
2024-03-26_12-57-52.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: