Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mWeb -Chat - Weird behavior with 3D Touch on chat screen #2705

Closed
kavimuru opened this issue May 5, 2021 · 40 comments · Fixed by #3927
Closed

mWeb -Chat - Weird behavior with 3D Touch on chat screen #2705

kavimuru opened this issue May 5, 2021 · 40 comments · Fixed by #3927
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented May 5, 2021

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


Expected Result:

A context menu should open without issues

Actual Result:

Context menu opens with blue flashing or opens with overlapping white background on mWeb iOS

Action Performed:

  1. Navigate to https://staging.expensify.cash/
  2. Login with existing account
  3. Open any chat
  4. Long press on any message

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web ✔️ iOS only on Safari

Version Number: 1.0.38-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Issue occurs inconsistently (3/5) in iOS (iPhone 12 Pro - iOS 14.4)
Expensify/Expensify Issue URL:

RPReplay_Final1620241170_2.mp4

View all open jobs on Upwork

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels May 5, 2021
@MelvinBot
Copy link

Triggered auto assignment to @laurenreidexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 5, 2021
@laurenreidexpensify
Copy link
Contributor

I can't replicate this on Android mWeb - gonna see if anyone can replicate on iOS and confirm.

@laurenreidexpensify
Copy link
Contributor

@conorpendergrast if you can repro on iOS let me know! thanks

@conorpendergrast
Copy link
Contributor

Reproduced on iOS, through Chrome and Safari.

Specifically, after I do a force-touch on a message, the clipboard icon is selected:

IMG_62C4B251B366-1

@laurenreidexpensify
Copy link
Contributor

Thanks sending for Engineering Triage, who can then apply the External label

@MelvinBot
Copy link

Triggered auto assignment to @tgolen (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@tgolen
Copy link
Contributor

tgolen commented May 6, 2021

Looks like a complete issue to me. I'll send to external

@tgolen tgolen added the External Added to denote the issue can be worked on by a contributor label May 6, 2021
@MelvinBot
Copy link

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

@JmillsExpensify
Copy link

Thanks for checking us out on Upwork here: https://www.upwork.com/jobs/~0127aabc0bc553db20.

If you are interested in working on this issue, please apply to this Upwork posting AND then post directly on this issue with a proposal with the technical explanation of the changes you will make. Somebody from Expensify will review your proposal, and if your proposal is accepted, you will be hired in Upwork for the job.

@isagoico
Copy link

While testing this PR #2320

We found that the behaviour in this issue slightly changed a bit.

Blue screen displayed when trying to edit message.

Expected Result

A context menu should show up

Actual Result

Screen becomes blue after long tap on message

Action Performed

  1. Open https://staging.expensify.cash/
  2. Login with existing account
  3. Open any chat
  4. Long tap a message of your own

Platform

iOS ✔️

Notes/images/Video

Bug5063742_RPReplay_Final1620758969.mp4

@JmillsExpensify
Copy link

JmillsExpensify commented May 12, 2021

Thanks! Still waiting on Upwork proposals.

@isagoico
Copy link

Issue reproducible during today's KI retests

@isagoico
Copy link

Issue reproducible today during KI retests

@JmillsExpensify
Copy link

I'm just coming back from vacation. I'll update the Upwork post since we're not getting any proposals.

@tgolen tgolen removed their assignment Jun 8, 2021
@tgolen tgolen added the Exported label Jun 8, 2021
@MelvinBot
Copy link

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

@tgolen
Copy link
Contributor

tgolen commented Jun 8, 2021

@marcaaron I am going to be going OOO for the next 10 days, so I reassigned this using the exported chore. Thanks for taking over for me!

@marcaaron
Copy link
Contributor

Still waiting for proposals here.

@parasharrajat
Copy link
Member

parasharrajat commented Jun 13, 2021

Proposal

ISSUE

As per my understanding, this issue is due to the fact that text is selectable on an M-Web browser. Also, IOS has 3d touch.

Solution

  1. We should make the menu Item including Icon non-selectable on the Context Menu for all platforms as I think nobody would want to select the text on the context menu.
  2. Now we should make text non-selectable for messages on Mobile devices and M-web. also for touch devices that are not web. For this, we can check if isSmallScreenWidth or canUseTouchScreen is true but we should also check the User-agent string as Few laptops could have touch screens as well on the web.

This will remove the blue screens while long press.

Disabling the text selection for tablets or Ipads is also important which will be covered by canUseTouchScreen and UA string checking.

On mobile devices, text messages will be copied via our Copy to clipboard feature.

I don't recommend disabling 3d or haptic touch which can be done via preventDefault() on touchforcechange event. This feature decides the long-press and other unique behaviors of IOS users.

Text selection can be disable using selectable=false on Text and for icons, wrapping them in Text.

@isagoico
Copy link

Issue reproducible today during KI retests.

@marcaaron
Copy link
Contributor

We should make the menu Item including Icon non-selectable on the Context Menu for all platforms as I think nobody would want to select the text on the context menu.

This seems reasonable.

Now we should make text non-selectable for messages on Mobile devices and M-web. also for touch devices that are not web. For this, we can check if isSmallScreenWidth or canUseTouchScreen is true but we should also check the User-agent string as Few laptops could have touch screens as well on the web.

Not sure I would go this far. Why should we not be able to select only some portion of text natively? Probably the behavior should be consistent on all platforms.

Another solution could be to just clear any document selection when the menu mounts.

e.g. with https://developer.mozilla.org/en-US/docs/Web/API/Selection/removeAllRanges

@parasharrajat
Copy link
Member

parasharrajat commented Jun 14, 2021

Yeah. Awesome. 👍 Didn't think of it that way. Actually, there has been discussion around making text non-selectable on mobile devices.

but yeah I agree. Makes perfect sense if the issue is only producible on M-web. Removing the selection will close the native menu.

@parasharrajat
Copy link
Member

Now we should make text non-selectable for messages on Mobile devices and M-web. also for touch devices that are not web. For this, we can check if isSmallScreenWidth or canUseTouchScreen is true but we should also check the User-agent string as Few laptops could have touch screens as well on the web.

But we only want to clear the selection for the above. Web needs selection.

@marcaaron
Copy link
Contributor

Actually, there has been discussion around making text non-selectable on mobile devices.

Sorry I don't have much context about that.

But we only want to clear the selection for the above. Web needs selection

Maybe just do it for all small screens? Anyways, not sure if it's the best idea just one idea :)

@parasharrajat
Copy link
Member

It's worth trying and I believe it will work just fine but there could be a catch I have seen that sometimes the menu does not even open which is blocked by text selection. There seems to be an open issue to that #2279

@isagoico
Copy link

Issue reproducible today during KI retests.

@marcaaron
Copy link
Contributor

@JmillsExpensify @parasharrajat is the only one who proposed a solution on this one so I think we can move forward.

@marcaaron marcaaron assigned parasharrajat and unassigned marcaaron Jun 22, 2021
@JmillsExpensify
Copy link

JmillsExpensify commented Jun 22, 2021

Oh great, thanks for the clarity! @parasharrajat you want to apply for this one on Upwork and I can hire you? https://www.upwork.com/jobs/~0127aabc0bc553db20

@parasharrajat
Copy link
Member

@JmillsExpensify Done.

@JmillsExpensify
Copy link

Hired! Thank you

@parasharrajat
Copy link
Member

parasharrajat commented Jun 23, 2021

Waiting for update on #2279. Before I move forward here.

@isagoico
Copy link

Issue reproducible during KI retests.

@parasharrajat
Copy link
Member

#2279 is fixed. PR for this one will be up as soon as possible.

@isagoico
Copy link

isagoico commented Jul 5, 2021

Issue reproducible during KI retests.

@parasharrajat
Copy link
Member

Sorry for delay here. Working on the PR today.

@isagoico
Copy link

Issue reproducible during KI retests

@JmillsExpensify
Copy link

Waiting on QA/deploy. Then'll I'll update for payment/regressions hold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants