[Pay Aug 18 if no regressions] Copy to clipboard - Unable to copy images or links

[isagoico]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Expected Result:

• User should be able to copy images and link URLs when using the copy to clipboard feature.
• Right-clicking or long-pressing on a link should open the ReportActionContextMenu, but the Copy to Clipboard menu item should instead say Copy URL to Clipboard. Pressing that menu item, the link url should be copied to the clipboard.
• Right-clicking or long-pressing on an image should open the ReportActionContextMenu, but the Copy to Clipboard menu should instead say Copy Image to Clipboard. Pressing that menu item, the image should be copied to the clipboard.

Actual Result:

When copying and pasting an image, the HTML coding is displayed instead of the link or image.

Action Performed:

1. Log in to e.cash and navigate to a conversation
2. Click on add attachment
3. Upload a picture
4. RIght click the message with the image
5. Click on copy to clipboard
6. Paste the image on a text input

Workaround:

No way of copying an image by using copy to clipboard.

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android ✔️
Desktop App ✔️
Mobile Web ✔️

Version Number: 1.0.60-0

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

Notes/Photos/Videos:

Expensify/Expensify Issue URL:

View all open jobs on Upwork

---

From @iwiznia https://expensify.slack.com/archives/C01GTK53T8Q/p1622579145427200

we sequester the right click button to show the actions menu, but that makes it impossible to copy a link that's in a comment or being able to copy an image pasted.

Upwork job: https://www.upwork.com/jobs/~015ed0068d5788ef5a

[MelvinBot]

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

[TomatoToaster]

Cool I think this makes sense to be done externally.

[MelvinBot]

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

[puneetlath]

Looks like this is fixed for links, but still needs to be addressed for images. Created an Upwork job for it.

[iwiznia]

Sorry, but I don't see it's fixed for any type.

[parasharrajat]

For Images, we have to use the native Modules for Mobile devices but previously we left that part intentionally. So are we ready to go that path @roryabraham?

For links, I suggest a different context menu than the current one. It would have two items
a) Copy Link
b) Open Link.

[puneetlath]

Hm, it's working for me on desktop on staging for links.

[parasharrajat]

I think that's not what the issue meant.

Try this is a message with link https://google.com Now try to copy the link only.

[dklymenk]

For all attachments (not just images) some html parsing needs to happen. To extract links from html. Here is how what I would do for images for example:

  const el = document.createElement('html');
  el.innerHTML = html;
  const imageUrl = el.querySelector('img').getAttribute('src');

imageUrl is something we'll pass to clipboard.

EDIT: Sorry, I forgot this is react native world. We can use React.createElement instead of document.createElement.
EDIT2: Actually, nvm. We'll probably need to use some other lib to parse html in react native https://www.npmjs.com/package/react-native-html-parser .

[roryabraham]

@puneetlath I updated the OP here a bit, can we please update the Upwork job to reflect that?

[puneetlath]

Hm, @roryabraham @iwiznia @parasharrajat I think a few different issues are being conflated here. The original issue reported here was that when you copy a message that contains an image, it puts the raw html on your clipboard instead of just the image. This also was happening with messages that contained links, but isn't anymore (I had already removed @isagoico's screenshot that showed it).

I think we should split up what y'all are saying into separate issues and implement it slightly differently.

1. When a message contains an image and is copied (either by hovering the message and choosing the "copy to clipboard" option or by right-clicking/long-pressing) copy the image itself. We can do this for messages that contain images since we don't support messages that have images + text. Any image input is only the image. (this issue)
2. When a url is right-clicked/long-pressed select the text of the url. This will make it so that we only copy the URL since we already support copying only selected text. This would match the behavior I expect when using a typical web page and right clicking a URL. (separate issue)

That's how I'd implement this. And I'd do it in two different issues. What do y'all think?

[iwiznia]

There's #3317 too

[roryabraham]

That's how I'd implement this. And I'd do it in two different issues. What do y'all think?

It's a good summation of the problem, but I would approach it slightly different. Those two issues could be separate, but imo they should not be. I'd prefer to see the same contributor work on both these issues (if they want to do 2 PRs, that's fine). My reasoning is that, at a very high level the solution is the same:

When the user right-clicks or long-presses an html element in a chat message, the ReportActionContextMenu should open, and the functionality of the Copy to clipboard action should be tailored to that HTML element.

When a message contains an image and is copied ... copy the image itself. We can do this for messages that contain images since we don't support messages that have images + text.

I'm not a huge fan of this implementation. It would work for now, but in order to complete 2 above, we would need to detect which type of HTML element is rightClick/longPressed to open the ReportActionContextMenu. For example, if an <a> tag is rightClick/longPressed, then we know we should display "Copy URL", and copy the href. Similarly, if an <img> tag is right-clicked, we know we should display "Copy Image", and copy the image to the clipboard.

Any image input is only the image.

Relying on this would be fine, but since we have to implement a more sophisticated solution for links anyways, let's use the same solution for images. Plus then we get the added bonus that it will be easier to support images + text + links together in one message, if we ever want to do that.

[puneetlath]

Okay I'm good with that. What do you think of what I proposed for links?

When a url is right-clicked/long-pressed select the text of the url. This will make it so that we only copy the URL since we already support copying only selected text. This would match the behavior I expect when using a typical web page and right clicking a URL.

[roryabraham]

Hmmm I'm not sure I completely understand that proposal.

we already support copying only selected text

Is this true on mobile? I don't think so...

[roryabraham]

Any new proposals here? @puneetlath Should we increase the offer for this ticket?

[Drewfergusson]

@roryabraham I was looking at this issue to possibly work it. I agree with you that we should be able to wrap the individual HTML elements ( and ) so that when a user clicks that it brings up specific action menus. We can do this in the RenderHTML file if we pull out and reuse some of the logic for positioning in the ReportActionItem controller.

The issue I was seeing was there doesn't seem to be an off-the-shelf copy-image solution that we know is going to work on all devices (ios, web, and mac). React-native docs point us to @react-native-clipboard/clipboard but if you look they don't yet support images react-native-clipboard/clipboard#6. We could make something custom of course but seems like that's its own issue.

If I can see a workable solution for images in the clipboard then I can make a proposal.

[roryabraham]

@Drewfergusson Would you be interested in working on the portion of this that targets links/URLs? If you can build a general solution to tailor the content of our custom context menu based on the html tag that was interacted with to open the menu, I think that would be a really valuable first step. I think that you're correct that the solution will involve the RenderHTML file.

Then we could move on to a follow-up that focuses on the ability to copy images, and more-or-less copies your solution used to target links in the context menu.

[Drewfergusson]

@roryabraham sounds like a plan. I just submitted a proposal on Upwork. Also if you make a separate ticket for creating an image to the clipboard solution link it and I'd like to take a look at it too.

[aliabbasmalik8]

PROPOSAL

We get the source of the image OR URL from HTML at this point and copy it to the clipboard instead of the HTML
https://github.com/Expensify/Expensify.cash/blob/c5552954fd4ebcef0874edc10feaa80456f1de35/src/pages/home/report/ReportActionContextMenu.js#L88-L90

and also show appropriate text in case of the image OR URL at this point
https://github.com/Expensify/Expensify.cash/blob/c5552954fd4ebcef0874edc10feaa80456f1de35/src/pages/home/report/ReportActionContextMenu.js#L70

Thanks

I used this package https://www.npmjs.com/package/html-react-parser to parse HTML
and HERE is output in case of IMAGE

and I think we can use image/URL preview when we have URL in the message as you can see this approach used in slack or another social platform by pasting some URL.

[roryabraham]

Thanks @aliabbasmalik8, but I think we're going to move forward with @Drewfergusson on this issue for now. @puneetlath Will you please hire @Drewfergusson on Upwork?

[puneetlath]

@Drewfergusson I don't see a proposal from you on this job https://www.upwork.com/jobs/~015ed0068d5788ef5a. Can you apply there so I can hire you? Thanks!

[Drewfergusson]

@puneetlath I submitted my proposal with the name Andrew Michael Ferguson. It was a couple of days ago so you may have to look. Not sure if this link works for you but this is the link where I can view my proposal. https://www.upwork.com/ab/proposals/1409996432714612737

[Drewfergusson]

Hey, @puneetlath @roryabraham I was working on testing the solution I came up with and ran into an issue with what I proposed.

In my proposal, I am essentially creating a reusable component to wrap some "thing" (in this case the link) with a react native Pressable component . This special Pressable then triggers a Popover. This works fine on browser, desktop, and mobile web. (PR here)

For mobile though, links at their core have a Text element with an onPress attribute. It seems that on mobile, events are not propagating out as they do in a browser normally. (or something is preventing this that I don't know of). If I remove the onPress attribute from the Text component, wrapping it with the special Pressable I created works fine. As soon as I add back the onPress attribute then wrapping does not propagate onPress nor onLongPress events. It seems like the same Text component or Pressable component will have to have to have the onLongPress and on onPress attributes. It will not be as cleanly composable as I first thought.

Still working on a nice solution on the mobile side and should have an update soon. Let me know if anything comes to mind.

[Drewfergusson]

@roryabraham @puneetlath I left a comment in the PR (link) that it's ready for review, just following up here.

[mallenexpensify]

Was curious about this issue because I often try to right-click to copy links.
Checked the PR, looks like it was held on #4194 til yesterday

[roryabraham]

Just got my first review of @Drewfergusson's PR in today, so we're making progress 👍

[puneetlath]

PR has been merged. Will be paid out on Aug 18 if no regressions.

[parasharrajat]

Just to notify you guys. Image part is still pending. May be a separate issue. Linked PR only added feature for Links

[roryabraham]

Yeah, as stated in this comment and the previous one, @Drewfergusson only worked on the portion of this issue related to copying links, but he also set up a really easily extensible framework to display different types of context menus depending on what was interacted with.

So we need to follow-up on this issue with part 2. I created a separate issue over here: #4629

[puneetlath]

Paid! Thanks for the great work @Drewfergusson.