Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.91k forks source link

[$250] iOS - Report - Conversation does not scroll down after deleting an attachment #52861

Open lanitochka17 opened 1 day ago

lanitochka17 commented 1 day ago

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: 9.0.64-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5241542 Issue reported by: Applause - Internal Team

Action Performed:

  1. Open New Expensify app Hybrid
  2. Go to the conversation with the history of the message
  3. Send an image to the conversation
  4. Delete the sent image

Expected Result:

The conversation should scroll down after deleting the attachment

Actual Result:

Conversation does not scroll down after deleting an attachment

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/a4b7bd7e-da1c-479a-8ed3-a8d85f179637

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859628438339074230
  • Upwork Job ID: 1859628438339074230
  • Last Price Increase: 2024-11-21
Issue OwnerCurrent Issue Owner: @Pujan92
melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

bernhardoj commented 1 day ago

Proposal

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

The chat doesn't scroll after deleting an attachment.

What is the root cause of that problem?

This happens because hasNewestReportAction is false. https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsView.tsx#L471

hasNewestReportAction compares the last action created with the report lastVisibleActionCreated. https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsView.tsx#L288

When we delete the attachment, lastVisibleActionCreated is updated to the previous action created. However, the last action is still the deleted attachment.

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

Currently, we have 2 places of hasNewestReportAction logic. If we look at the one in ReportActionsList, we can see that the report actions are filtered, so only visible actions will be considered the last action.

https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsList.tsx#L185-L197 https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsList.tsx#L360

So, the hasNewestReportAction in ReportActionList works correctly, but not in ReportActionsView. To fix this, we need to filter out the actions in ReportActionsView too. We can move the filtered actions from ReportActionsList https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsList.tsx#L185-L197

to ReportActionsView. Then, use it for hasNewestReportAction. https://github.com/Expensify/App/blob/c18812ae3cac59025da0f6c8657750dfcb779a65/src/pages/home/report/ReportActionsView.tsx#L288

const visibleReportActions = useMemo(
    () => reportActions.filter(...),
    [reportActions, isOffline],
);

const hasNewestReportAction = visibleReportActions.at(0)?.created === report.lastVisibleActionCreated || visibleReportActions.at(0)?.created === transactionThreadReport?.lastVisibleActionCreated;

Then, pass the filtered actions to ReportActionsList.

<ReportActionsList
    sortedVisibleReportActions={visibleReportActions}

we can consider moving the hasNewestReportAction from ReportActionsList to ReportActionsView too, so we only have 1 logic for it

melvin-bot[bot] commented 13 hours ago

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

melvin-bot[bot] commented 13 hours ago

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