App: [HOLD for payment 2024-04-09] [$500] Weird pop up when hit `esc` key immediately, after clicking `uparrow` to edit the last message

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: Reproducible in staging?: needs reproduction Reproducible in production?: needs reproduction 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: @iwiznia Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1710538564054589

Action Performed:

Open any chat
hit up arrow key to edit the last message
Immediately hit esckey

Expected Result:

Edit action should cancel and no pop up to edit the message

Actual Result:

Edit box reappears after a second NOTE: This is happening occasionally

Workaround:

unknown

Platforms:

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

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~019e21e5668d684c41
Upwork Job ID: 1770530705295687680
Last Price Increase: 2024-03-20
Automatic offers:

ntdiary | Reviewer | 0

About this issue

Original URL
State: closed
Created 3 months ago
Comments: 40 (18 by maintainers)

Commits related to this issue

Really non-performant comment editing — committed to Expensify/App by yuwenmemon 3 years ago

Most upvoted comments

Paid!

slafortune on Apr 10, 2024

@ntdiary Can you please complete the checklist and then accept the offer sent - https://www.upwork.com/nx/wm/offer/101529844

@slafortune, thank you! Have just accepted it. 😄

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@ntdiary] The PR that introduced the bug has been identified. Link to the PR: #2320

[@ntdiary] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/2320#discussion_r1559411676

[@ntdiary] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

[@ntdiary] Determine if we should create a regression test for this bug. yeah

[@ntdiary] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. As below

[@slafortune] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Regression steps:

Login with an account
Go to any chat and send a message (chat 1)
Go to another chat (chat 2)
Go back to chat 1
Press up (to start editing last message) and escape (to stop editing last message) quickly
Make sure the edit composer input won’t appear after a second for no reason.

ntdiary on Apr 10, 2024

@roitman-g provided good reproduction steps, and I also think it’s fine to put the cancel in the unmount phase.

Also, I’d like to add that the reason the debounced function isn’t canceled here is, when we switch back and forth between reports, multiple instances of the same report can exist.

So, even though the top-level editing component captures the key press event and executes its own debouncedSaveDraft.cancel, but the hidden editing instances below fail to execute their cancel function. 😂

🎀 👀 🎀 C+ reviewed

ntdiary on Mar 22, 2024

under review. 😃

ntdiary on Mar 22, 2024

Proposal

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

When clicking up arrow key (to edit the last message) and escape (to cancel editing the last message), the popup to edit the message appears back again after a second. Issue is easy to reproduce this way - go to some chat (chat 1), go to another chat (chat 2), return back to the exact previous chat 1, after this the issue is constant and not occasional. And if you keep repeating it long enough sometimes the popup is not closable at all.

What is the root cause of that problem?

The debounced function that saves the draft is not being cancelled when the component (ReportActionItemMessageEdit) unmounts.

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

Instead of cancelling the debounced function when deleting the draft, it is a better way to cancel it when the component unmounts - this solves all the issues I stated in re-state of the proposal section and everything works smoothly.

What alternative solutions did you explore? (Optional)

N/A

roitman-g on Mar 21, 2024

Yep, here you go:

https://github.com/Expensify/App/assets/521248/bffa0b23-dcf4-4864-b306-8d5ca219813c

iwiznia on Mar 22, 2024