[HOLD for payment 2023-09-06] [$1000] Android - Clicking on different option quickly opens many tabs

[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. Click at your avatar
2. Click on different option quickly

Expected Result:

The app should opens only the first clicked option

Actual Result:

The app opens all the clicked options

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.47-2
Reproducible in staging?: y
Reproducible in production?: y
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

Screen_Recording_20230728-231420.mp4

Screen_Recording_20230729_132217_New.Expensify.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016e037ea65271ad6b
• Upwork Job ID: 1686148949072994304
• Last Price Increase: 2023-07-31
• Automatic offers:
• tienifr | Contributor | 26033117
• mejed-alkoutaini | Reporter | 26033119

[melvin-bot]

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

[ahmdshrif]

Proposal

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

click on two options quickly in the settings, two pages open simultaneously due to multiple navigation actions being fired

What is the root cause of that problem?

two navigation actions are triggered in quick succession, and we don't have a mechanism in place to prevent multiple navigations from happening simultaneously.

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

Instead of directly calling the navigation function as shown in the code snippet below:

App/src/pages/settings/InitialSettingsPage.js

Lines 187 to 189 in eb02006

	action: () => {
	Navigation.navigate(ROUTES.SETTINGS_SHARE_CODE);
	},

We will create a new debounced navigation function that ensures the navigation is not called multiple times within a short period. This will prevent rapid and unintended navigation.

constructor(props) {
      this.navigate = _.debounce(this.navigate.bind(this), 300);
...
}  

navigate(route) {
    Navigation.navigate(route);
}      

...
 action: () => {
    // add to all actions
    thsi.navigate(ROUTES.SETTINGS_SHARE_CODE);
}        

What alternative solutions did you explore? (Optional)

we can introduce a disableOptionForNavigation flag to prevent the navigation function from being called again while the previous navigation is still being processed. This flag should be set to true when the first navigation is triggered and reset to false when the setting focus change (if blurred mean navigation is complete )

componentDidUpdate(prevProps) {
// if focus change we need to reset flage
    if (this.props.isFocused !== prevProps.isFocused && this.disableOptionForNavigation ) {
        this.disableOptionForNavigation = false;
    }
}

const NavigateWithDisable  (route ) {
  if (this.disableOptionForNavigation){
      return;
  }
  Navigation.navigate(route);
  this.this.disableOptionForNavigation = true;
}

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016e037ea65271ad6b

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

Was able to repro on Daniel Silva's phone, I wasn't able to via BrowserStack. Seems like an External issue...

[tienifr]

Proposal

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

The app opens all the clicked options

What is the root cause of that problem?

We're not preventing users from selecting other options when the first option's navigation is in progress.

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

This issue has happened before, but for the PressableWithFeedback, let's look at how it's handled here.

This problem is the same, just that it does not apply to one single element, but a menu, where we only want 1 execution to complete before another one, so I propose that we address them with a general solution.

1. We can create a hook useSingleExecution, which will have similar logic to here, like follow

const useSingleExecution = () => {
    const [isExecuting, setIsExecuting] = useState(false);

    const singleExecution = useCallback((action) => () => {
        if (isExecuting) {
            return;
        }

        setIsExecuting(true);

        const execution = action();
        InteractionManager.runAfterInteractions(() => {
            if (!(execution instanceof Promise)) {
                setIsExecuting(false);
                return;
            }
            execution.finally(() => {
                setIsExecuting(false);
            });
        });
    }, [isExecuting]);

    return { isExecuting, singleExecution }
}

What this will do is, with any action passed in, it will only allow 1 such action to occur at a time. This is the same approach we're already using for the PressableWithFeedback, but now we're applying it to more places with a general solution

2. Then in here, we can add:
   const { isExecuting, singleExecution } = useSingleExecution();
   and pass to the MenuItem

disabled={isExecuting}
onPress={singleExecution(item.action)}

With this, only 1 MenuItem clicks will happen at a time, and the rest of the MenuItem will be disabled when one is executing
3. Since Navigation.navigate is currently not a promise, we can add a hook useWaitForNavigate to convert it to a promise, to be able to use with the above.
4. (Optional) We might want to choose a different style (background color, text color) to associate with executing state of the MenuItem, rather than using the same disabled style
5. The same useSingleExecution hook can be used in PressableWithFeedback, the code there will be much cleaner

What alternative solutions did you explore? (Optional)

Other places that use MenuItem could have the issue, we can fix them using the same clean approach as above, there'll only be a few lines of code change in the main component, the heavy lifting is already abstracted well in the hook.

[muxriddinmuqimov77]

Hey @kbecciv @mananjadhav @mallenexpensify.
I told this issue a long time ago (Here is the link: https://expensify.slack.com/archives/C049HHMV9SM/p1688707995939759),
but @kbecciv left and said "Not valid bud, device is OOS". It's been happening second time (Maybe more than 2 times) with me so far and I think it's not fair. I have so many bug reports that is left as couldn't reproduce. But These days I'm seeing some of these bug reports on issues list and added to Upwork (Other bug reporter as reporter role) that was already reported by me but left as some reasons. Is this fair?

[mananjadhav]

I investigated this further. I generally would want to avoid _.debounce because it is as good as setTimeout.

I think @tienifr's approach of useSingleExecution would work. I am not too concerned about changing the styles in this issue but I would let @mallenexpensify @youssef-lr take a stab at it.

🎀 👀 🎀 C+ reviewed.

[melvin-bot]

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

[melvin-bot]

@mananjadhav, @youssef-lr, @mallenexpensify Huh... This is 4 days overdue. Who can take care of this?

[youssef-lr]

I like @tienifr's solution as well!

[melvin-bot]

📣 @mananjadhav Please request via NewDot manual requests for the Reviewer role ($1000)

[muxriddinmuqimov77]

@mallenexpensify I didn't get paid yet.

[mananjadhav]

@mallenexpensify This couldn't be pinpointed to a specific PR. This was an lag in the App we fixed by adding a throttling/single thread execution.

Considering this is prevalent more on Android only. I don't think we need regression test steps.

[JmillsExpensify]

$1,000 approved for payment via NewDot based on BZ summary.

[youssef-lr]

Not overdue.

[mallenexpensify]

@muxriddinmuqimov77 I assumed you were the below, is that not correct?

[muxriddinmuqimov77]

Not it is not me. 2023 sen 11, Dush, 11:35 Matt Allen ***@***.***>:

…

@muxriddinmuqimov77 <https://github.com/muxriddinmuqimov77> I assumed you were the below, is that not correct? [image: image] <https://user-images.githubusercontent.com/22508304/266928104-0c24a871-2564-4714-bc3f-b0d631f7ddc9.png> — Reply to this email directly, view it on GitHub <#23867 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AYFDM5LCJTQE563OXNRD6DDXZ2WKJANCNFSM6AAAAAA24T6HYI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mallenexpensify]

Requested refund from the contributor I screenshotted above
@muxriddinmuqimov77 can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~016e037ea65271ad6b

Can you please provide a screenshot of your profile with name/avatar so I can be sure to pay the correct person. Thx

[muxriddinmuqimov77]

Requested refund from the contributor I screenshotted above @muxriddinmuqimov77 can you please accept the job and reply here once you have? https://www.upwork.com/jobs/~016e037ea65271ad6b

Can you please provide a screenshot of your profile with name/avatar so I can be sure to pay the correct person. Thx

Sure. Here it's.

When you submit an offer to the job, I'll accept it. Thanks

[muxriddinmuqimov77]

Hey @mallenexpensify. Any updates?

[mallenexpensify]

@muxriddinmuqimov77 hired, can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~016e037ea65271ad6b
In the future, it's easier/better/faster if you apply via the job link when it's first posted (if you're not auto-hired via API) then for us to extend the job to you then have you accept then we have to pay.

[mallenexpensify]

@mejed-alkoutaini I figured out the issue!
This issue was originally created with you as the bug reported. It was then updated to @muxriddinmuqimov77 because they reported the bug before you

vs

That said... @muxriddinmuqimov77 's bug reported wasn't acted up so, if you hadn't reported the bug again, we wouldn't have fixed it. Therefore, I think it's fair to compensate for reporting the bug, you don't need to approve the refund I requested.

reminder to self, payment owed is $250 for @muxriddinmuqimov77 for reporting, I think I hired them for $1000 by mistake.

[muxriddinmuqimov77]

@mallenexpensify Will you send updated offer or update the amount after I accept the offer?

[mallenexpensify]

New offer sent for $250 @muxriddinmuqimov77 !

[muxriddinmuqimov77]

@mallenexpensify Accepted. Thanks!

[mejed-alkoutaini]

@mallenexpensify I approved the refund 2 days ago, can you create a new milestone for it ?

[melvin-bot]

@mananjadhav, @youssef-lr, @mallenexpensify, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

Final payment recap
Issue reporter: @mejed-alkoutaini paid $250 via Upwork (reasoning above)
Issue reporter: @muxriddinmuqimov77 paid $250 via Upwork
Contributor: @tienifr paid $1000 via Upwork
Contributor+: @mananjadhav is due or paid $1000 via NewDot (I'm uncertain which, @JmillsExpensify approved the payment last week)

@mejed-alkoutaini , I issued you a bonus on the original contract so it should show up for you, thanks for the patience.

Finally.... @mananjadhav , can you please fill out the BZ checklist above from above so we can get this issue closed?

[mananjadhav]

@mallenexpensify Please check my comment here. Let me know if you need further information.

[mallenexpensify]

Thanks @mananjadhav , closing