Vue CKEditor 4 Component implementation

[engineering-this]

Early PR of CKEditor4-Vue Integration

File structure, utils, scripts, configuration files, etc. borrowed from https://github.com/ckeditor/ckeditor5-vue

Source files

• /src/ckeditor.js - CKEditor component, nothing fancy there
• /src/plugin.js - Vue plugin, which just wraps the component

Sample

To run sample you need to build first, by npm run build, then you can just open sample/index.html in your browser. I've put all the components templates in the html files and all logic in main.js. The main purpose of sample for now is manual testing. After some refactoring and polishing it might be used as docs example.

Notes for sample

• Multiple components in one file is Vue anti-pattern, however I did it for now to keep it as simple as possible.
• Before including load-script and support of editorUrl property component could be used directly from source by importing it as ES6 Module and using type="module" attribute on script which included it. If for any reason we wan't to allow using it from source again, than load-script should be included directly in our repo instead of a dependency.

Tests

Run with npm run tests.
To run tests for Firefox npm run tests -- -b Firefox.

For other flags look here - all borrowed from CKE5-Vue.

Just like in ckeditor5-vue I made test of two kind:

• Unit tests, which uses mocked editor: /tests/component.js, /tests/plugin.js, tests/helpers.js
• Integration tests, which uses actual editor: /tests/integration.js

Including CKEditor

Just like in other CKE4 frameworks integration component will detect if CKEDITOR namespace is present, if not it will load from CDN. Also ckeditor.js url can be specified by editorUrl prop.

Component features

Component can be initialized by using ckeditor tag in parent's component template:
<ckeditor></ckeditor>

Supported optional property attributes:

• value - editor data, it needs to be named like it, so v-model can be used for two-way binding. We could add an alias data, but having two attributes doing same thing is not the best thing to do.
• type - editor type, classic or inline.
• editorUrl - link to ckeditor.js file, needed only if CKEDITOR namespace is not loaded other way.
• config - CKEDITOR.config object.
• tagName - Tag name for an element which is to be used for editor creation.
• readOnly - If editor should be initially read only.
• v-model - for two way data binding.

Supported optional event attributes:

• @input - corresponds with editor#change event, needed for v-model.
• @focus - editor#focus event.
• @blur - editor#blur event.
• @ready - when component is ready.
• @destroy - when component is destroyed.

Issues

• When editor is unmounted before it is fully initialised - it throws errors.
  This issue was fixed in Angular by initializing editor outside of component, and then moving it to right place.
• When component with classic editor is destroyed after iframe is removed from DOM CKEditor shows error editor-destroy-iframe.
  This was fixed in Angular by removing option to use classic editor...

Vue router offers guards out of the box, so I've added method destroyEditor which will return promise, which resolves once editor is destroyed. In samples I've declared guards that delay destroying component until editor is destroyed. This fixes both mentioned above issues, we should advocate using guards like this in our docs.

Another case which reproduces above issues is when component is unmounted by v-if. I've managed to add guard-like logic in samples for v-if, which also does the trick.

Note: You might find some leftovers here and there.

Todo

Actually it's hard to list any todos here, it works nice, scripts for building from CKE5 works, so apart from so general stuff like updating README and PACKAGE.json I don't see anything. But let's see what flaws will be found during review.

Helpful links:

https://vuejs.org/v2/guide/
https://learn-vuejs.github.io/vue-patterns/patterns/

[Comandeer]

Two way databinding example looks like it's broken. It's not obvious that the content of the editor is updated after blurring the textarea. What's more, selection becomes blurry when you switch right from textarea to the editor: caret position is reset (in IE11) or selection is totally removed (Chrome).

[jacekbogdanski]

I updated the sample description, so now it correctly tells about the required textarea blur.

What's more, selection becomes blurry when you switch right from textarea to the editor: caret position is reset (in IE11) or selection is totally removed (Chrome).

I don't think it's necessary to spend more time introducing throttling or other mechanisms to delay setting editor data. An example would be more complicated and the main feature (2-way binding via v-model) hidden under the fix. Note it's dev example also, so it's more about implementation show off than UX.

[Comandeer]

LGTM!

Review has been taken over by @commander

[f1ames]

Looking good, just some minor stuff here and there 👍

---

No 100% cc 😱 I think we can live with that anyway ;)

---

Also I would be for keeping branch name consistent, so we will have master and stable and version tags. Now I see it's master and development (and this develop branch targets development one). This is not strictly related to this PR but still wanted to mention that, so we can clean up after merging this PR.

[f1ames]

Travis CI some time ago updated their env and now you can use xenial (16.x) instead of trusty (14.x). See how it's done in CKE4.

[Comandeer]

Actually, aren't our integrations a good place to experiment with GitHub Actions?

[f1ames]

A perfect place I would say, but I would be for doing it as a separate issue to not block beta release. WDYT?

[f1ames]

@Comandeer could you extract it to a separate issue?

[f1ames]

@Comandeer do you think we could easily set up CD with GH Actions? Like automatically publishing tags and e.g. tagging based on commit messages or something like that? So releasing new version will simply require pushing a single commit to master?

[jacekbogdanski]

There is no problem with retargeting the current upstream right now. I suppose that going with master and stable branches, this PR should target stable now? development branch has been only added to prevent accidental merging this PR into master with README which could claim that we are done with beta development before publishing docs / making latest corrections.

Note I answered some of your questions related to my previous review at the origin comments as I couldn't answer them right under yours.

[f1ames]

Looks good 🎉

Great job @jacekbogdanski, @Comandeer and @engineering-this 😄

---

There is no problem with retargeting the current upstream right now. I suppose that going with master and stable branches, this PR should target stable now?

👍 The stable is suppose to have latest stable integration version. Until it's not published, probably stable should be the same as master or empty 🤔

development branch has been only added to prevent accidental merging this PR into master with README which could claim that we are done with beta development before publishing docs / making latest corrections.

So I guess we should still keep it in development until we publish it?