Skip to content

Add image file upload in QuillJS editor#8250

Merged
leio10 merged 14 commits intodevelopfrom
feat/wysiwyg_images_upload
Sep 22, 2021
Merged

Add image file upload in QuillJS editor#8250
leio10 merged 14 commits intodevelopfrom
feat/wysiwyg_images_upload

Conversation

@entantoencuanto
Copy link
Copy Markdown
Contributor

@entantoencuanto entantoencuanto commented Aug 1, 2021 •
edited by andreslucena
Loading

🎩 What? Why?

This PR:

  • Adds an Decidim::EditorImage model to store image files using ActiveStorage
  • Adds an admin API endpoint to create editor images. This action is only allowed to admins
  • Adds an API endpoint to upload images as ActiveStorage attachments
  • Updates admin editor to include an image button and includes 2 modules to Quill editor:
    • Image upload: This module allows the attachment of images using the API
    • Image resize: A module to change the image dimensions
  • Adds a migration task which parses all html contents editable by admins and replaces all base64 images with editor image items created by the task.
  • Adds an entry in CHANGELOG with instructions to use the migration to editor images task

📌 Related Issues

Testing

Describe the best way to test or validate your PR.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

imatge
imatge

♥️ Thank you!

@entantoencuanto entantoencuanto force-pushed the feat/wysiwyg_images_upload branch from d7704f4 to 2fa5ba1 Compare August 1, 2021 21:29
@andreslucena andreslucena changed the title Feat/wysiwyg images upload Add image file upload in QuillJS editor Aug 30, 2021
@entantoencuanto entantoencuanto force-pushed the feat/wysiwyg_images_upload branch from 8a94443 to 8dcfd94 Compare August 31, 2021 20:34
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Sep 1, 2021

@entantoencuanto following up in your last comment in the issue:

  1. What should I look for? Can you add screenshots 🙏🏽? I can't find the image upload icon (imatge) in the editor's toolbar. Also drag and dropping is not working (it uploads to a base64 image)
  2. I see that we also have the migration task from Migrate base64 images to file uploads in WYSIWYG editor #7713, but it's missing the CHANGELOG instructions. Can you add them please?

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Sep 1, 2021

Also, there are Lint warnings

@entantoencuanto entantoencuanto force-pushed the feat/wysiwyg_images_upload branch from a525bdf to eee31b4 Compare September 1, 2021 11:26
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Sep 1, 2021

What should I look for? Can you add screenshots 🙏🏽? I can't find the image upload icon (imatge) in the editor's toolbar. Also drag and dropping is not working (it uploads to a base64 image)

Never mind, I can see it in your staging server

@entantoencuanto entantoencuanto force-pushed the feat/wysiwyg_images_upload branch 6 times, most recently from 105c410 to b5297ea Compare September 6, 2021 18:19
@entantoencuanto entantoencuanto marked this pull request as ready for review September 6, 2021 18:56
andreslucena
andreslucena requested changes Sep 7, 2021
View reviewed changes
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Sep 7, 2021

@entantoencuanto I've added screenshots to your PR body.

Please review my proposed change in the task.

It's great to finally have this feature 🎉 🎉

andreslucena
andreslucena previously approved these changes Sep 8, 2021
View reviewed changes
@entantoencuanto entantoencuanto force-pushed the feat/wysiwyg_images_upload branch from 94809aa to 86f9664 Compare September 9, 2021 16:03
@leio10 leio10 force-pushed the feat/wysiwyg_images_upload branch from 86f9664 to 6d743bd Compare September 21, 2021 14:30
leio10
leio10 suggested changes Sep 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 👏👏👏
Please, address my comment and we can merge this! ❤️

decidim-core/lib/decidim/content_parsers/inline_images_parser.rb Outdated
#
# @see BaseParser Examples of how to use a content parser
class InlineImagesParser < BaseParser
AVAILABLE_ATTRIBUTES = {
Copy link
Copy Markdown
Contributor

@leio10 leio10 Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Having this hash in the core module creates a dependency cycle (as core shouldn't know about other modules). But as this constant is only used for the migration task, I think it would be enough to move it to the .rake file.
Also, I wouldn't use .freeze to help apps and modules to register new attributes for their own classes.

Copy link
Copy Markdown
Contributor Author

@entantoencuanto entantoencuanto Sep 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I've added the attributes as a method in the task because as a constant the hash is generated empty

@leio10 leio10 merged commit b7f0106 into develop Sep 22, 2021
@leio10 leio10 deleted the feat/wysiwyg_images_upload branch September 22, 2021 07:59
@entantoencuanto entantoencuanto mentioned this pull request Dec 9, 2021
Closed
@andreslucena andreslucena mentioned this pull request Dec 15, 2021
Merged
@microstudi microstudi mentioned this pull request Dec 15, 2021
Closed
@raulpopadineti raulpopadineti mentioned this pull request Sep 26, 2022
Closed
1 task
@raulpopadineti raulpopadineti mentioned this pull request Nov 1, 2022
Closed
1 task
@basicavisual basicavisual mentioned this pull request Jan 11, 2023
Merged
@Quentinchampenois Quentinchampenois mentioned this pull request Mar 2, 2023
Closed
@lawrencemcn lawrencemcn mentioned this pull request Mar 15, 2023
Closed
@alecslupu alecslupu mentioned this pull request Apr 3, 2023
Merged
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena andreslucena left review comments

+1 more reviewer

@leio10 leio10 leio10 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: core

Projects

None yet

Milestone

0.26.0

Development

Successfully merging this pull request may close these issues.

Migrate base64 images to file uploads in WYSIWYG editor Image file upload in WYSWYG editor

4 participants

@entantoencuanto @andreslucena @leio10 @alecslupu