[$250] Assign Task - When changing Assignee, the page lags when closing modal

[lanitochka17]

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: 1.4.81-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: N/A Email or phone of affected tester (no customers): applausetester+emilio@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Open New Expensify App
2. Sign into a valid account (HT account is better for reproduction)
3. Create a Task and then click on the IOU
4. Change the Assignee and notice the page closing.

Expected Result:

User expects the page to close "clean"

Actual Result:

The page has a slight "lag/Delay" when closing, visible for a couple of seconds.

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
• [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/2c655493-f95d-4e0a-99f7-be2a7c0d4bba

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01794355bb64abf4a9
• Upwork Job ID: 1800741657347973622
• Last Price Increase: 2024-07-10
• Automatic offers:
• allgandalf | Reviewer | 103151963
• truph01 | Contributor | 103151965

Issue Owner

Current Issue Owner: @allgandalf

[melvin-bot[bot]]

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

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

[JmillsExpensify]

I'll open it up to the community for proposals.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01794355bb64abf4a9

[melvin-bot[bot]]

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

[truph01]

Proposal

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

The page has a slight "lag/Delay" when closing, visible for a couple of seconds.

What is the root cause of that problem?

After selecting the assignee in https://github.com/Expensify/App/blob/8d11d0b185083a6a813daeb0fe7c0697a5627ef6/src/pages/tasks/TaskAssigneeSelectorModal.tsx#L182, some Onyx updates will occur that affect the JS thread. Those Onyx updates are from TaskActions.editTaskAssignee that was called here, they updates both the report and report actions which are heavy.

Right after that we goBack, so the navigation animation is affected by the Onyx updates.

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

Make use of useWaitForNavigation, we can navigate first, then handle the Onyx data update in the background, the Onyx data update is not user-visible so it's best done in the background so other user-visible things like navigation/animation is not obstructed

Navigation.dismissModal(report.reportID);

waitForNavigate(() => {
    // handle the data update here
});

The data update is this block

And do the same in the new task case

What alternative solutions did you explore? (Optional)

Run the navigation in InteractionManager.runAfterInteractions so it waits for the Onyx updates to complete before navigating

InteractionManager.runAfterInteractions(() => {
    Navigation.dismissModal(report.reportID);
});

And in here too

Another optional point to consider, in https://github.com/Expensify/App/blob/8d11d0b185083a6a813daeb0fe7c0697a5627ef6/src/pages/tasks/TaskAssigneeSelectorModal.tsx#L240, it's connecting to the full reports collection even though only 1 report from the route param is needed. We can change to connect to only that 1 report by withOnyx or useOnyx, it will also help it to be more smooth.

Some other notes:

• After the changes, after pressing the button, the option looks like it's scrolling up, I believe this issue is being fixed in https://github.com/Expensify/App/issues/41922#issuecomment-2112071705
• We can freeze the sections by storing it by usePrevious, and use it instead of sections right after updating the data (before InteractionManager.runAfterInteractions for the navigation), so the sections will be freezed and is not updated while we're watiting for navigation.

[truph01]

Proposal has been updated to add an optional change

[truph01]

Proposal updated to add alternative fix

[allgandalf]

@truph01 , Your current RCA is:

some Onyx updates will occur that affect the JS thread. Right after that we goBack, so the navigation animation is affected by the Onyx updates.

Can you please point as to what Onyx updates you are referring to? it's necessary to have a clear RCA for us to proceed with solution,

We should know why first then we can think of possible solution to a bug , thanks and looking forward to your reply

[truph01]

Can you please point as to what Onyx updates you are referring to?

@allgandalf Appreciate the feedback! I added more details to the proposal.

[allgandalf]

Chill melvin, will review the updated proposal today/tomorrow 👍

[melvin-bot[bot]]

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

[allgandalf]

@JmillsExpensify Discussing something internally

[allgandalf]

Internal discussion on this issue was completed, will review the pending proposal tomorrow 👍

[melvin-bot[bot]]

@JmillsExpensify @allgandalf 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!

[allgandalf]

sorry was AFK, will review today/tomorrow, reviewing an urgent deploy blocker today

[allgandalf]

@truph01 , your RCA is correct, and according to the expected result:

User expects the page to close "clean"

I think your solution is good to go!

@truph01 's Main Soution looks good to me here

🎀👀🎀 C+ reviewed

@roryabraham , please find the initial summary here

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@JmillsExpensify, @roryabraham, @allgandalf Whoops! This issue is 2 days overdue. Let's get this updated quick!

[allgandalf]

friendly bump @roryabraham for assignment

[melvin-bot[bot]]

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

[allgandalf]

bump to @roryabraham for assignment

[melvin-bot[bot]]

@JmillsExpensify @roryabraham @allgandalf this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

@JmillsExpensify, @roryabraham, @allgandalf Huh... This is 4 days overdue. Who can take care of this?

[allgandalf]

@roryabraham , the assignment summary is here, can you please assign when you find time :) thanks

[melvin-bot[bot]]

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

[allgandalf]

With all respect 🙇

🎀👀🎀

[melvin-bot[bot]]

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

[allgandalf]

@MonilBhavsar , you can find the summary here

[MonilBhavsar]

Proposal looks good!

it's connecting to the full reports collection even though only 1 report from the route param is needed. We can change to connect to only that 1 report by withOnyx or useOnyx, it will also help it to be more smooth.

Did we check the impact of this change. How much is it helping? And do you mean to use selector?

[truph01]

Did we check the impact of this change. How much is it helping?

@MonilBhavsar I checked and it doesn't help much with regards to the lagging issue in the OP, I thought it's nice to have.

And do you mean to use selector?

I tested using both and similar result, I lean towards using useOnyx (if we include that change) as it's the new standard.

The main solution in What changes do you think we should make in order to solve the problem? is the part that makes it look smooth and the lagging is gone.

[melvin-bot[bot]]

@JmillsExpensify, @MonilBhavsar, @allgandalf Eep! 4 days overdue now. Issues have feelings too...

[JmillsExpensify]

Working through the proposal. @MonilBhavsar thoughts on that last response?

[allgandalf]

waiting for assignment melvin...

[MonilBhavsar]

Okay, thanks for clarifying. Let's move ahead and we can update to use useOnyx

[melvin-bot[bot]]

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[truph01]

PR https://github.com/Expensify/App/pull/45691 can be reviewed

[allgandalf]

Left some comments over the PR @truph01 , thanks

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[allgandalf]

Woah, @truph01 can you verify on staging if this is still reproducible ?

[allgandalf]

friendly bump @truph01

[truph01]

@allgandalf I tested it on my end, and the bug is still reproducible, but not consistently.

[allgandalf]

how should we proceed here @MonilBhavsar ?, Qa said the bug is not reproducible and contributor says it is reproducible but not consistently

[truph01]

@allgandalf What about your end? Can you reproduce it?

[allgandalf]

bump @MonilBhavsar

[MonilBhavsar]

Sorry missed it somehow. i think we should still fix it and improve the code. I guess it will be constantly reproduced on high traffic accounts?

[allgandalf]

I guess it will be constantly reproduced on high traffic accounts?

Let me check on current staging

[allgandalf]

The issue is reproducible to me on staging, i will go forward with reviewing the PR

https://github.com/user-attachments/assets/d1fe1775-a348-4349-a8e0-9eee923c1b7d