[$1000] Long pressing on emoji suggestions highlights the text/attachments behind the emoji list on mweb

[kavimuru]

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

---

Action Performed:

1. First of all , open android app, go to any chat, type in :per (to get the suggestion list)
2. Now long press any of the emoji from the suggestion list and notice that the emoji gets placed on the chat box
3. Next, repeat the same steps on mweb chrome and notice that in mweb,the emoji doesn’t gets placed on the chat box instead it highlights the text or attachments (anything that is present behind the suggestion list)

Expected Result:

There should be consistency between the mweb and android/web where long pressing the emoji from the suggestion list should place the emoji on the chat box on mweb

Actual Result:

Long pressing the emoji in the emoji suggestion list selects the text/attachment (any text) behind the suggestion list on mweb chrome

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native
• [x] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [ ] MacOS / Chrome / Safari
• [ ] MacOS / Desktop

Version Number: 1.2.88-0 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:

https://user-images.githubusercontent.com/43996225/227600855-78111fc3-51ce-45a8-8a35-34ec189fb169.mp4

https://user-images.githubusercontent.com/43996225/227600896-d54af611-7285-4520-af01-bfba253134bb.mp4

Expensify/Expensify Issue URL: Issue reported by: @priya-zha Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679662560223179

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0170cd69222271989f
• Upwork Job ID: 1640771245599219712
• Last Price Increase: 2023-04-18

[MelvinBot]

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

[MelvinBot]

@garrettmknight Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[garrettmknight]

Confirmed, agree that we should make this consistent and the expected result makes sense.

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0170cd69222271989f

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[redstar504]

Proposal

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

On mobile web, long pressing on an emoji suggestion results in the report action context menu being opened after the emoji suggestion component is dismissed.

What is the root cause of that problem?

The root cause seems to be the order of the event loop. The press is being intercepted by the view below the emoji suggestions as soon as focus is lost as a result of the long press. First focus is lost, and then the press handler is firing.

It does not happen with normal presses because there is no responder for that event on the underlying view, and it's also being intercepted using onMouseDown={() => e.preventDefault()} which does not emit for a long press.

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

The easiest way I found to solve this problem is to simply defer the function call that hides the emoji suggestions until after the event loop completes. This can be very easily achieved by replacing:

https://github.com/Expensify/App/blob/f68a1994433bf93d9a747720bdb1a0f9c8c6bf10/src/pages/home/report/ReportActionCompose.js#L833

with

setTimeout(() => this.resetSelectedEmojis(), 0)

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

Chat context menu shows when we long press an emoji suggestion.

What is the root cause of that problem?

When we long press the emoji suggestion, the suggestion list will be dismissed because the composer is blurred.

https://github.com/Expensify/App/blob/3a9ce6fc59bb54d25749579be9b082c7600b235b/src/pages/home/report/ReportActionCompose.js#L831-L834

The blur is the normal behavior on web. We can verify this by long press any element, for example the send button. When we long press an element, we can catch the event by listen to contextmenu, which is what we do for the chat item, but not with the emoji suggestion.

https://github.com/Expensify/App/blob/3a9ce6fc59bb54d25749579be9b082c7600b235b/src/components/PressableWithSecondaryInteraction/index.js#L21

So, how the event is triggered even though we long press the emoji suggestion?

I don't have a reference to why it happens, but we can reproduce it with a simple html. Here is the codepen https://codepen.io/bernhardoj/pen/XWPwBJO to try it.

(video):

https://user-images.githubusercontent.com/50919443/229002990-742ffd23-8ebe-4db7-8fb0-c06b6d8a6f09.mp4

I change the console log to alert to debug it easier. The codepen is trying to replicate our chat page. The red box is the emoji suggestion which will hide when the input is blurred. Execute the same steps and notice an alert with text CONTEXT is shown but not with SUGGESTION CONTEXT. I'm guessing because the red box becomes hidden, the oncontextmenu is fired on the element behind it that receives the touch.

The purpose of contextmenu event is for the right click event on web/desktop.

https://github.com/Expensify/App/blob/3a9ce6fc59bb54d25749579be9b082c7600b235b/src/components/PressableWithSecondaryInteraction/index.js#L28-L31

On native/mWeb, we already have a long press event.

https://github.com/Expensify/App/blob/3a9ce6fc59bb54d25749579be9b082c7600b235b/src/components/PressableWithSecondaryInteraction/index.js#L59-L67

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

We can listen the contextmenu event only for desktop and web (i.e. isSmallScreenWidth is false).

1. Wrap with withWindowDimensions
2. If this.props.isSmallScreenWidth is false, don't add the listener.

Or is it better to use DeviceCapabilities.canUseTouchScreen ?

[rushatgabhane]

@bernhardoj it would be great if you can share the html on codepen :)

[rushatgabhane]

Anyway, both solutions feel like a hack to me because we don't know the exact root cause

[bernhardoj]

Added the codepen and video.

because the red box becomes hidden, the oncontextmenu is fired on the element behind it that receives the touch.

This is the root cause which you can validate with the codepen I provided.

[redstar504]

After digging into this a bit more, I have found that none of the other emoji components, or the context menu items, respond to a long press on mWeb. I think if we want to keep things consistent we should match the behavior of the other components and prevent a long press from actually closing the emoji suggestion dialog.

Long pressing on an emoji suggestion does not actually add the emoji, but just keeps the text you entered up to the point that it opened, so it does not make sense to close the menu when it happens. I have updated my proposal to reflect this solution.

[rushatgabhane]

Chrome mobile web behavior seems different in this regard to other platforms, which dismiss the long press after it has occurred for the visible element

is there some documentation for this? is this bug logged for chrome? if yes, then won't it fix itself after some chrome updates?

prevent a long press from actually closing the emoji suggestion dialog

@bernhardoj @redstar504 I think that is a hack.

[bernhardoj]

Here is my opinion on why disabling the contextmenu event for mobile web is not a "hack".

First of all, the current issue we are facing is currently the behavior of browsers (except for Safari mobile, I will explain more on this later) as I explained here https://github.com/Expensify/App/issues/16485#issuecomment-1491180218. Here is a sandbox https://codesandbox.io/s/context-menu-oyj370 to test that behavior on non-mobile web. Right click the red box and you will see the contextmenu event is triggered on the red box.

Now, why it does not happen on iOS Safari? That is because iOS Safari never fires contextmenu on long press. You can validate this too by focusing on main composer then long press on emoji picker button. On Android Chrome, the keyboard is dismissed because of the contextmenu event while on iOS Safari is not. There are several reports of iOS Safari doesn't fire the contextmenu event 1, 2, 3, 4.

If you read the 3rd source (the twitter one), they said

so you have to implement your own long-press detection and handling

which we already does with the Pressable, but the onLongPress event is overridden by the contextmenu event, so it never fires. (idk if this another browser bug or not, but imagine if both event fires) https://github.com/Expensify/App/blob/4ee21e95a38858df28e46caf54dc97e1bca9bc7a/src/components/PressableWithSecondaryInteraction/index.js#L59-L67

That's why disabling the contextmenu event on mobile web makes sense as we already handle the long press with onLongPress.

[MelvinBot]

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

[garrettmknight]

Still working on this one @rushatgabhane can you review the updated proposal and if you still think it's a hack we'll double and find another proposal?

[redstar504]

The root cause seems to be the order of the event loop. The press is being intercepted by the view below the emoji suggestions as soon as focus is lost as a result of the long press. First focus is lost, and then the press handler is firing.

It does not happen with normal presses because there is no responder for that event on the underlying view, and it's also being intercepted using onMouseDown={() => e.preventDefault()} which does not emit for a long press.

The easiest way I found to solve this problem is to simply defer the function call that hides the emoji suggestions until after the event loop completes. This can be very easily achieved by replacing:

https://github.com/Expensify/App/blob/f68a1994433bf93d9a747720bdb1a0f9c8c6bf10/src/pages/home/report/ReportActionCompose.js#L833

with

setTimeout(() => this.resetSelectedEmojis(), 0)

It may be a hack but if we can solve it in one line it might be worth saving the effort. If we're after a pure solution to this problem, the best way might be to refactor the EmojiSuggestion into using the Popover component, since it seems have more robust gesture handling.

[rushatgabhane]

@Beamanator @bernhardoj makes a good point that this is a Safari only issue https://github.com/Expensify/App/issues/16485#issuecomment-1493632391 - iOS Safari never fires contextmenu on long press

Should we implement their workaround?

[redstar504]

Aren't we trying to move away from platform/device specific conditions? It's possible for there to be touch screens that also have a mouse, and it's also possible for small screens to use a mouse (say you have your browser width narrowed to fit other windows on the screen). To completely disable the context menu for either of those could have an adverse effect.

[rushatgabhane]

@redstar504 you make a good point. I think that's a reason we don't implement any workarounds. They always bite us back in the future

[redstar504]

My solution simply changes the order so that the contextmenu event is called before the emoji suggestions box is hidden. This effectively defers the function that hides the emoji suggestions, resetSuggestedEmojis, until the end of the event loop. It's not uncommon to use setTimeout to achieve such an effect. We could use any small delay, but to illustrate the point, even a delay of 0 works.

In fact, this same concept is already implemented in the app in multiple places:

• AttachmentPicker
• Form
• UnreadIndicatorUpdater
• ReportActionCompose
• DeepLinkWrapper

[MelvinBot]

@garrettmknight @Beamanator @rushatgabhane 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!

[Beamanator]

Yeah I agree it's best to avoid platform-specific workarounds if possible - @rushatgabhane do you think this latest idea has the legs to get us where we need to go?

[ntdiary]

Proposal

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

Long pressing the emoji in the emoji suggestion can't place that emoji on the chat box.

What is the root cause of that problem?

emoji row is a focusable element, and long pressing them will trigger a focus event on android chrome web, which also means that the main composer chat box will be blurred and EmojiSuggestions will be hidden. https://github.com/Expensify/App/blob/64f4a7814e87bd7c4d085776abf5cb8a67b9e8bc/src/components/EmojiSuggestions.js#L85 https://github.com/Expensify/App/blob/64f4a7814e87bd7c4d085776abf5cb8a67b9e8bc/src/pages/home/report/ReportActionCompose.js#L833

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

We need to do:

1. add onLongPress={() => props.onSelect(index)} to the <Pressable> component, just like onPress. https://github.com/Expensify/App/blob/64f4a7814e87bd7c4d085776abf5cb8a67b9e8bc/src/components/EmojiSuggestions.js#L93
2. keep the focus of the main composer from leaving when long press.

The new version (0.19.x) of react-native-web supports onPointerDown handler, we can call e.preventDefault() directly in this handler. Before the upgrade is complete, we need to use element.addEventListener or element.onpointerdown to call e.preventDefault(), we can add listener to the View container. https://github.com/Expensify/App/blob/64f4a7814e87bd7c4d085776abf5cb8a67b9e8bc/src/components/EmojiSuggestions.js#L117

https://user-images.githubusercontent.com/8579651/230594350-2de063b8-a4b9-4eda-89ac-2f06f7d873e2.mp4

What alternative solutions did you explore? (Optional)

If we still want the emoji not to be selected until the touch is released. We need to add onPointerUp handler instead of onLongPress handler,

[MelvinBot]

@garrettmknight, @Beamanator, @rushatgabhane Whoops! This issue is 2 days overdue. Let's get this updated quick!

[garrettmknight]

@rushatgabhane mind taking a look at the last proposal?

[rushatgabhane]

on it today

[MelvinBot]

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