[HOLD App Navigation Reboot #11768] [$1000] Web - Double click on message on user details page opens group with that user if group chat with that user is the latest chat

[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. Open the app
2. Open any group (ensure that group is the latest chat)
3. Click on any all user names or profiles on top of group
4. From the list, select any one user
5. Double click on 'Message username'

Expected Result:

App should open user chat as it does on normal click and normal double click on 'Message username' while navigating the user details from that user report or while the latest chat is not a group with that user

Actual Result:

App reopens the group chat (i.e. group chat which is current latest chat for user)

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.2.90.4

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Recording.2383.mp4

double.click.issue.group.chat.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679823965622129

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011c35869503b85073
• Upwork Job ID: 1641050959484149760
• Last Price Increase: 2023-03-29

[MelvinBot]

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

[MelvinBot]

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

[slafortune]

Looks good!

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[alexxxwork]

Proposal

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

App opens wrong chat when double-click on 'message user' from user details page opened from group chat

What is the root cause of that problem?

This issue is caused by double calling Report.navigateToAndOpenReport([details.login]) here:

App/src/pages/DetailsPage.js

Line 191 in 55d6098

onPress={() => Report.navigateToAndOpenReport([details.login])}

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

We could disable menu item after pressing here:

App/src/pages/DetailsPage.js

Lines 188 to 194 in 55d6098

	<MenuItem
	title={`${this.props.translate('common.message')}${details.displayName}`}
	icon={Expensicons.ChatBubble}
	onPress={() => Report.navigateToAndOpenReport([details.login])}
	wrapperStyle={styles.breakAll}
	shouldShowRightIcon
	/>

like so:

<MenuItem
    title={`${this.props.translate('common.message')}${details.displayName}`}
    icon={Expensicons.ChatBubble}
    onPress={() => { this.setState({messageButtonDisabled:true}); Report.navigateToAndOpenReport([details.login])}}
    wrapperStyle={styles.breakAll}
    shouldShowRightIcon
    disabled={this.state.messageButtonDisabled}
/>

To do so we also need to add state to the DetailsPage component and it's better to write a function for onPress

[tienifr]

Proposal

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

App reopens the group chat (i.e. group chat which is current latest chat for user) when double clicking on 'Message username' in the group member profile.

What is the root cause of that problem?

The root cause is in this line where it triggered twice when double clicking on the MenuItem https://github.com/Expensify/App/blob/main/src/pages/DetailsPage.js#L191. This is the exact same problem that we faced before with the OptionRow in this issue.

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

We should apply the same fix as for the OptionRow, you can see the that solution in here.

To elaborate the fix:

We should update the MenuItem component so that it:

1. Disables the Pressable in its onPress callback
   By adding const [isDisabled, setIsDisabled] = useState(false); to the component and then use setIsDisabled(true); after this line in onPress
2. Saves a reference to the value returned by props.onPress here (called result).
3. Calls InteractionManager.runAfterInteraction, and in the callback to runAfterInteraction:
   Checks if the value from result is a promise. If so, wait for the promise to resolve. This is to provide a mechanism by which we can wait for business logic to complete before re-enabling the Pressable, if we want to.
   Then re-enables the Pressable by setIsDisabled(false)

More specifically, add the following code to replace this line.

let result = props.onPress(e);

if (!(result instanceof Promise)) {
    result = Promise.resolve();
}

InteractionManager.runAfterInteractions(() => {
    result.then(() => setIsDisabled(false));
});

This line

App/src/components/MenuItem.js

Line 88 in a9534b8

disabled={props.disabled}

also needs to be updated to disabled={props.disabled || isDisabled}

What alternative solutions did you explore?

• If we want to fix for more such component we can apply the same fix, the above will work for all MenuItem.
• If in any case we don't want the above prevent-double-click logic to apply to all MenuItem, we can introduce a prop to MenuItem like preventDoubleClick and only have the above logic if that prop is true

[Litande]

Proposal

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

Double click on message on user details page opens group with that user if group chat with that user is the latest chat

What is the root cause of that problem?

the second click on "message USER" cancel navigation and returns back to prev screen.

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

we can change the call on press in DetailsPage like this it will effectivly cause a single click on this button and block others.
onPress={_.once(function () { Report.navigateToAndOpenReport([details.login]) })}

What alternative solutions did you explore? (Optional)

[MelvinBot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[alexxxwork]

We should apply the same fix as for the OptionRow, you can see the that solution in here.

@tienifr thanks for the reference! Right now DetailsPage is a class component. So we should either leave it as a class and use constructor or make it a function and use useState callback.

[ShogunFire]

Proposal

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

When we double click on a profile name the screen stays the same instead of going to the chat of the user

What is the root cause of that problem?

We use a custom action for navigation from a drawer.

In this method

The first time we click, it pushes the new route to the stack.

On the second click it sees that we are on the same screen so it doesn't push the new route and return here

App/src/libs/Navigation/DeprecatedCustomActions.js

Line 91 in 55d6098

return DrawerActions.closeDrawer();

The problem is that before we return we pop everything from the stack so we return to the screen before the first click here:

App/src/libs/Navigation/DeprecatedCustomActions.js

Line 82 in 55d6098

navigateBackToRootDrawer();

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

We can do the check if we are on the same screen before popping the stack. That way if we are on the same screen we don't pop the stack and stay on the same screen.

Since changing DeprecatedCustomActions is not an option we can put the check before calling pushDrawerRoute

What alternative solutions did you explore? (Optional)

[ShogunFire]

Can someone clarify why modifying DeprecatedCustomActions.js should not be accepted ? It looks like it is there that there is a problem

[alexxxwork]

Can someone clarify why modifying DeprecatedCustomActions.js should not be accepted ? It looks like it is there that there is a problem

@ShogunFire as far as I know it's due to upcoming navigation system rebuild

[tienifr]

@tienifr thanks for the reference! Right now DetailsPage is a class component. So we should either leave it as a class and use constructor or make it a function and use useState callback.

Thanks @alexxxwork for the feedback, in my proposal I'm not touching the DetailsPage component, I'm only modifying the MenuItem component and used useState there.

[alexxxwork]

Thanks @alexxxwork for the feedback, in my proposal I'm not touching the DetailsPage component, I'm only modifying the MenuItem component and used useState there.

@tienifr sorry, missed it. So you are making the changes global? I don't think it's a universal approach to disable MenuItem after pressing it. How do you propose to turn it back when there's no promise in result?

[alexxxwork]

Instead there better be a prop to the component similar to shouldDelayFocus - shouldDelayPressing that allows to disable onPress for 300ms or so.

[tienifr]

@alexxxwork it was the same way we're doing with the OptionRow, you can read more details there if curious about the approach 😁

[ShogunFire]

Can someone clarify why modifying DeprecatedCustomActions.js should not be accepted ? It looks like it is there that there is a problem

@ShogunFire as far as I know it's due to upcoming navigation system rebuild

Then I think we should hold this issue until the new navigation system rebuild

[thesahindia]

I couldn't repro this issue. Is there anything else that I should be doing?

[MelvinBot]

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

[thesahindia]

It's on hold!

[MelvinBot]

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

[thesahindia]

Same #16557 (comment)

[slafortune]

Changing to weekly 👍

[slafortune]

Still on hold for #11768

[slafortune]

Still on hold

[slafortune]

Still on hold

[slafortune]

still on hold

[slafortune]

Testing this since #11768 has been closed - seems to operate as expected now. Double-clicking open chat for the person selected only.