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

Text highlighting and copy to clipboard are not working as expected #2845

Closed
isagoico opened this issue May 13, 2021 · 18 comments
Closed

Text highlighting and copy to clipboard are not working as expected #2845

isagoico opened this issue May 13, 2021 · 18 comments
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented May 13, 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:

Selected text should be displayed if the user right clicks the message

Actual Result:

Selected text is not shown after menu open. No way user can know what is going to be copied full message or selected text.

Action Performed:

  1. Navigate to a conversation
  2. Highlight 2 or more lines of text in the conversation
  3. Right click on the message and select copy in the menu

Workaround:

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

Platform:

Where is this issue occurring?

Web ✔️
iOS
Android
Desktop App
Mobile Web

Version Number: 1.0.43-0

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

Notes/Photos/Videos:

Grabando.183.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork


From @parasharrajat https://expensify.slack.com/archives/C01GTK53T8Q/p16208390261216007

When we select any text on message and press **Copy to Clipboard ** on context menu.
Selected text is not shown after menu open. No way user can know what is going to be copied full message or selected text.
You select the text and then move you cursor to different location and right click context menu will open at click location but copied text will be the selected text. In short, only copy selected text when context menu is opened over it.
You drag you cursor while clicking among messages, try to select text from multiple messages or Select the User name as well. Then Copy to Clipboard action copies everything that was selected. I think Copy to Clipboard was meant for per message only.

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

Triggered auto assignment to @sonialiap (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 13, 2021
@Viacheslav80
Copy link
Contributor

Proposal

we will replace the selection range after 5ms, it will have been cleared by then. Why it's cleared I don't know.
We modify executeSecondaryInteractionOnContextMenu(e) in
/src/components/PressableWithSecondaryInteraction/index.js

47:      e.preventDefault(); 
-        const selection = window.getSelection().toString();
+        const selection = window.getSelection();
+        const {anchorNode, anchorOffset, focusNode, focusOffset} = selection;
+        setTimeout(()=>selection.setBaseAndExtent(anchorNode, anchorOffset, focusNode, focusOffset), 5);
-        this.props.onSecondaryInteraction(e, selection);
+        this.props.onSecondaryInteraction(e, selection.toString());

@sonialiap sonialiap removed their assignment May 21, 2021
@sonialiap

This comment has been minimized.

@sonialiap sonialiap added Engineering Weekly KSv2 and removed Daily KSv2 labels May 21, 2021
@MelvinBot
Copy link

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

@sonialiap sonialiap added the Improvement Item broken or needs improvement. label May 21, 2021
@marcaaron

This comment has been minimized.

@marcaaron
Copy link
Contributor

Bump @isagoico if you have any more context. Can we create two issues for this?

@isagoico
Copy link
Author

Sorry for missing this before. I combined the issues since they were similar imo. Will separate them and post the other issue here 😊

@marcaaron marcaaron assigned marcaaron and unassigned marcaaron May 25, 2021
@isagoico
Copy link
Author

Created the other issue here #3134

@marcaaron marcaaron removed their assignment May 25, 2021
@marcaaron marcaaron added the External Added to denote the issue can be worked on by a contributor label May 25, 2021
@MelvinBot
Copy link

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

@michaelhaxhiu
Copy link
Contributor

Oopsie, I let this one slip in favor of some other project duties. Sorry, prioritizing right now.

@michaelhaxhiu michaelhaxhiu changed the title Copy to clipboard not working as expected when selecting text Copy to clipboard is not working as expected when selecting text Jun 1, 2021
@michaelhaxhiu michaelhaxhiu changed the title Copy to clipboard is not working as expected when selecting text Text highlighting and copy to clipboard are not working as expected Jun 1, 2021
@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Jun 1, 2021

Ok so I think I see two separate issues/jobs, and would like a buddy check:

  1. Highlighting more than 1 row of text doesn't work, whether it be top-to-bottom or bottom-to-top. It skips certain lines randomly.

  2. Copy to clipboard and paste changes (i) the ordering of some text and (ii) random words/text can appear like "NEW" which weren't copied

@Jag96
Copy link
Contributor

Jag96 commented Jun 1, 2021

Highlighting more than 1 row of text doesn't work, whether it be top-to-bottom or bottom-to-top. It skips certain lines randomly.

This sounds like #1341 to me so I wouldn't create another job for that.

Copy to clipboard and paste changes (i) the ordering of some text and (ii) random words/text can appear like "NEW" which weren't copied

I'm not sure that is an issue here and is kind of related to #1341 so I wouldn't file anything for it.

I think the main issue here is that if you highlight part of a message and right-click, the context menu shows and then the highlighted text is unhighlighted, and the expected behavior is that the highlighted text should remain highlighted while the context menu is open.

@parasharrajat
Copy link
Member

Yeah, @Jag96 has a point.

I think the main issue here is that if you highlight part of a message and right-click, the context menu shows and then the highlighted text is unhighlighted, and the expected behavior is that the highlighted text should remain highlighted while the context menu is open.

Also adding up to it, I think if user selects some text and open the context menu over it, only then selected text should be copied.
for eg;

My name is Rajat. E.cash is cool.

if user selects Rajat and then right click over cool then whole copy to clipboard should trigger instead of selected copy.
Make sense?

@Jag96
Copy link
Contributor

Jag96 commented Jun 1, 2021

if user selects Rajat and then right click over cool then whole copy to clipboard should trigger instead of selected copy.
Make sense?

Makes sense to me, it looks like this is how it is currently working on staging and production, but I agree this is an important note for regression testing for the PR that fixes this issue

@michaelhaxhiu
Copy link
Contributor

Ah thanks for clarifying guys, that makes sense! 😄

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Jun 2, 2021

I think the main issue here is that if you highlight part of a message and right-click, the context menu shows and then the highlighted text is unhighlighted

This isn't happening anymore on Web or Desktop, which is/was the core of my confusion! Here's a recording for reference of me trying many variants of copying-to-clipboard, and the context menu + highlighted text behaving properly:

jcnle

Text remains highlighted when the context menu is opened via right-click.

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Jun 2, 2021

My name is Rajat. E.cash is cool.

if user selects Rajat and then right click over cool then whole copy to clipboard should trigger instead of selected copy.
Make sense?

@parasharrajat hmm I am not having issues with this one on Web or Desktop. The only scenario for highlight + copy-to-clipboard that I think is weird right now is this one:

  • I highlight some text and then right-click somewhere that is outside of the highlight text rows. See that it copies the content from where I right-clicked versus what I highlighted .
  • But this is sort of an edge case to optimize for - it feels weird to right-click off your highlighted text blurb.

ringring

@michaelhaxhiu
Copy link
Contributor

I'm closing this GH for the time being because the initially reported bug is no longer existent. Glad to re-open if I'm misunderstanding / or make a new GH for what @parasharrajat brought up if it's indeed a bug.

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 Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants