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.

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

[HOLD for Payment 2024-09-09][$250] Mention - Mention list takes 2-3 seconds to remove after sending a message #45485

Closed
1 of 6 tasks
lanitochka17 opened this issue Jul 16, 2024 · 47 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 16, 2024

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.7-4
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/22744&group_by=cases:section_id&group_order=asc&group_id=306201
Email or phone of affected tester (no customers): shussain+andmob2@applausemail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Open a chat
  2. Mention a user by entering their username/email
  3. Send the message without selecting the user from the list

Expected Result:

After sending the message without selecting the user from the list, the mention list should be immediately removed

Actual Result:

Mention list takes 2-3 seconds to remove after sending a message

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

Bug6543739_1721125677358.WhatsApp_Video_2024-07-16_at_15.25.50.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~012c64667ec4d95daf
  • Upwork Job ID: 1813327338752177204
  • Last Price Increase: 2024-07-30
Issue OwnerCurrent Issue Owner: @ishpaul777
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 16, 2024
Copy link

melvin-bot bot commented Jul 16, 2024

Triggered auto assignment to @sakluger (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.

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@lanitochka17
Copy link
Author

@sakluger 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

@sakluger sakluger added the External Added to denote the issue can be worked on by a contributor label Jul 16, 2024
Copy link

melvin-bot bot commented Jul 16, 2024

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

@melvin-bot melvin-bot bot changed the title Mention - Mention list takes 2-3 seconds to remove after sending a message [$250] Mention - Mention list takes 2-3 seconds to remove after sending a message Jul 16, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 16, 2024
Copy link

melvin-bot bot commented Jul 16, 2024

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

@ishpaul777
Copy link
Contributor

Awaiting proposals

@sakluger
Copy link
Contributor

Still no proposals.

@melvin-bot melvin-bot bot added the Overdue label Jul 22, 2024
@ishpaul777
Copy link
Contributor

we are looking for proposals

@melvin-bot melvin-bot bot removed the Overdue label Jul 23, 2024
Copy link

melvin-bot bot commented Jul 23, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@fabOnReact
Copy link
Contributor

fabOnReact commented Jul 26, 2024

Proposal

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

The mention list is not removed immediately after sending a message. The mention list is removed after 2-3 seconds on Android.

What is the root cause of that problem?

  • The function handleSendMessage runs in a worklet and the logic is wrapped in runOnJS which calls an asynchronous callback on the UI thread. The delay is caused by the asynchronous callbacks.
  • setNativeProps is used to update immediately the text of the native Android ReactEditText (on iOS UITextView), without a delay of 2-3 seconds.

setNativeProps(animatedRef, {text: ''}); // clears native text input on the UI thread

In the video below you can see how the mention list disappears with 2-3 seconds of delay, while the RNMarkdownTextInput text disappears immediately.

Screen.Recording.2024-07-26.at.4.11.57.PM.mov

After commenting setNativeProps, the TextInput text/value and the suggestions disappear together after a delay of 2-3 seconds.

Screen.Recording.2024-07-26.at.4.08.46.PM.mov

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

The solution consists of the following steps:

  1. Passing a forwardRef from ReportActionCompose.tsx to ComposeWithSuggestions.
  2. Set the ref on the HostComponent responsible for displaying the Suggestions. From a quick investigation, I was able to hide the Suggestions by changing the style of the View HostComponent in AutoCompleteSuggestionsPortal.
  3. hiding the Suggestions using ref.current.setNativePros({{ transform: [{ scale: 0 }] }) from the ReportActionCompose.tsx function handleSendMessage

You can find here my detailed explanation of why we use setNativeProps to clear the value of the TextInput.

What alternative solutions did you explore? (Optional)

An alternative approach is useAnimatedStyle or useAnimatedProp to hide the Suggestions.

  1. Use a reanimated shared value to save the state change for hiding the suggestions. The message is sent with SendButton and the ReportActionCompose handleSendMessage function.
  2. create an animatedStyle based on the shared value.
  3. use animatedStyle in Suggestions.tsx to hide the suggestions.

I don't believe it will work, because the logic runs asynchronously on the JS thread, which causes this delay of 2-3 seconds.
We need to update immediately the native UI and skip any update in JavaScript, which can be done using setNativeProps.

https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue/#remarks

When you change the sv.value the update will happen synchronously on the UI thread. On the other hand, on the JavaScript thread the update is asynchronous. This means when you try to immediately log the value after the change it will log the previously stored value.

@melvin-bot melvin-bot bot added the Overdue label Jul 26, 2024
@fabOnReact
Copy link
Contributor

Proposal

Updated

  • added clear implementation steps for the solution
  • added information about alternative solutions

@ishpaul777
Copy link
Contributor

Thanks for your proposal @fabOnReact, it looks promising 👍, would you be able to provide a testing branch so i can test this quickly

@melvin-bot melvin-bot bot removed the Overdue label Jul 26, 2024
Copy link

melvin-bot bot commented Jul 29, 2024

@sakluger, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
@ishpaul777
Copy link
Contributor

not overdue, just waiting for test branch to test proposal solution

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 30, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jul 30, 2024

@sakluger @ishpaul777 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@sakluger sakluger added the Weekly KSv2 label Aug 14, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2024
@sakluger sakluger removed the Daily KSv2 label Aug 14, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 14, 2024
@perunt
Copy link
Contributor

perunt commented Aug 19, 2024

Hey guys, I'm back. I'll add this to my list and start working on this soon.
So, I'll take care of the suggestion box disappearing immediately

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added the Overdue label Aug 23, 2024
@ishpaul777
Copy link
Contributor

not overdue, @perunt Any updates?

@melvin-bot melvin-bot bot removed the Overdue label Aug 23, 2024
@perunt perunt mentioned this issue Aug 26, 2024
50 tasks
@perunt
Copy link
Contributor

perunt commented Aug 26, 2024

@ishpaul777 the original issue was that the suggestion didn't hide immediately. However, we later agreed that this was expected behavior. Now, the actual issue is the inconsistent behavior between platforms. Is my understanding correct?
If yes, then I have a fix for that.

@ishpaul777
Copy link
Contributor

Now, the actual issue is the inconsistent behavior between platforms.

thats correct @perunt

@perunt
Copy link
Contributor

perunt commented Aug 26, 2024

Nice! I made kind of a one-line fix for that. The problem was that the gesture handler was checking if the view was translucent, so to avoid it, I added a transparent color to that view

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Aug 26, 2024
@ishpaul777
Copy link
Contributor

Deployed 3 days ago, automation failed Can we add Awaiting payment label

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@sakluger sakluger changed the title [$250] Mention - Mention list takes 2-3 seconds to remove after sending a message [HOLD for Payment 2024-09-09][$250] Mention - Mention list takes 2-3 seconds to remove after sending a message Sep 10, 2024
@sakluger sakluger added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Sep 10, 2024
@sakluger
Copy link
Contributor

Sorry for the delay in handling payment. @ishpaul777 as far as payment goes, I could use your advice.

While I do think that you (@ishpaul777) are due payment for your review, I don't think any other payment is due here. Android should have been tested and included on the original PR, so I don't think we should pay for the follow-up PR that was created to align the behavior. What do you think?

@ishpaul777
Copy link
Contributor

Yes thats correct, btw @perunt is a agency member (Margelo, i guess) so no payments anyways.

@sakluger sakluger added Daily KSv2 and removed Weekly KSv2 labels Sep 10, 2024
@sakluger
Copy link
Contributor

Ah, good call. @ishpaul777 here is the Upwork offer for you: https://www.upwork.com/nx/wm/offer/103906016

@sakluger
Copy link
Contributor

@ishpaul777 can you let me know once you've accepted the offer?

@ishpaul777
Copy link
Contributor

thanks! I accepted the offer

@sakluger
Copy link
Contributor

Thanks! All paid.

We already have a regression test for this which is how this was caught. So I think we're fine to close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Status: No status
Development

No branches or pull requests

7 participants