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

[$250] Noticeable Delay When Selecting Assignee During Task Assignment #44443

Open
5 of 6 tasks
izarutskaya opened this issue Jun 26, 2024 · 30 comments
Open
5 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@izarutskaya
Copy link

izarutskaya commented Jun 26, 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: v9.0.2-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): abebemiherat@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. FAB > Assign Task
  2. Add title > Go to Assignee
  3. Click on Assignee > Select User

Expected Result:

There should be no noticeable delay, and the process should be smooth when selecting an assignee from the dropdown during task assignment

Actual Result:

Noticeable delay when selecting an assignee

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

Bug6525043_1719393634471.Screen_Recording_2024-06-25_at_11.30.08_PM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0179306ad6d57e7abf
  • Upwork Job ID: 1805923850414823822
  • Last Price Increase: 2024-07-10
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

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

Copy link

melvin-bot bot commented Jun 26, 2024

Triggered auto assignment to @mountiny (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Jun 26, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@izarutskaya
Copy link
Author

@MitchExpensify 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.

@izarutskaya
Copy link
Author

We think this issue might be related to the #vip-vsb

@mountiny mountiny added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

Job added to Upwork: https://www.upwork.com/jobs/~0179306ad6d57e7abf

@melvin-bot melvin-bot bot changed the title Noticeable Delay When Selecting Assignee During Task Assignment [$250] Noticeable Delay When Selecting Assignee During Task Assignment Jun 26, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 26, 2024
Copy link

melvin-bot bot commented Jun 26, 2024

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

@melvin-bot melvin-bot bot added the Daily KSv2 label Jun 26, 2024
@mountiny
Copy link
Contributor

I barely saw som delay in the video, demoting and marking external

@nyomanjyotisa
Copy link
Contributor

Proposal

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

Delay When Selecting Assignee During Task Assignment

What is the root cause of that problem?

const debouncedOnSelectRow = useCallback(lodashDebounce(onSelectRow, 200), [onSelectRow]);

This line of code causes a 200 millis delay when selecting an assignee during task assignment

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

change that code to the following code since we need to keep the onSelectRow not called more than once in 200 millis

    const lastExecutedRef = useRef(0);
    const debouncedOnSelectRow = useCallback((item: TItem) => {
        const now = Date.now();
    
        if (now - lastExecutedRef.current >= 200) {
          onSelectRow(item);
          lastExecutedRef.current = now;
        }
    }, [onSelectRow]);

Result (no delay on selecting assignee and not going back twice on multiple mouse clicks on select Priority mode):
https://github.com/Expensify/App/assets/73281575/60b796d8-87d4-4efa-92d4-124393933fef

What alternative solutions did you explore? (Optional)

N/A

@mountiny
Copy link
Contributor

@huzaifa-99 @parasharrajat this seems to be coming from your PR then. can you have a look please

@parasharrajat
Copy link
Member

Yes, looks like it @mountiny. IMO, I see no delay and I thought it is fine on the original PR. @huzaifa-99 any idea on enhancing this?

IMO, this issue should still be counted as an enhancement, not a regression.

@huzaifa-99
Copy link
Contributor

I didn't see any noticeable delays during tests, and IMO this is not a regression.

@huzaifa-99 any idea on enhancing this?

will look into it shortly today.

@mountiny
Copy link
Contributor

Yeah I am not 100% sure either, the animation is a bit clunky so it might be actually from that

Copy link

melvin-bot bot commented Jul 1, 2024

@mananjadhav, @mountiny, @MitchExpensify Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Jul 1, 2024
@MitchExpensify
Copy link
Contributor

What is the latest here @mananjadhav ?

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

melvin-bot bot commented Jul 3, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jul 4, 2024
@mountiny
Copy link
Contributor

mountiny commented Jul 5, 2024

@MitchExpensify I cannot really reproduce, can you? Do you want to close it if you cannot?

@melvin-bot melvin-bot bot removed the Overdue label Jul 5, 2024
@nyomanjyotisa
Copy link
Contributor

btw I still can reproduce this issue on staging, and also it can be reproduced on prod now

it's 0.2 second delay tho

@mountiny
Copy link
Contributor

mountiny commented Jul 5, 2024

it's 0.2 second delay tho

Yeah I think this is the main problem, its small enough that its hard to notice. I am not sure if we have to do anything about this if its to help with double selection/ openinig

@mananjadhav
Copy link
Collaborator

@mountiny Are we then prioritizing this? Can you please confirm?

@mountiny
Copy link
Contributor

mountiny commented Jul 8, 2024

@mananjadhav Can you review the proposal? I think that if we can:

  • remove the delay
  • and ensure we cannot trigger the navigation/ action upon click twice

we should implement this cc @huzaifa-99 @parasharrajat what do you think of the proposal?

@parasharrajat
Copy link
Member

We experimented with leading and trailing edge debouncing and there were some issues with leading-edge debouncing then we added a wait time of 200ms to make sure logic worked which is showing some delay. Here is the explanation #25604 (comment) which warrants a 200ms delay.

we need to think of a different approach to solve both problems.

@huzaifa-99
Copy link
Contributor

we should implement this cc @huzaifa-99 @parasharrajat what do you think of the proposal?

The leading/trailing debounce is a bit messy in some cases

we need to think of a different approach to solve both problems.

I second this

@mananjadhav
Copy link
Collaborator

Read through the reasoning, yeah doesn't make sense removing the delay. Let's find an alternative solution.

@mountiny
Copy link
Contributor

I have shared this with SWM as it seems like there are no good proposals for this now

Copy link

melvin-bot bot commented Jul 10, 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 10, 2024

@mananjadhav @mountiny @MitchExpensify 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!

@SzymczakJ
Copy link
Contributor

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

@melvin-bot melvin-bot bot added Overdue and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Jul 12, 2024
@mountiny
Copy link
Contributor

@SzymczakJ excited for your investigation

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 12, 2024
@SzymczakJ
Copy link
Contributor

Work in progress, experimenting with some other solutions but they don't fix the issue yet 😬

@melvin-bot melvin-bot bot removed the Overdue label Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
Development

No branches or pull requests

8 participants