[IMP] Clipboard: support images in the clipboard #5098
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
rrahir
changed the title
rrahir
changed the title
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
11 times, most recently
from
9695adc to
ad869b0
Compare
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
5 times, most recently
from
13e3841 to
405eb7c
Compare
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
from
405eb7c to
4a8361d
Compare
| const file = await this.getImageFromUser(); | ||
| const file = await this.userImageUpload(); | ||
| if (file.size > MAX_FILE_SIZE) { | ||
| throw new ImageImportError(_t("The file you are trying to upload is too large (> 5 mB)")); |
There was a problem hiding this comment.
To be honest, IMO this kind of limitation is the worst possible user experience. Whenever I'm faced with this I rage-quit half of the time, rather than finding a shitty tool to reduce the image size.
I prefer if it works but it's slow rather than not working at all.
There was a problem hiding this comment.
yup, it might not be useful at upload. The filestore implementation might reject files too big but it can be taken care of at that moment.
There was a problem hiding this comment.
=> removed the upload limitation
LucasLefevre
reviewed
Jan 9, 2025
Comment on lines
94
to
192
| image.src = imageUrl; | ||
| image.addEventListener("load", () => { |
There was a problem hiding this comment.
probably doesn't matter in practice, but should the event listener be attached before setting src ? If somehow image.src = imageUrl; is instant and synchronous, the event handler would never be called
There was a problem hiding this comment.
I just couldn't make it work without that, i might have missed something...
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
3 times, most recently
from
f3e07a4 to
22ca025
Compare
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
4 times, most recently
from
d53c87b to
2c70940
Compare
This commit adds the support of images in the clipboard. This allows the user to insert images from their OS faster than going through the "insert" menu and also to extract charts/images from the spreadsheet to use it in other applications (word, powerpoint, discuss apps, ...). Features: - Png images will be pasted to the clipboard instantly on "copy" - Charts can be added to the clipboard via a menu button "copy as image" - Pasting an image from the OS will add it to the spreadsheet as an image and place it in the current sheet. - Both images and charts can be downloaded as images by the user via the figure menu Limitations: - Charts will not be added to the clipboard directly as it can mislead users when using cross-sheet copy/paste. Namely, it would add the chart as an image on paste and not as a chart. - non-png images need to be converted as png to be added in the clipboard and this operation can be quite slow. We add a limit of 5mb image. Past that point, the image is not copied to the clipboard in order to avoid slowing down the user experience. Task: 4235104
rrahir
force-pushed
the
master-handle-images-in-clipboard-rar
branch
3 times, most recently
from
8cd390d to
b377210
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This commit adds the support of images in the clipboard. Namely,
the filestore and dispatch a CREATE_IMAGE accordingly.
oOS clipboard. It can then be pasted somewhere else, like a discuss
app, a text editor maybe?.
TODO
because we never check agains the actual type of the file we upload
but it's wrong. Maybe use a generic
image/*mimetype instead of usingpng. We still need to purge out dangerous mimetypes like image/svg(+xml)
we are copy pasting from the same spreadsheet clipboard (potential
useless upload)
clipboard has some text/* types along the image/* so it ignores the
later. for a fully integrated feature, we should probably decorrelate it
from the standard copy paste and make a menu "copy image to os
clipboard" ? tbd
Task: 4235104
review checklist