search

Doist / typist

The mighty Tiptap-based rich-text editor that powers Doist products.
https://typist.doist.dev
MIT License
468 stars 12 forks source link

Images copied from the browser are pasted as inline links rather than handled via `onImageFilePaste` #397

Closed frankieyan closed 1 year ago

frankieyan commented 1 year ago

Not sure if this is truly a bug, but when you copy an image from the browser and paste it into the composer, it shows up as an inline image that is sourced from the original URL, instead of being uploaded (via onImageFilePaste).

This is somewhat of a problem if the URL requires authentication, such as when copied from GitHub. It's also a bit inconsistent compared to other instances of inline images where they are always uploaded to our servers. The same image copied from a browser but pasted to other desktop apps (e.g. Signal, Whatsapp) often end up being uploaded as well.

For our users, this may not be very transparent because the pasted image will likely still be viewable by them, and they'd only realize a problem if they inspect the URL of the image.

Reproduction

image

Additional information

Clipboard data from https://evercoder.github.io/clipboard-inspector/ image

It's possible that this conflicts with the solution we have in place to prevent tabular data copied from Numbers from pasting an image: https://github.com/Doist/typist/pull/290

rfgamaral commented 1 year ago

Not sure if this is truly a bug, but when you copy an image from the browser and paste it into the composer, it shows up as an inline image that is sourced from the original URL, instead of being uploaded (via onImageFilePaste).

Yes, I believe this should be considered a bug. Explicitly copied images should always be handled by uploader (if there is one).

It's possible that this conflicts with the solution we have in place to prevent tabular data copied from Numbers from pasting an image: #290

Indeed, this is the problem, and it's a tricky one. To have our cake and eat it too, we need to figure out a better way to distinguish the user intention when copying something to the clipboard.

Looking at multiple clipboard data examples, I can think of a few ways to work around this, but they are all band aids that could be causing some other issue (like this one) in the future. The proper fix would be to identify edge cases (like Numbers adding an image file to the clipboard), and work around only those edge cases. But for Numbers, specifically, I can't think of a way to identify that edge case.

rfgamaral commented 1 year ago

A more pragmatic solution to this problem could be to remove the solution that we have in place to prevent tabular data copied from Numbers from pasting an image, which would prioritize pasting images (if one is available), thus forcing users to use the specific "paste as plain-text" keyboard shortcut if they want to paste text.

This may not be what users expected, because they expect things to work intelligently, in a way that the system "guesses" what they want (I expect that too, but I'm not sure it will be possible here), but it would be a way to have everything working, which right now we don't have.

@frankieyan Thoughts?

frankieyan commented 1 year ago

A more pragmatic solution to this problem could be to remove the solution that we have in place to prevent tabular data copied from Numbers from pasting an image, which would prioritize pasting images (if one is available), thus forcing users to use the specific "paste as plain-text" keyboard shortcut if they want to paste text.

I agree with this. We don't have any feedback from users that pasting from Numbers is something that happens often enough to warrant images being deprioritized, and I think having onImageFilePaste handle pasted images consistently is more important here.

rfgamaral commented 1 year ago

@frankieyan In that case, and since I don't have access to Numbers, do you mind doing the following:

If possible, screen record these tests so I can double-check everything is working as expected.

frankieyan commented 1 year ago

@frankieyan In that case, and since I don't have access to Numbers, do you mind doing the following:

  • Remove these lines and launch the local Storybook
  • Copy a table from Numbers, and...
    • Paste it to the rich-text editor with Cmd+V
      • Was the image pasted?
    • Paste it to the rich-text editor with Cmd+Shift+V
      • Was the table text pasted correctly?

If possible, screen record these tests so I can double-check everything is working as expected.

Sorry Ricardo, I haven't found the time to come back to this issue yet, as we have a big release coming up tomorrow. I should be able to test this out closer to the end of the week!