Skip to content

Conversation

@julien-nc
Copy link
Member

This is based on #1900.

This is an implementation of a file-specific attachment folder for images. When uploading an image or inserting a link, the created image file is placed in a folder next to the .md file (in the .md file owner's storage). The editor then gets those images via a Text internal API endpoint.

With this solution, image files don't have to be shared anymore. Users having access to the .md file are able to get the attached images even if the files are not explicitly shared with them. When such user uploads or insert a link, the image is then accessible to everyone having access to the .md file.

Same with public access. Access to images and ability to upload or insert a link is now possible.
Permission check is done in both user share and public link contexts. Upload and link insertion are only possible when having write permissions.

Text server side serves previews of the image files.
image/x-ms-bmp and image/jpg mime types have been added.

  • If we go for this, should we also copy image files in the attachment folder when "adding image from Files"?
  • What's the best way to write the image link in the markdown content? For the moment, it's a link with a custom protocol: text://image?imageFileName=plop.jpg&textFileId=1234.
  • If the .md file is moved in its owner's storage, the attachment folder is not following and we loose all the images. How could we fix that?
  • Can we assume it's ok to delete attachments when then are deleted in the editor? The user might hit CTRL+z after deleting an image. Could we do an attachment cleanup when the editor is closed?

@julien-nc julien-nc merged commit 2d72f6c into enh/58/insert-local-images Nov 22, 2021
@delete-merged-branch delete-merged-branch bot deleted the enh/58/insert-local-images-in-attachment branch November 22, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants