ckeditor / ckeditor4-angular

Official CKEditor 4 Angular component.
Other
55 stars 32 forks source link

Clean up classic editor warnings #127

Closed jacekbogdanski closed 3 years ago

jacekbogdanski commented 4 years ago

We are still warning users about divarea plugin requirement which is no longer true - https://github.com/ckeditor/ckeditor4-angular/blob/4fff407eb8e01dc2ea771ce385c5ed78c096612f/src/ckeditor/ckeditor.component.ts#L433-L438

We should check if this warning could be safely removed (probably with some code refactoring also) and correct https://github.com/ckeditor/ckeditor4-angular/blob/4fff407eb8e01dc2ea771ce385c5ed78c096612f/src/ckeditor/ckeditor.component.spec.ts#L122-L171 unit tests which are failing when run in random order.

Note that we should still use divarea plugin by default as it's described by our docs https://github.com/ckeditor/ckeditor4-angular/blob/4fff407eb8e01dc2ea771ce385c5ed78c096612f/src/ckeditor/ckeditor.component.ts#L55-L65 and changing that would introduce breaking changes.

Dumluregn commented 4 years ago

Note that we should still use divarea plugin by default as it's described by our docs

AFAICS this sentence is a leftover from the versions before 1.0.0, when we switched the default editor type to Classic - see the last line quoted above, our docs or changelog.

jacekbogdanski commented 4 years ago

You are right, we've updated editor to classic but didn't do it with docs - https://github.com/ckeditor/ckeditor4-angular/blob/4fff407eb8e01dc2ea771ce385c5ed78c096612f/src/ckeditor/ckeditor.component.ts#L55-L65

I'm also wondering if we shouldn't deprecate divarea editor type - https://github.com/ckeditor/ckeditor4-angular/blob/4fff407eb8e01dc2ea771ce385c5ed78c096612f/src/ckeditor/ckeditor.ts#L30

Because it's in contrary to other integrations and overall editor implementation where divarea is just a plugin, not editor type.

Dumluregn commented 4 years ago

Because it's in contrary to other integrations and overall editor implementation where divarea is just a plugin, not editor type.

Yeah, I guess it's because at first divarea editor was the default one, so it wasn't removed to keep the backward compatibility and TBH I don't know if we are not stuck with it till the end of the world 🤔

f1ames commented 4 years ago

Yeah, I guess it's because at first divarea editor was the default one, so it wasn't removed to keep the backward compatibility and TBH I don't know if we are not stuck with it till the end of the world

Well, I think we can remove it, but it will mean breaking change and 2.0.0 release. It will require some docs updates too (and probably some short migration guide) but it will be good to have consistent API for all integrations I suppose.

Dumluregn commented 4 years ago

Ok, makes sense. I will create a separate issue for deprecating the divarea editor type as it's less urgent than fixing this one.

f1ames commented 3 years ago

Let's cover it in #130.