-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Hold for payment 2024-02-07] [$500] MEDIUM: Fix Google Docs copy/paste of images #34306
Comments
Triggered auto assignment to @conorpendergrast ( |
Job added to Upwork: https://www.upwork.com/jobs/~0107c9e5dbbdb903ab |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.MEDIUM: Fix Google Docs copy/paste of images What is the root cause of that problem?NA - Feature Request What changes do you think we should make in order to solve the problem?When we copy an image from google, google will write the HTML content to the clipboard where it will includes source of the image copied etc. <meta charset='utf-8'>
<meta charset="utf-8"><b style="font-weight:normal;" id="docs-internal-guid-ef70d940-7fff-9d33-5271-c83bd573c118"><span
style="font-size:11pt;font-family:'Open Sans',sans-serif;color:#000000;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre;white-space:pre-wrap;"><span
style="border:none;display:inline-block;overflow:hidden;width:624px;height:451px;"><img
src="https://lh7-us.googleusercontent.com/vt_1xrWmyFRWKb9zB-uFaougvcrbPSm5hdPaKUMEOtcJuHwtwFAJ_ubht3BAFoaDn59-S5AAJcaEPE8RCnsayNNjcDjhhXZTjWO1_6lgQ-EYbKCnpqiy0Xz5DFod1mOn4CDQHFtElQd8em7G-DqilBk"
width="624" height="451" style="margin-left:0px;margin-top:0px;" /></span></span></b> We're handling the pasting of HTML here App/src/components/Composer/index.tsx Lines 226 to 241 in 8aec07c
We should be adding a new condition here which will check for the source of the
Once we have the url of the image, using App/src/components/Composer/index.tsx Lines 233 to 238 in 8aec07c
In the above block of code, we should be adding the following code. try {
// validate if the image source is a valid URL.
const url = new URL(embeddedImages[0]?.src);
// considering images from google, cause google will return the content as html
if (embeddedImages[0].src && embeddedImages[0].src.includes('googleusercontent')) {
handlePasteFileFromGoogleDoc(url.href);
return;
}
// eslint-disable-next-line no-empty
} catch {} const handlePasteFileFromGoogleDoc = useCallback(
(source: string) => {
fetch(source, {method: 'GET'}).then((response) => {
response.blob().then((blob) => {
const fileType = response.headers.get('content-type') ?? 'image/png';
const extension = fileType.split('/').pop() ?? 'png';
// here name is `pasted-file` because the URL we're fetching isn't allowing to fetch the filename due to some CORS related issues..
// if we want to fetch the file we should be creating a proxy URL where we take the image source and fetch and return via API we're using
// which will solve issue.
// Anyway I don't consider we need that cause, we're not caring for the image (or atleast I feel that way.)
const blobToFile = new File([blob], `pasted-file.${extension}`, {type: fileType});
onPasteFile(blobToFile);
});
});
},
[onPasteFile],
); The above code works for image copied through docs which are image URLs, but to manage the images copied into clipboard as base64 string also need to be handled.
Also, since we're onto sheets when I copy the cells from sheets, it would be copied as text but I see whatsapp (atleast for excel) it copy as image. To create the same effect, google sheets is sending us the HTML which we need to convert the html to into image.
Code changes.function dataURItoBlob(dataURI: string): File {
// convert base64 to raw binary data held in a string
// doesn't handle URLEncoded DataURIs - see SO answer #6850276 for code that does this
const byteString = atob(dataURI.split(',')[1]);
// separate out the mime component
const mimeString = dataURI.split(',')[0].split(':')[1].split(';')[0];
const extension = mimeString.split('/')[1];
// write the bytes of the string to an ArrayBuffer
const ab = new ArrayBuffer(byteString.length);
// create a view into the buffer
const ia = new Uint8Array(ab);
// set the bytes of the buffer to the correct values
for (let i = 0; i < byteString.length; i++) {
ia[i] = byteString.charCodeAt(i);
}
// write the ArrayBuffer to a blob, and you're done
const blob = new Blob([ab], {type: mimeString});
const file = new File([blob], `pasted-file.${extension}`, {type: mimeString});
return file;
} Handle Copied cells from google sheets const domparser = new DOMParser();
const dom = domparser.parseFromString(pastedHTML, TEXT_HTML);
const embeddedImages = dom.images;
// if pasted HTML is HTML from sheet (i.e sheet-table)
const sheetHTML = dom.getElementsByTagName('google-sheets-html-origin').item(0);
if (sheetHTML) {
document.body.appendChild(sheetHTML);
html2canvas(sheetHTML as HTMLElement).then((canvas) => {
const file = dataURItoBlob(canvas.toDataURL('image/png'));
onPasteFile(file);
});
document.body.removeChild(sheetHTML);
return;
} Handle images copied from sheets // If a HTML has image.
if (embeddedImages[0]?.src) {
const pastedFile = dataURItoBlob(embeddedImages[0].src);
onPasteFile(pastedFile);
return;
} What alternative solutions did you explore? (Optional)NA DemoKapture.2024-01-11.at.09.32.38.mp4 |
ProposalPlease re-state the problem that we are trying to solve in this issueThe ability to directly copy/paste charts and images from Google Docs, Google Sheets, Google Slides straight into the chat. What is the root cause of that problem?Missing functionality. What changes do you think we should make in order to solve the problem?
(Requires Google API key)
Check if clipboard data contains
Check if clipboard data contains base64 image source and if true perform the folowing actions:
VideosMacOS: Chrome / Safari724d150e-94ed-451c-a5b8-c9daa4fa74ac.mp4 |
@conorpendergrast, @Santhosh-Sellavel Huh... This is 4 days overdue. Who can take care of this? |
@Santhosh-Sellavel Could you take a look at the two proposals so far and confirm if we can move ahead with either of them? Thanks! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@conorpendergrast I'll try to get a hold of it by tomorrow. |
@Santhosh-Sellavel tomorrow came and went; can you give a new ETA for when you can handle this? |
Sorry for the delay, everyone. Both @b4s36t4 & @ikevin127's proposal looks good here. But @b4s36t4's proposals work well for Google Docs & Google Slides, but it doesn't work for charts copied from Google Sheets. @ikevin127 I like your proposal, looks good to me as it addresses charts case from Google Sheets case as well. |
@ikevin127's proposal looks good to me. C+ Reviewed |
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@Santhosh-Sellavel as far as I see when I copy the charts from google sheets, It is coping the string (values) from the graph. Even in the google docs it's not working perfectly.
|
To fix the graph issue, I'm working on a alternate solution which should solve the issue (but might require a package to be installed). |
@Santhosh-Sellavel PR #34971 is ready for review! 🚀 |
haha, sorry if I was too intimidating, but the PR is literal copy of my proposal. Hoping this is taken care!!!!.
|
Thank you for your contribution to this issue! |
Thanks to you both, it will go the other way around in future |
Some manual testing using the external TLDR: the link violates our Content Security Policy directive: "connect-src" because the I thought I should let the team handling this issue know about it in case they are not getting the PR notifications anymore. |
Created backend change to allow for this content-src https://github.com/Expensify/Cloudflare-Workers/pull/119 |
Hi, why did @github-project-automation move to CRITICAL, I'm curious? |
Also, I think there's more work to be done here, though it's off to an awesome start! I am able to copy/paste a chart from Google Sheet directly into NewDot, which is great. However, this didn't work:
Reopening this issue to fix this. Thanks for the great work on this so far! |
I tested out Google Slides, and that also works! So it looks like it's just Google Docs that's remaining (I've ticked off Sheets and Slides in the OP) |
Appreciate the amazing feedback! 🚀 I'm having some trouble understanding how come the Google Docs scenario mentioned here #34306 (comment) doesn't work 🧐 Note: I'm pretty sure it's not a Google Doc permission related issue as the images / objects copied to the clipboard cannot be restricted even if the Google Doc file is Restricted. We implemented 2 types of clipboard paste for this feature so far:
Since the scenario mentioned in the comment above matches the 2nd clipboard type, there shouldn't be any issue❓ MacOS: Chrome / SafariScreen.Recording.2024-02-01.at.19.31.27.mov |
Wow, amazing video! |
I can't reproduce the issue anymore, not sure what I did. Closing again. Great work!! |
Reopening to work out payment. Looks like the automation failed there, so I'll handle once the regression period is over (looks like that's 2024-02-07) |
Payouts due:
Upwork job is here. |
Requested on ND |
$500 approved for @Santhosh-Sellavel based on summary above. |
Strategy:
As a SaaS tool, we expect our customers to be using other best-of-breed SaaS products, especially Google Docs. Having a fantastic integration for data exchange with Google Docs is a way to stand out.
Problem:
At one point in the past, we were able to just directly copy/paste charts and image from Google Docs, Google Sheets, Google Slides, etc -- straight into NewDot. That doesn't work anymore.
Solution:
Add the ability to copy objects (not screenshots of objects, just the object itself) from Google Workspace into an Expensify chat, specifically:
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: