[FIX] Added plain text pasting and fixed issues with image pasting. #169
Conversation
- Add check for image content before processing paste events - Prevent default paste behavior only when handling images - Insert images at cursor position instead of document start - Add plain text paste handler to strip formatting - Clean up image paste logic with better type checking and error handling
charlwillia6
requested review from
nang-dev,
Fryingpannn and
a team
as code owners
|
@Himanshu-Singh-Chauhan Please test |
…sues-with-pasting
…hen a user starts typing after pasting.
|
tested, works. awesome work. 🔥 |
|
|
||
| // Only log and process if we actually have an image | ||
| if (hasImageItem) { | ||
| console.log("Pasting image"); |
There was a problem hiding this comment.
This was already in the code. I only moved it to inside this logic. @Fryingpannn do you want me to delete it?
Screen.Recording.2024-11-17.at.5.03.33.PM.movcopy pasting over existing text not working |
….com/charlwillia6/pearai-submodule into charlwillia6/fix-issues-with-pasting
|
copy pasting over existing text not working should be fixed now |
…sues-with-pasting
Works for me! Excellent job @Himanshu-Singh-Chauhan! |
…sues-with-pasting
This has been resolved by @Himanshu-Singh-Chauhan |
|
I can't reproduce the error in the description ): are u able to repro it? if so what are the steps? |
I don't think it is platform dependent. And it is an error that doesn't happen all the time. The change I made is for a more aggressive check for what the user is trying to paste. Before, the code was not properly checking whether what was being pasted was an image or text. Sometimes it would think it was an image when it was really text, and sometimes it didn't register screenshots properly as images, so it wouldn't even add them to the input box. If I imported them as a file from path, it would work fine. I think two things are happening. Sometimes, when you add an image to the chat input box, after that it would think everything was an image that you were trying to paste. So, it would fail until you reloaded the webviews or sometimes it would reset if you created a new session. Second, it didn't always properly know if what you were pasting was an image or text. My changes just do a better job if checking what is being pasted, and makes sure that doesn't treat it as an image if it isn't one. |
|
Hmm I don't think I can merge this until I can reproduce an error we currently have and see this fixes it. I'm not sure if fully necessary. Cause rn it works fine for me? Also now with this change we're gonna have 2 paste actions ( Most important thing rn for this is to understand how to repro the error in the screenshot u sent. Then tackle that issue specifically. |
I will try to find a way to reproduce. This is affecting a lot of users, so it is a necessary fix. Maybe it's not an issue on Mac. I have no idea. I will see if there is any cleanup I can do too. Remember, we are also fixing two separate issues with this PR. Image pasting and plain text pasting. |
So, the reason that there are two different "paste" handlers, is because one really just handles images, and the other handles general pasting, specifically for this PR, plain text pasting. They both do two different things. While we could combine them, that would not really be ideal. I also asked Claude: Looking at both paste handlers, they serve different but complementary purposes:
They could be combined, but keeping them separate actually makes sense because:
That said, they could be combined into a single handler in editorProps like this: handlePaste(view, event) {
const items = event.clipboardData.items;
const hasImageItem = Array.from(items).some(item => item.type.startsWith('image/'));
if (hasImageItem) {
for (const item of items) {
if (!item.type.startsWith('image/')) continue;
const file = item.getAsFile();
if (!file) continue;
if (modelSupportsImages(
defaultModel.provider,
defaultModel.model,
defaultModel.title,
defaultModel.capabilities,
)) {
event.preventDefault();
handleImageFile(file).then((resp) => {
if (!resp) return;
const [img, dataUrl] = resp;
const { schema } = view.state;
const node = schema.nodes.image.create({
src: dataUrl,
});
const tr = view.state.tr.insert(0, node);
view.dispatch(tr);
});
return true;
}
}
}
// Handle text paste
event.preventDefault();
const text = event.clipboardData.getData('text/plain');
const lines = text.split(/\r?\n/);
const { tr } = view.state;
const { schema } = view.state;
let pos = view.state.selection.from;
tr.delete(view.state.selection.from, view.state.selection.to);
lines.forEach((line, index) => {
if (index > 0) {
tr.insert(pos++, schema.nodes.hardBreak.create());
}
tr.insertText(line, pos);
pos += line.length;
});
view.dispatch(tr);
return true;
}However, keeping them separate is likely better for code organization and maintainability. The current structure makes it clearer which code handles which functionality and follows TipTap's extension-based architecture. @Fryingpannn The one paste handle is extending the image handling of the editor. This is more for when you add an image using the file import, but also for pasting. It is also doing it by manipulating DOM events. The handlePaste function is for general pasting. Like for text. They do two separate things. There is just a check in the handlePaste function to make sure it isn't an image. It doesn't make sense to combine them. Also, the image pasting part of this doesn't just fix the issue above in the screenshot (bad request), it also fixes the issue with pasting screenshots with CTRL+V. That doesn't work most of the time. If you need me to send you a video of that not working, then I will. Let me know if you can reproduce that issue. Let me know what I can do to get this approved. Thanks. |
|
Will get to your comment soon. Adding another comment about something I remembered when testing earlier: when I copy pasted image from google docs it didn't work. Which works for me on my current version. |
Hmm. Can you do a git pull and try again? I found an issue and fixed. There was a useEffect that was conflicting with the changes to the paste handlers. Also, have @Himanshu-Singh-Chauhan test too with Windows please. Because on all versions that I have had of PearAI, copy and pasting almost never works for images. It is indeed an issue. |
|
Here is a recording showing that I took a screenshot and it is in my clipboard. I try to use CTRL+V and it doesn't work. CTRL+V for screenshots does not work 99% of the time. recording-2024-11-24-05-50-11.webm |
|
Here is a screen recording of it working with this PR: Screen.recording.1.online-video-cutter.com.mp4 |
Better checks for the selected model.
|
@Fryingpannn I took your advice and created one paste handler so there was nothing conflicting. Also, I made sure there was better checking for the selected model. I think that one of the issues was that when the extension first loaded, the selected model wasn't known to the image paste handler, so pasting an image would fail due to a |
|
@Himanshu-Singh-Chauhan - This PR can be closed. It has been replaced by #259 |
Description ✏️
Closes #144
Fixed issues with pasting links and meta data getting pasted instead of the raw link.
There was also issues with pasting screenshots as images. This fixes those issues.
Fixed this issue:
Moved logging
[2024-11-15T16:40:45] [LOG] Pasting imageso that this log only prints when actually pasting an image.@Himanshu-Singh-Chauhan will need to test and verify.
Checklist ✅