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.

Sign up for GitHub

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

Deleted messages are properly gone from the UI without re-appearing briefly #10892

Closed
wants to merge 5 commits into from
Closed

Deleted messages are properly gone from the UI without re-appearing briefly #10892

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e5a386
Remove success data pendingAction reset from delete report comment
Sep 8, 2022
b989c39
Merge branch 'main' of github.com:Expensify/App into paulogasparsv-de…
Sep 8, 2022
f056583
Merge branch 'main' of github.com:Expensify/App into paulogasparsv-de…
Sep 12, 2022
733f6af
Merge branch 'main' of github.com:Expensify/App into paulogasparsv-de…
Sep 13, 2022
3a678f2
Merge branch 'main' of github.com:Expensify/App into paulogasparsv-de…
Sep 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1196,18 +1196,6 @@ function deleteReportComment(reportID, reportAction) {
},
];

const successData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
value: {
[sequenceNumber]: {
pendingAction: null,
},
},
},
];

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just rubber ducking so this change makes sense to me...

The #10780 introduced a bug where in some cases (usually if the api took some time to delete the message) if you successfully deleted a message it would re-appear for a second and then disappear again.

So by setting the pendingAction: null on success we would briefly show the message again because it is seen as "not deleted" otherwise and since it is not pendingAction === 'delete' then we would not hide it in OfflineWithFeedback?

I think maybe this change will also be fixed by the PR changes we are discussing here #10929 (comment)

I'll do some testing and report back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So after testing my branch I see that this issue is no longer present. How would you like to proceed?

Copy link
Contributor Author

@PauloGasparSv PauloGasparSv Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If your P.R. fixes it by that pendingAction check, I think we should dismiss this one and link these issues to yours:
#10799
#10753

Is that ok?

const optimisticData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
Expand All @@ -1226,7 +1214,7 @@ function deleteReportComment(reportID, reportAction) {
sequenceNumber,
reportActionID: reportAction.reportActionID,
};
API.write('DeleteComment', parameters, {optimisticData, successData, failureData});
API.write('DeleteComment', parameters, {optimisticData, failureData});
}

/**
Expand Down