CRITICAL: Desktop - Chat Switcher is slow and doesn't recognize keystrokes

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Type a message in a chat
2. Type command+k to open chat switcher
3. Immediately start typing the name of a person you already have chats with

Expected Result:

The characters you type show up in search

Actual Result:

Many characters are missed. ie. I typed lauren quickly and only the n showed.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.73.0
Reproducible in staging?: n/a
Reproducible in production?: n/a
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
Notes/Photos/Videos: Any additional supporting documentation

n/a

Expensify/Expensify Issue URL:
Issue reported by: @mallenexpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695286362554219

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011dba4ca10589e48c
• Upwork Job ID: 1707643423922569216
• Last Price Increase: 2023-10-17

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[melvin-bot]

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

[stephanieelliott]

Adding relevant labels

[melvin-bot]

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

[studentofcoding]

Proposal

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

Desktop - Chat Switcher is slow and doesn't' recognize keystrokes

What is the root cause of that problem?

On src/pages/SearchPage.js we have set a debounce of 75ms this.debouncedUpdateOptions = _.debounce(this.updateOptions.bind(this), 75); meanwhile we set focus to the Input (BaseOptionsSelector.js) after 300ms on CONST.ANIMATED_TRANSITION.. This condition makes the race condition, that if the user types quickly (before the the Input isFocused) then some keystrokes will be missed.

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

1. Change CONST.ANIMATED_TRANSITION to less than this.debouncedUpdateOptions time (in the demo below I set ANIMATED_TRANSITION = 50)

2. We should move the call this.debouncedUpdateOptions() inside the callback function of setState(), and add the debouncedUpdateOptions more time than CONST.ANIMATED_TRANSITION, to ensure that it'll only called after ANIMATED_TRANSITION on Search Input is done & after the state update.

onChangeText(searchValue = '') {
    this.setState({searchValue}, () => {
        this.debouncedUpdateOptions();
    });
}

Note: as ANIMATED_TRANSITION is being used on another component as well, we have to decide the final value of it.

Result

fix_search_lag.mov

What alternative solutions did you explore? (Optional)

None

@cubuspl42 @stephanieelliott

[melvin-bot]

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

[cubuspl42]

@studentofcoding

This means that if the user types quickly, some keystrokes will be missed because the state update is debounced and does not happen immediately, when onChangeText function is called

Could you elaborate? I understand why it does not happen immediately, but how does debouncing explain the missing keystrokes?

[studentofcoding]

@studentofcoding

This means that if the user types quickly, some keystrokes will be missed because the state update is debounced and does not happen immediately, when onChangeText function is called

Could you elaborate? I understand why it does not happen immediately, but how does debouncing explain the missing keystrokes?

Based on my observations the reason is because of race conditions when debouncedUpdateOptions() is called before the state is fully updated, thus causing some keystrokes to be missed @cubuspl42.

[cubuspl42]

@studentofcoding Can this race condition be observed when, for example, we add some console.log statements?

Could you provide a video with a sequence of logged events demonstrating the race? I'd like to understand better what kind of race condition we're dealing with. I believe that the solution might be working, but the root cause analysis is sometimes even more important than a fix itself.

[studentofcoding]

@studentofcoding Can this race condition be observed when, for example, we add some console.log statements?

Could you provide a video with a sequence of logged events demonstrating the race? I'd like to understand better what kind of race condition we're dealing with. I believe that the solution might be working, but the root cause analysis is sometimes even more important than a fix itself.

Sure @cubuspl42, let me add the demonstration tonight

[stephanieelliott]

Hey @studentofcoding and progress with the demonstration?

[melvin-bot]

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

[cubuspl42]

I asked for help with figuring out this one on Slack

[melvin-bot]

@cubuspl42 @stephanieelliott 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!

[mallenexpensify]

Still a lil slow, I typed lucien here then pressed enter quickly

2024-04-30_20-17-48.mp4

[mallenexpensify]

We're off hold, Fabric is live and the chat switch is still a lil slow, at least for me. In a few tests I did right now, it was just the first letter that doesn't show, if you type quickly after typing cmd+k in Desktop. @filip-solecki , any thoughts on next best steps here?

[filip-solecki]

I will try to find the root cause and hopefully also some solution in a few days

[filip-solecki]

Hi! Coming back after investigation.
The problem is in the BaseSelectionList. In the screenshot I uploaded, you can see that we deliberately delay the focus on the input. This was changed(by removing shouldDelayFocus flag) a long time ago to fix the focus problem when going back to the page with SelectionList. Basically, the only roving we can do in this case is to conditionally bypass setTimeout, in this particular case. It seems that just in this case the error mentioned above should not occur, because from the ChatSwitcher page it is not possible to go anywhere further and then go back to it.

@mallenexpensify

[mallenexpensify]

Thanks @filip-solecki . Does this affect other parts of the app too? I've been noticing that the first character often gets missed. ie. here I typed black and only lack shows.

• [HOLD for payment 2023-09-21] Web - Input field not focused on click of back button from Get assistance page #25893
  @thiagobrez , @roryabraham , since you worked on the above. Can you review the above and provide more context/feedback? Nearly every time I cmd+k to open the chat switcher and start typing, the first character (or more) don't show. I'd love to fix this if we can.

[filip-solecki]

Currently it can occur on every page with SelectionList when you start typing before animation transition is over.

[mallenexpensify]

Thanks @filip-solecki . What do you propose are the next best steps here? (I'm unfamiliar with SelectionList and def not an engineer).

[filip-solecki]

The simplest solution might be to provide an option to bypass the timeout causing missing letters on certain pages where it's possible. This is not a perfect solution since there are pages that need this timeout to prevent previously mentioned bugswhen navigating back.

Another option is to discover an alternate solution for the aforementioned bug. The current timeout is essentially a workaround. If we can find a different approach, we won't need the timeout and the related issue will be eliminated.

[mallenexpensify]

Thanks @filip-solecki. Since this is a part of #newdot-quality and you're a member of the channel, can you start a conversation there about how we want to proceed? With more 👀, we might be able to find a solution that covers all bases. Thx

[mallenexpensify]

@filip-solecki posted in #newdot-quality here. I cross posted to the SWM room to see if any devs could dive in.

Here's the post from Filip

There's an ongoing issue related to the slow Chat Switcher that has existed for quite some time now. From my investigation, it appears that this issue is caused by the BaseSelectionList code fragment(attached). Specifically, the input in the SelectionList (which is implemented in numerous places, for example, the Chat Switcher) grabs focus only after the transition ends, which takes 300 milliseconds. Consequently, this might lead to missing a few characters.

Originally, the delay was incorporated to fix a different problem associated with navigation back to a page embedding a SelectionList.

Currently, we have two potential approaches we can undertake to fix this issue:

1. Eliminate Delay in Some Pages - We could eliminate the delay on pages where backward navigation is not used. For instance, negating this delay in the Chat Switcher page would address the issue there, but there's a potential for it to recur on other pages.
2. Find Alternative Solutions - Another tactic would be to discover another fix for the navigation issue, which would allow us to eliminate the timeout altogether. This would, in turn, automatically rectify the problem of missing characters.

Indeed, the second option seems inherently superior, though it could demand an extended period of research and testing to stumble upon an alternative solution for this issue.
What are your thoughts?

[kevinksullivan]

followed up in slack

https://expensify.slack.com/archives/C05LX9D6E07/p1717540088086959?thread_ts=1716804890.419359&cid=C05LX9D6E07

It seems like we either need to find someone internal to get on this and make it Daily if we think it's truly CRITICAL, or lower the priority level so it is not CRITICAL.

[mallenexpensify]

PR is in review. Let me know if I can hellp with testing, I have a feeling regressions on this could be pretty bad.

[mallenexpensify]

@aimane-chnaif can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01bcbf7accb280d408

I reviewed the issue and PR, there are pretty much no comments on this issue but you reviewed the PR and made a couple comments, so I set the price at $250.

[aimane-chnaif]

Accepted thanks.
@mallenexpensify what do you think of #43004 (comment)?

[mallenexpensify]

Contributor+: @aimane-chnaif paid $250 via Upwork.

@aimane-chnaif , for urgent issues, they need to have the High Priority label in order to be auto-increased in price. The issue you linked to was an error that I'm addressing internally.