Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up editor types #133

Merged
merged 13 commits into from
Sep 16, 2020
Merged

Clean up editor types #133

merged 13 commits into from
Sep 16, 2020

Conversation

Dumluregn
Copy link
Collaborator

@Dumluregn Dumluregn commented Sep 8, 2020

EDIT:
This PR now removes the divarea editor type instead.

Proposed changelog entry (2.0.0):

* [#130](/~https://github.com/ckeditor/ckeditor4-angular/issues/130): `Divarea` is no longer treated as the separate editor type. [`divarea` plugin](https://ckeditor.com/cke4/addon/divarea) can still be used just by adding it to the custom configuration. 

This PR contains:
- API docs fix (update info that classic editor is the default one, not divarea);
- tests fix (tests for warning messages were failing);
- mechanism adding floatingspace plugin to the build if it is in removePlugins config and the editor type is inline (so analogous to the divarea editor type) together with unit tests;
- some minor refactoring.

I'd say there is still some code that could use a refactor (e.g. removePlugin() method), but it wasn't the subject of this PR so I didn't touch it now.

Short note about tests - changing from beforeEach() to beforeAll() (which was reverting the recent change introduced in ebf6aed) was necessary as the config was changing after it was passed to the component. Now the config is updated properly, i.e. before the describe() block that contains it. And it is separate for each editor type and each config suit, so everything works fine.

Closes #130.

@jacekbogdanski jacekbogdanski self-assigned this Sep 9, 2020
@jacekbogdanski jacekbogdanski self-requested a review September 9, 2020 08:23
Copy link
Member

@jacekbogdanski jacekbogdanski left a comment

Choose a reason for hiding this comment

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

I don't really remember what was the resolution of our discussion on how we should handle deprecating divarea editor type. However, if we would like to follow #130 we should just drop the whole logic responsible for adding divarea plugin and throw warring in case of this type, that it's no longer supported and the developer should use divarea plugin instead.

It may be a breaking change for someone, so it should be included in a major release. We could do it "easy" way and instead of dropping the current logic for this type, just send a warning that it's deprecated. But I'm not sure if that really makes sense looking at that the current implementation is totally different from our standard approach. WDYT?

src/ckeditor/ckeditor.component.ts Outdated Show resolved Hide resolved
@f1ames
Copy link
Contributor

f1ames commented Sep 9, 2020

I don't really remember what was the resolution of our discussion on how we should handle deprecating divarea editor type. However, if we would like to follow #130 we should just drop the whole logic responsible for adding divarea plugin and throw warring in case of this type, that it's no longer supported and the developer should use divarea plugin instead.

I think the conclusion was that we can do #127 easily ("just" clean up warnings) and it's main goal is to have green CI. And #130 is about deprecating/removing diveare editor type. That was the reason we split it into two task I suppose.

If it turns out that it doesn't make sense to cover #127 separately from #130 we may try to cover both 🤔

@Dumluregn Dumluregn self-assigned this Sep 11, 2020
@jacekbogdanski jacekbogdanski self-assigned this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate divarea editor type
3 participants