mWeb/Chrome - Chat- The details page and bottom navigation menu open at the same time

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Issue found we executing PR #13204

Action Performed:

1. Go to URL https://staging.new.expensify.com/
2. Login with any account
3. Navigate to any chat conversation
4. Navigate to any chat
5. Hold and press with your finger under the user name

Expected Result:

The standard details popover is opened

Actual Result:

1. The bottom navigation menu is opened
2. The details page and bottom navigation menu open at the same time occasionally

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Mobile Web

Version Number: 1.2.36.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Bug5846018_Record_2022-12-03-01-24-00.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[s77rt]

Proposal

diff --git a/src/components/PressableWithSecondaryInteraction/index.js b/src/components/PressableWithSecondaryInteraction/index.js
index a6b21a59e1..b2d918d784 100644
--- a/src/components/PressableWithSecondaryInteraction/index.js
+++ b/src/components/PressableWithSecondaryInteraction/index.js
@@ -5,6 +5,8 @@ import {LongPressGestureHandler, State} from 'react-native-gesture-handler';
 import * as pressableWithSecondaryInteractionPropTypes from './pressableWithSecondaryInteractionPropTypes';
 import styles from '../../styles/styles';
 import hasHoverSupport from '../../libs/hasHoverSupport';
+import CONST from '../../CONST';
+import * as Browser from '../../libs/Browser';
 
 /**
  * This is a special Pressable that calls onSecondaryInteraction when LongPressed, or right-clicked.
@@ -64,20 +66,39 @@ class PressableWithSecondaryInteraction extends Component {
         const defaultPressableProps = _.omit(this.props, ['onSecondaryInteraction', 'children', 'onLongPress']);
 
         // On Web, Text does not support LongPress events thus manage inline mode with styling instead of using Text.
+
+        // On LongPress browsers fire contextmenu event triggering the secondary interaction
+        // However Safari does not, thus we use LongPressGestureHandler
+        if (Browser.getBrowser() === CONST.BROWSER.SAFARI) {
+            return (
+                <LongPressGestureHandler onHandlerStateChange={this.callSecondaryInteractionWithMappedEvent}>
+                    <Pressable
+                        style={this.props.inline && styles.dInline}
+                        onPressIn={this.props.onPressIn}
+                        onPressOut={this.props.onPressOut}
+                        onPress={this.props.onPress}
+                        ref={el => this.pressableRef = el}
+                        // eslint-disable-next-line react/jsx-props-no-spreading
+                        {...defaultPressableProps}
+                    >
+                        {this.props.children}
+                    </Pressable>
+                </LongPressGestureHandler>
+            );
+        }
+
         return (
-            <LongPressGestureHandler onHandlerStateChange={this.callSecondaryInteractionWithMappedEvent}>
-                <Pressable
-                    style={this.props.inline && styles.dInline}
-                    onPressIn={this.props.onPressIn}
-                    onPressOut={this.props.onPressOut}
-                    onPress={this.props.onPress}
-                    ref={el => this.pressableRef = el}
-                    // eslint-disable-next-line react/jsx-props-no-spreading
-                    {...defaultPressableProps}
-                >
-                    {this.props.children}
-                </Pressable>
-            </LongPressGestureHandler>
+            <Pressable
+                style={this.props.inline && styles.dInline}
+                onPressIn={this.props.onPressIn}
+                onPressOut={this.props.onPressOut}
+                onPress={this.props.onPress}
+                ref={el => this.pressableRef = el}
+                // eslint-disable-next-line react/jsx-props-no-spreading
+                {...defaultPressableProps}
+            >
+                {this.props.children}
+            </Pressable>
         );
     }
 }

Details

The issue is caused by the use of LongPressGestureHandler which is actually unnecessary as browsers indeed support long press (either by design or because we have another running code that implements it).

Long pressing an item will fire the contextmenu event which is an event that we are already listening for (to cover right-click). So there is no need to use LongPressGestureHandler

However, Safari does not seem to be firing that event on long press, so I kept LongPressGestureHandler only for Safari.

[aldo-expensify]

Thanks for the proposal @s77rt , I played with it a bit and noticed that:

• it fixes the issue with Chrome
• the bug still exists for Safari

I don't think we want this solution because it introduces code that renders differently depending on the browser, adding more complexity for little gain. I say little gain because it doesn't fix the bug completely (the bug still exists for Safari) and the bug itself is an edge case that doesn't block the user from using the app.

Note: If we remove LongPressGestureHandler for Safari too, then the long press doesn't work anymore for Safari, so I think this is not an option.

[aldo-expensify]

I wonder if we have an easy way of detecting that a modal (PersonalDetails in this case) has been opened and abort opening the ContextMenu in this case

[aldo-expensify]

When we are looking at a chat, Navigation.getActiveRoute() yields a URI like /r/6814748762881472. If we open the personal details modal, the URI changes to /details?login=11%40expensifail.com

Something that works across browsers could be to add this check:

        if (!Navigation.getActiveRoute().startsWith(`/${ROUTES.REPORT}/`)) {
            console.log('Abort, not in report anymore!');
            return;
        }

in ReportActionItem.showPopover

[melvin-bot]

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.

[aldo-expensify]

Asked for opinions on how to proceed here: https://expensify.slack.com/archives/C01GTK53T8Q/p1670294483056029

Because of the issue being an edge case (unlikely to happen) and low ROI, it may be better to just do nothing.

@muttmuure do you think we should close?

[aldo-expensify]

I'll go ahead a close considering the slack conversation.