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

pin ckeditor version #59

Merged
merged 2 commits into from
Dec 8, 2023
Merged

pin ckeditor version #59

merged 2 commits into from
Dec 8, 2023

Conversation

eluhr
Copy link
Contributor

@eluhr eluhr commented Dec 8, 2023

Due to a BC in ckeditor 4.22 the ckeditor in the widgets test stopped working. This solution has worked well so far

@eluhr eluhr requested review from schmunk42 and handcode December 8, 2023 08:18
Copy link
Member

@handcode handcode left a comment

Choose a reason for hiding this comment

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

That's ok for me.
But out of interest and for the documentation: what exactly is the BC break in ckeditor 4.22?

@eluhr
Copy link
Contributor Author

eluhr commented Dec 8, 2023

Sorry, i mean version 4.23. This is a lts where you need a special license for

Bildschirmfoto 2023-12-08 um 09 38 04

@eluhr eluhr merged commit ca5a505 into master Dec 8, 2023
@eluhr eluhr deleted the feature/pin-ckeditor-version branch December 8, 2023 08:39
@handcode
Copy link
Member

handcode commented Dec 8, 2023

Thanks.
For the records: So it's not a technical BC problem, but an EOL and/or license problem.
https://ckeditor.com/ckeditor-4-support/

@handcode
Copy link
Member

handcode commented Dec 8, 2023

Sorry that I am now coming back after the merge/approve, but...

as far as i see:

so far so good (bad?)

But in the form view only the JsonEditorWidget is used

->widget(\dmstr\jsoneditor\JsonEditorWidget::className(), [

So: can't we completely remove the dependency on 2amigos/yii2-ckeditor-widget (and then also the ckeditor contraint from this PR) here?

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.

2 participants