[HOLD for payment 2023-07-21] [$1000] Long Pressing on reply opens the context menu for the ReportPreview action.

[kavimuru]

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 a DM with any user.
2. Send a Money request.
3. Long Press on the reply 1 on the preview of money request in the DM.
4. Observe the behaviour.

Expected Result:

It should not open the context menu on the IOU Preview like other platforms.

Actual Result:

It opens the context menu on Android chrome but other platforms does not open it.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.30-5
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.2023-06-15.at.6.37.45.PM.mov

az_recorder_20230621_223902.1.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0164ddcb49f1eae0da
• Upwork Job ID: 1672146114797383680
• Last Price Increase: 2023-07-07

[melvin-bot]

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

[hungvu193]

Proposal

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

Long Pressing on reply opens the context menu for the ReportPreview action.

What is the root cause of that problem?

Problem came from here:

App/src/pages/home/report/ReportActionItemThread.js

Lines 42 to 48 in e6c4b4e

	<PressableWithoutFeedback
	onPress={() => {
	Report.navigateToAndOpenChildReport(props.childReportID);
	}}
	accessibilityRole="button"
	accessibilityLabel={`${props.numberOfReplies} ${replyText}`}
	>

We didn't implement the onLongPress for PressableWithoutFeedback yet, so when we long pressed, it will open the ContextMenu.

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

We should add the onLongPress={() => {}} here.

            <PressableWithoutFeedback
                onLongPress={() => {}}
                onPress={() => {
                    Report.navigateToAndOpenChildReport(props.childReportID);
                }}
                accessibilityRole="button"
                accessibilityLabel={`${props.numberOfReplies} ${replyText}`}
            >

What alternative solutions did you explore? (Optional)

Add onLongPress default props our GenericPressable.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0164ddcb49f1eae0da

[melvin-bot]

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

[melvin-bot]

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

[ShogunFire]

I don't think this is a bug, it acts this way for any comment and long pressing on mobile is the equivalent of right clicking on other platforms which also open the context menu. @eVoloshchak am I missing something ?

[bfitzexpensify]

@ShogunFire is right, I didn't realise that was the way to right-click on an Android. The behaviour is the same, and I don't think it opening the context menu the same way as right-clicking elsewhere on the message does is unexpected. I agree this isn't a bug and I'm going to close this out.

[parasharrajat]

Sorry, I didn't understand this comment @bfitzexpensify. Can you please rephrase? Right-click and long-press are two different gestures. We can also have a long press on the web if we want.

long pressing on mobile is the equivalent of right-clicking on other platforms

This is wrong. This is merely a conjecture.

I disagree with @ShogunFire. Can you right-click on Android or iOS? No, Right. So long pressing on the Android and iOS does not open the context menu on the reply. But is opening on Mobile web Chrome which is the bug. Did you really test it on Android or iOS and is it opening the context menu for you there?

There are five platforms that we mainly focus on.

1. Android
2. iOS
3. Desktop
4. web
5. Android Chrome / iOS Safari

long-pressing does not apply to Desktops and Web. Similarly, Right-click does not apply to the rest of the list.

Because Android mobile web has the same gesture as Android and iOS. It should behave the same which is this bug.

@bfitzexpensify Let me know if you need more clarification here.

[bfitzexpensify]

Right-click and long-press are two different gestures. We can also have a long press on the web if we want.

Huh, I didn't know this! I thought they were the same thing. Appreciate the clarification. Given that, I agree that this is in fact a bug. Let's reopen.

[ShogunFire]

Proposal

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

On android and ios the context menu doesn't show when we long press a reportpreview action

What is the root cause of that problem?

For android and ios we don't handle the long press event here:

App/src/pages/home/report/ReportActionItemThread.js

Lines 42 to 48 in e6c4b4e

	<PressableWithoutFeedback
	onPress={() => {
	Report.navigateToAndOpenChildReport(props.childReportID);
	}}
	accessibilityRole="button"
	accessibilityLabel={`${props.numberOfReplies} ${replyText}`}
	>

So when we long press nothing happens because this Pressable doesn't propagate the longPress event

The reason the context menu still shows when we long press on mobile web is because here:

App/src/components/PressableWithSecondaryInteraction/index.js

Line 27 in 1e308ca

this.pressableRef.addEventListener('contextmenu', this.executeSecondaryInteractionOnContextMenu);

We add a listener on the contextmenu event and this contextmenu event fires on long press on mobile web. For web it is called when we right click. For ios and android it is never called.

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

Ia haven't found a solution to propagate the long press event from one pressable to his parent pressable so I think what we can do is pass a new prop onSecondaryInteraction to ReportActionItemThread like this:
onSecondaryInteraction ={showPopover}
Then in ReportActionItemThread replace the PressableWithoutFeedback by a PressableWithSecondaryInteraction and pass it the function like this
onSecondaryInteraction ={props.onSecondaryInteraction }

Result:

2023-07-02.15-45-59.mp4

What alternative solutions did you explore? (Optional)

1. If we don't want to show the contextmenu on long press on mobile web we have to remove the listener on contextmenu in PressableWithoutFeedback if we are on touchscreen

2. If we want we can also change the condition here
   

   App/src/pages/home/report/ReportActionItem.js

   Line 453 in 1e308ca

   preventDefaultContextMenu={!props.draftMessage && !hasErrors}

   
   to prevent the contextMenu when the item has a thread and we are on touchscreen but this will prevent the context menu on the whole item and not just the "x replies" line.

[bfitzexpensify]

@eVoloshchak proposal from @ShogunFire ready for review when you get a chance

[ShogunFire]

@eVoloshchak To be honest I haven't tried my proposals, I think we should know exactly what is the expected behaviour because I am not sure the behaviour on android and ios was intentional.

[melvin-bot]

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

[eVoloshchak]

@ShogunFire, expected result - is the same behavior on all platforms (i.e. it should show context menu on long press, as it does on mWeb Chrome)

If we want to show the contextmenu when we long press on android and ios what we could do is add a boolean props to BaseGenericPressable that says that we should propagate the longclick event, because right now it doesn't propagate the event if we don't have a prop onLongPress (we could also pass an empty onLongPress but that's a little weird)

Could you elaborate on that more please? Why is working fine on mWeb Chrome?

[ShogunFire]

Proposal

Updated

[ShogunFire]

@eVoloshchak

Could you elaborate on that more please? Why is working fine on mWeb Chrome?

On mWeb chrome it doesn't use the long press event to execute the secondaryinteraction it uses the context menu event that is fired by browsers.

And sorry my first proposal was wrong I haven't found a way to propagate the long press event between pressable

[melvin-bot]

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

[melvin-bot]

📣 @ShogunFire You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @parasharrajat Please request via NewDot manual requests for the Reporter role ($250)

[ShogunFire]

Ready for review

[melvin-bot]

🎯 ⚡️ Woah @eVoloshchak / @ShogunFire, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @ShogunFire got assigned: 2023-07-11 17:53:58 Z
• when the PR got merged: 2023-07-12 23:26:54 UTC

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.40-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Long press was not working when clicking on "x replies" on android/ios #22670

If no regressions arise, payment will be issued on 2023-07-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@eVoloshchak] The PR that introduced the bug has been identified. Link to the PR:
• [@eVoloshchak] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@eVoloshchak] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@eVoloshchak] Determine if we should create a regression test for this bug.
• [@eVoloshchak] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[ShogunFire]

Hello @kavimuru I have accepted the job on upwork, can you make the final paiement ? Thanks

[bfitzexpensify]

Payment made. @parasharrajat and @eVoloshchak are both receiving payments via New Dot. @eVoloshchak a reminder to complete the BZ checklist when you get a chance

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: while this was technically caused by Add ReportActionItemThread and Thread Replies UI  #18274 (the PR in which thread component was created), it just surfaced the limitation of Pressable, not caused the bug in the first place

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: this is a fairly simple bug, not sure if an additional discussion is needed here

• Determine if we should create a regression test for this bug
  Is it easy to test for this bug? Yes
  Is the bug related to an important user flow? No
  Is it an impactful bug? No
  The bug has a very low impact, doesn't break the usage of the app, I don't think we should add a regression test

[parasharrajat]

Requested reporting bonus 250.

[JmillsExpensify]

@bfitzexpensify Can you please summarize the appropriate individual payments for all parties involved in this issue? This is holding up @eVoloshchak's and @parasharrajat's NewDot payments. More information on this compliance process in Slack.

[bfitzexpensify]

Sure, @JmillsExpensify these are the payments due:

@parasharrajat - $250 reporting payment
@eVoloshchak - $1500 for C+ and #urgency bonus
@ShogunFire - $1500 for contributor and #urgency bonus (paid via upwork ✅)

[bfitzexpensify]

Agreed with your reasoning re: not needing a regression test @eVoloshchak. We're good to close this out - thanks for the work everyone!

[JmillsExpensify]

Reviewed details for @parasharrajat and @eVoloshchak. This is accurate based on summary from Business Reviewer and approved for payment in NewDot.