Skip to content
This repository was archived by the owner on Jan 5, 2018. It is now read-only.

Conversation

@phenaproxima
Copy link

I have a use case wherein I would like to be able to slap drupal-entity tags into my CKEditor, but without using Entity Embed's UI at all, and without its button(s) appearing in the editor.

This PR decouples the "engine" part of Entity Embed -- i.e., the widget definition -- from the UI and command parts. They are split into two separate JS files -- engine.js and plugin.js -- and two separate plugins on the Drupal side (drupalentity and drupalentity_button).

@mirodietiker
Copy link

That sounds really interesting and i think if entity_embed wants to offer nice helpers for all use cases we need this kind of separation.

@slashrsm
Copy link
Member

Great work! I agree with @mirodietiker. This change makes a lot of sense. I gave it a quick sanity check only as I don't have time to do a proper review ATM. I hope me or somebody else comes back to properly review and test asap.

@webflo
Copy link
Contributor

webflo commented Nov 12, 2015

👍

@balsama
Copy link

balsama commented Nov 12, 2015

This is great - and likely a common use case. +1

@davereid
Copy link
Member

Please, I'd love to review this as well.

Copy link
Member

Choose a reason for hiding this comment

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

Seems odd that this would still extend EmbedCKEditorPluginBase?

Copy link
Member

Choose a reason for hiding this comment

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

This is strange indeeed.

@slashrsm
Copy link
Member

I tested this PR and it seems to breaks CKEditor.

Could we convert "engine" into a JS library? Would this allow us to use it in button plugin without extending "engine" class.

This PR is definitely a step in the right direction, but it needs some more love.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants