Fix issue of duplicated blocks upon pasting an image into the Slate E…
Fixes Issue
Inside the onImageLoad function in withDeserializers extension of Volto Slate.
uploadContent adds the block using Redux and FormData, but the .then promise callback of uploadContent also adds a new Node in the Slate Editor using Transforms.insertNodes(editor, image); which eventually leads to adding multiple images in the editor.
Deploy Preview for volto canceled.
| Name | Link |
|---|---|
| Latest commit | 138a634059a0254ba957ca6f0e11a78b60ffc044 |
| Latest deploy log | https://app.netlify.com/sites/volto/deploys/65e9ee11d325e500086f471f |
Deploy Preview for plone-components canceled.
| Name | Link |
|---|---|
| Latest commit | b36387acb0c3c358d226bf4bece2d4277908df0f |
| Latest deploy log | https://app.netlify.com/sites/plone-components/deploys/666d49be95110e00089759de |
@plone/volto-team review requested.
@aryan7081 A couple of extra context as upon further testing I noticed the following:
- copy and pasting an image from the website or from the internet results in the correct behavior regardless of your change
- The code that you disabled indeed prevents 2 images from being uploaded when pasting from the os (MAC OS Sonoma in my case)
- That is not enough to prevent the slate empty block from being added or from the image being inserted in the first slate block when pasting from the os clipboard for an image copied from Finder app. It should be tested also in other os systems such as Linux or Windows
@ichim-david Sorry for the delay. Working on this and adding tests as well. Can I test this on linux and windows vm?
@ichim-david Sorry for the delay. Working on this and adding tests as well. Can I test this on linux and windows vm?
@aryan7081 do the minimum test first on whatever platform you're working on and if you can reproduce the issue there then try to fix it, there is no point in doing cross-testing until necessary.
The createImageBlock function within packages/volto-slate/src/utils/volto-blocks.js currently utilizes addBlock to insert a new block at the index position plus one. However, the addBlock function adds multiple blocks: one for the block being added and another empty block if the type is not 'slate'.
To address this, I've attempted to resolve the issue by modifying the createImageBlock function as follows:
const currBlockId = properties.blocks_layout.items[index];
const currBlockHasValue = blockHasValue(properties.blocks[currBlockId]);
let id, newFormData;
if (currBlockHasValue) {
[id, newFormData] = addBlock(properties, 'image', index + 1);
newFormData = changeBlock(newFormData, id, { '@type': 'image', url });
} else {
[id, newFormData] = insertBlock(properties, currBlockId, { '@type': 'image', url });
}
This change ensures that a block is inserted at the (index - 1) position if the current(index) block is empty. Otherwise, it proceeds to create another block as usual. Please let me know your thoughts or any further adjustments needed.
Will push cypress tests once this approach is validated.
https://github.com/plone/volto/assets/71966131/cb836cb1-c490-4525-b06a-015b2e1442da
@aryan7081 please run code quality checks locally before pushing commits. See Linting.
However our documentation lacks the specific commands that need to be run, so I don't blame you for not running undocumented commands. See https://github.com/plone/volto/issues/5341
The CI jobs call https://github.com/plone/volto/blob/main/.github/workflows/code-analysis.yml. You can look at it for the commands that do not pass the checks, and try running them locally.
@stevepiercy Working on cypress tests.
@plone/volto-team This works fine for me, pity is that we don't have any Cypress test for it. I don't even know if it's possible to model that interaction I've investigated it a bit this morning, we have clipboard mocks for pasting text but I have no clue how to mock the image data. I've tried though, if someone wants to take over:
https://github.com/plone/volto/tree/testingpasteimages
For the rest I'd merge it.
@ichim-david I'd appreciate another review from you!
@sneridagh will have a look at this tonight.
@sneridagh Tonight ended up this morning :) Tested it again and there is no more duplication of images but I still find some strange behavior or some differences in behavior at least.
Pasting from os clipboard will upload and use an image block.
Pasting from the same website or other websites will add an img tag with the link to that source.
Ex:
Not saying that it's the fault of this ticket. It just seems to me less useful this img tag linking since you don't have control over size or alignment like you have when it's uploaded through the image block.
@ichim-david paste from a website you mean copy the html with along text or only the image? Is this different than before?
@ichim-david paste from a website you mean copy the html with along text or only the image? Is this different than before?
@sneridagh right click copy image only. It's not different from before. I guess it would be useful to copy paste alongside text otherwise it's better to drag and image from desktop to get proper image block usage alongside upload and image options.
Ready for merging from my part.