[HOLD for payment 2023-07-17] [$1000] In the Assign Task flow, when typing an assignee’s email, there is massive lag

[kavimuru]

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. In NewDot, press the green + button
2. Click Assign Task
3. Enter a title and click next
4. Click “Assignee”
5. Start typing a name or email
6. There is massive lag

Expected result:

When I enter a name or email in the Assignee screen, the name/email I type should appear in realtime as I type

Actual result:

The letters lag several seconds after I type them in the Assignee screen.

NOTE: There's about a 4 second lag. Seems like a scaling issue. Account with policy admin on hundreds of policies, and huge members list

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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:
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
Notes/Photos/Videos: Any additional supporting documentation

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cbd41563f4009996
• Upwork Job ID: 1666211042283540480
• Last Price Increase: 2023-06-27

[melvin-bot]

Triggered auto assignment to @sonialiap (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]

@sonialiap Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[sonialiap]

Reproducible

For me clicking on Assignee, clicking into the text field, trying to type, all of it is super laggy

[melvin-bot]

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

[melvin-bot]

Current assignee @sonialiap is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @francoisl (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Thanos30]

Proposal

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

Text Input lags when opening Assignee section on Task creation.

What is the root cause of that problem?

The root cause of the issue lies on TaskAssigneeSelectorModal.js file, and specifically on the updateOptions function.

The updateOptions function is being called on onChangeText callback of the TextInput, and it is also being called on the second useEffect hook we have. This makes it re-render more than needed times, which causes the lag we face.

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

Calling the updateOptions() function on the onChangeText callback is not necessary, because this will trigger the second useEffect Hook anyway, since the searchTerm is on the updateOptions dependency array. So we can remove it from there. Removing it solves the problem almost completely.

Additionally, while not a big fan of the setTimeout function, in this case it is essential to use it, to make sure we don't overflow the component with updateOptions() an unnecessary amount of times while the user types.

The onChangeText will look like this:

const onChangeText = (newSearchTerm = '') => {
        setSearchValue(newSearchTerm);
    };

The second useEffect hook on TaskAssigneeSelectorModal.js file will look like this:

useEffect(() => {
        Timing.start(CONST.TIMING.SEARCH_RENDER);
        Performance.markStart(CONST.TIMING.SEARCH_RENDER);

        const debounceTimer = setTimeout(updateOptions, 300);

        return () => {
            clearTimeout(debounceTimer);
            Timing.end(CONST.TIMING.SEARCH_RENDER);
            Performance.markEnd(CONST.TIMING.SEARCH_RENDER);
        };
    }, [updateOptions]);

, where we set a debounceTimer for the call which we clear on return.

This makes the component much smoother to use.

Video:

Screen.Recording.2023-06-09.at.4.32.58.PM.mov

[melvin-bot]

@francoisl, @sonialiap, @rushatgabhane Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[francoisl]

Cool, thanks for the proposal @Thanos30. Removing the duplicate call to updateOptions() makes sense to me.

I'm less convinced about the setTimeout, do you still notice a performance issue without it? Also, is it easier than using _.debounce() in this situation?

[Thanos30]

@francoisl We can still work with only removing the duplicate call to updateOptions(), I just thought there is no need to trigger the search with every user type. We could possibly use the _.debounce() method too, it's up to you wether you would like to implement a debounce search here or not, both will have the same outcome.

[francoisl]

I think we tend to prefer _.debounce() over setTimeout() in situations like this, but _.debounce also uses setTimeout under the hood, so I don't know if there's an advantage to using one over the other.

I'll test tomorrow on a workspace that has a large number of members, to see if there's a performance advantage to using setTimeOut/_.debounce.

In the meantime, @rushatgabhane do you have an opinion on this one?

[francoisl]

Ok, I ran more tests on a local domain with ~7500 accounts.

• On main:

lag_issue_main.mov

• Just removing the call to updateOptions() from onChangeText. It feels better typing keys individually, but the lag is still visible if you smash a lot of keys very quickly

lag_issue_option1.mov

• With a setTimeout() and the delay set to 100 (300 feels too long), in addition to removing the extra call to updateOptions(). This one feels the smoothest IMO

lag_issue_option2.mov

Overall I think using the combination of the timeout and removing the extra updateOptions() might be the best improvement, what do you think @rushatgabhane?

[Thanos30]

Did you try a combination of the 2? It smooths it out completely for me, and the updateOptions call on change text callback is not needed, it's an extra re-render for no reason

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.38-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix massive lag on Task Assignee Selection Modal #22095

If no regressions arise, payment will be issued on 2023-07-17. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@rushatgabhane] The PR that introduced the bug has been identified. Link to the PR:
• [@rushatgabhane] 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:
• [@rushatgabhane] 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:
• [@rushatgabhane] Determine if we should create a regression test for this bug.
• [@rushatgabhane] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Thanos30]

@francoisl HOLD for payment was on 2023-07-17. Bumping to make sure everything is okay

[francoisl]

Thanks for the reminder. I didn't see or hear about any issues with this, so let's go ahead with payments @sonialiap please.

[Thanos30]

Thank you @francoisl 🙏 Also, you probably missed it before, should I make an offer on the job after all? I didn't see the auto-assingment.

[francoisl]

Yes I think you'll need to, though I don't have access to Upwork myself to verify. @sonialiap can you assist please?

[sonialiap]

Sorry about the delay! I missed that the issue's title changed to "payment". Working on this now!

[sonialiap]

@Thanos30 offer sent for fix (+bonus) - paid ✔️
@rushatgabhane to be paid via NewDot (+bonus)

[Thanos30]

@sonialiap Thank you! Offer accepted

[sonialiap]

@rushatgabhane when you get a chance could you please work through the checklist

[rushatgabhane]

1. The PR that introduced the bug has been identified. Link to the PR: Create Task FrontEnd Changes #17992

2. 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/17992/files#r1271456240

3. 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: https://expensify.slack.com/archives/C01GTK53T8Q/p1690122436052539

4. Determine if we should create a regression test for this bug. No. It won't happen again for these pages

5. 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 No

[rushatgabhane]

Made a manual request on new dot https://staging.new.expensify.com/r/5352934870483260

[sonialiap]

I believe we can close this, feel free to reopen if you need to.

[JmillsExpensify]

@sonialiap Can you please summarize the appropriate individual payments for all parties involved in this issue? This is holding up @rushatgabhane's NewDot payments. More information on this compliance process in Slack.

[sonialiap]

Issue reporter: none
Contributor: @Thanos30 $1500
Contributor+: @rushatgabhane $1500

[JmillsExpensify]

Reviewed details for @rushatgabhane. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot.