Implementation of the core feature

[engineering-this]

Summary of how integration works, after changes requested by reviewers:

1. Insert <ckeditor></ckeditor> in template to add editor.
2. Importing editor:

• When editor namespace is missing editor will be loaded from CDN.
• ckeditor.js location can be pointed as editorUrl property.
• ckeditor.js can be loaded in <script>.

3. Editor type can be chosen with type property. Allowed types are divarea and inline.
   Angular doesn't expose hook beforeDestroy which would allow us for proper cleanup. Workaround for that missing hook is that editor is initialized as child of body, outside of its component, and once is it created it is appended to right place. That's because unloading component by angular before editor is initialized results in console errors and possibly memory leaks. Framed editor can't be moved around, moving iframe around document breaks it, so we can't support classic editor. See Enable classic editor #6
4. Editor works with ngModel. However CVA interface implementation doesn't include disabled, because CKEditor doesn't support it either. It is similar, yet different to supported readOnly.
5. Data can be two-way bound by either data or ngModel.
6. Component exposes events:

• ready
• change
• dataChange
• focus
• blur

7. config can be set as attribute in template. It can be used to access any other event.
8. tagName property can be used to change element on which editor is created, although I'm not sure if we really need it.
9. Editor can be used with angular directives, eg. ngIf, hidden.
10. Repo contains sample app, which is helpful for testing, both manual and automatic.
11. Automatic tests:

• Unit test for component.
• Integration/unit tests for component usage with sample app.
• Basic e2e tests.

12. NPM scripts:

• start: "ng serve", // Build and serve sample, for manual testing.
• build: "ng build", // Build samples with component.
• build-package: "node ./scripts/build-package.js", // Build package - only our component
• test: "ng test", // Run automatic test other than e2e
• e2e: "ng e2e" // Run e2e tests

Only limitation comes from point 3. every other CKEditor 4 feature seems to work correctly.

Original PR message and edits:

Changes within this PR:

1. I’ve reformatted code to be compliant with /~https://github.com/cksource/att-ckeditor-dev code style.

2. Two-way data binding is implemented for property ngModel and for data (dataChange) properties. dataChange event is duplicate of change, but to allow two-way we need event like: ${eventName}change.
   This was tricky as ControlValueAccessor fires writeValue asynchronously after ngAfterViewInit while regular attribute binding is done way earlier even before ngOnInit.
   This produces inconsistency on when data is set on component.
   Editor should be initialized with initial data, and we shouldn’t set it on instanceReady.
   I changed it so editor is now created with initial data, however I had to move editor creation method into setTimeout.

3. I’ve updated tests, so methods beforeEach and afterEach will prevent further testing until editor is ready/destroyed.

4. I've added third sample called 'playground'. I've created it for manual testing and debugging. There are two editors, classic and inline, both are bound together, so they share same data. One is bound by ngModel and second by data.

---

Additionally I’ve done some research on topics from our talk, however I didn’t apply changes regarding it:

1. Dropping setTimeout from ngOnDestroy.
   Currently this breaks some tests, but I think this is an issue with tests, not our component.
   It is removed.

2. Is calling this.ngZone.run necessary?
   Actually calling this method is result of performing all editors logic inside ngZone.runOutsideAngular. And we should do this for performance purpose. Angular tracks changes, events and performs lots of other tasks. We don’t need to do all this for our editor contents. However we need to go back into ngZone on event callbacks so it is executed in Angular scope.

3. Can we pass data inside angular <ckeditorComponent> tags?
   Answer is: yes.
   However I think that would lead to more complications and I think we should drop this idea.
   Consider example:
   <ckeditorComponent>some content</ckeditorComponent>
   We can achieve content, by using <ng-content> tag in templates. However I didn’t find way to access it inside component constructor. We could put <ng-content> in <textarea> which we use for editor creation, but as updating data replaces <textarea> content with new string every component/element reference would be broken. Such approach would suggest users that they can put angular components inside our editor.

4. Disabled or readonly according to specs:
   disabled This Boolean attribute indicates that the user cannot interact with the control. If this attribute is not specified, the control inherits its setting from the containing element, for example

   ; if there is no containing element when the disabled attribute is set, the control is enabled.
   readonly This Boolean attribute indicates that the user cannot modify the value of the control. Unlike the disabled attribute, the readonly attribute does not prevent the user from clicking or selecting in the control. The value of a read-only control is still submitted with the form.

5. Fixed failing tests.
   Most of tests now are using done() in beforeEach and afterEach, so test cases won't run until component is fully reseted.

---

To do:

1. Reduce preset. Included in PR.

2. Consider other ways of importing editor.

• I think letting user to import editor is optimal, so user can include any build either official preset or custom one.

3. End-to-end testing. included in PR.

Closes #2
Closes #3
Closes #4

[jacekbogdanski]

This comment is redundant. It's a common practice used in object-oriented languages (e.g. Java, C#).

[jacekbogdanski]

To be consistent let initialize this value at declaration.

[jacekbogdanski]

After your changes updateData function is redundant and its content should be placed directly here.

[jacekbogdanski]

It's subjective but I like to treat const as a real business constant value like enums etc. Actually, you are changing the value of the element line below, so it's not constant at all. The let keyword wasn't placed here accidentally, however, I understand that this approach varies between developers. WDYT about small RFC on slack?

[jacekbogdanski]

And hey! You have to type fewer letters with let 🏆

[engineering-this]

It's generally suggested to use const everywhere unless you need to reassign value. Even my IDE suggests to change each let to const when it hasn't been reassigned. Popular linter configurations also requires to use const whenever possible.

It's subjective but I like to treat const as a real business constant value like enums etc.

In such case it's more common to use uppercased names to distinguish them from other declarations. In CKEditor codebase you can find plenty of them, eg. CKEDITOR.TRISTATE_DISABLED.

[jacekbogdanski]

Yep, this can be safely removed now.

[jacekbogdanski]

After your changes, you are replacing the negative value with an empty string so this condition is redundant.

[jacekbogdanski]

Editor initialization

I wonder what's wrong with initializing data on instanceReady event? This allows you to drop nasty timeout for editor creation and additional forceChange property. To improve the whole process you could just add condition to make sure that data really changed when using ng forms:

			if ( this.data !== this.instance.getData() ) {
				this.instance.setData( this.data );
			}

so editor's data change won't be duplicated.

I think that dataChange output gives us opportunity to treat change output as 1:1 CKEditor API - and now we can have 3 different listeners for data:

• dataChange for 2-way binding, only if data actually changed
• onChange used by CVA - also called when data changed
• change 1:1 CKEditor API

I already prepared some POC to test whole logic, you can check it at t/1b.

Inline editor

There is also an issue with inline editor - place selection at the end of the editor and start typing. Selection will be moved at the begining of the editor on every typed character.

Destroying editor

I think that we have to stress the whole timeout inside ngDestroy a lot. It doesn't seems to work properly:

1. Run samples.
2. Open console.
3. Move between samples e.g. from simple usage to integration playground.

Actual

[engineering-this]

Pushed few commits.
-Added IE polyfills.
-Ignored one test in IE10 because it fails.

All tests passe in IE11, tests passed in IE9, but there is some typeerror because of missing includes polyfill, although it is imported, probably something is wrong with polyfill for that it doesn't correctly work there.
Note polyfills doesn't affect output builded package. They are imported only for testing purposes.