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

CKEditor custom build no longer works with new CKEditor builder #593

Open
eleven59 opened this issue Aug 15, 2024 · 2 comments
Open

CKEditor custom build no longer works with new CKEditor builder #593

eleven59 opened this issue Aug 15, 2024 · 2 comments
Assignees

Comments

@eleven59
Copy link

The steps outlined here for using a custom CKEditor build with the ckeditor field no longer work with the new CKEditor builder. There are multiple reasons for this.

  • The downloaded package from CKEditor Builder is completely different now, and requires either running npm install in the folder downloaded from them, or using CDN. Neither will give you the ckeditor.js file you can just include as described in your docs.
  • The new CKEditor builder requires the generated JS file to be loaded as a module, and the CDN option also uses an importmap. But there is no way to provide the script type ("module" or "importmap") with CRUD::field()->custom_build() or even with Widget::add()->type('script').
  • The instructions for bpFieldInitCustomCkeditorElement use the global ClassicEditor to initialize the editor(s), but this is no longer available as a global as it is only available within the JS module.

In the end, I managed to hack it into a working state using the new CKEditor builder. However, this is not ideal and I do not suggest replacing your instructions with mine, as this does not work for dynamically created ckeditor fields since the initialization is only called once on page load. I only provide my steps here so you can get a working implementation going to experiment with further if needed. If I understand the new CKEditor implementation well, however, I think it is actually an opportunity to make it easier to use custom builds, as the only files that would need to be changed for each instance, potentially, would be the 'main.js' and 'script.css' files. I think the rest could be included quite easily if this would be implemented correctly in the Backpack/Pro codebase, which would greatly streamline the use of custom builds, again potentially.

Anyway, here's how I got it working, in case it helps.

On the final page of the CKEditor builder, I picked "Vanilla JS", "Cloud (CDN)", and "Copy code snippets").

1 Put script.css and main.js in public/assets/ckeditor and add the code from CKEditor's instructions. Also add an empty JS file called init.js.

2 In main.js, change the last line from the provided script:

from: ClassicEditor.create(document.querySelector('#editor'), editorConfig);
to: ClassicEditor.create(document.querySelector('.custom-ckeditor'), editorConfig);

3 Add the following to init.js

async function bpFieldInitCustomCkeditorElement(element) {

    // Since ClassicEditor is not available as global, we can't initiate the editor here and save it to a variable.
    // So we need to get the ckeditor instance from the initialized editor itself.
    // See https://ckeditor.com/docs/ckeditor5/43.0.0/framework/how-tos.html#how-to-get-the-editor-instance-object-from-the-dom-element

    element.on('CrudField:delete', function (e) {
        element[0].parentElement.querySelector('.ck-editor__editable_inline').ckeditorInstance.destroy();
    });

    element.on('CrudField:disable', function (e) {
        element[0].parentElement.querySelector('.ck-editor__editable_inline').ckeditorInstance.enableReadOnlyMode('CrudField');
    });

    element.on('CrudField:enable', function (e) {
        element[0].parentElement.querySelector('.ck-editor__editable_inline').ckeditorInstance.disableReadOnlyMode('CrudField');
    });
}

4 In the CRUD controller's create(/update) method, add:

Widget::add()->type('style')->content(public_path('assets/ckeditor/style.css'));
Widget::add()->type('style')->content('https://cdn.ckeditor.com/ckeditor5/43.0.0/ckeditor5.css');
Widget::add()->type('view')->view('ckeditor.init');

And for the CKEditor field itself, use:

CRUD::field('content')
->type('ckeditor')
->attributes([
    'data-init-function' => 'bpFieldInitCustomCkeditorElement',
    'class' => 'custom-ckeditor form-control'
])
->custom_build(['/assets/ckeditor/init.js']);

5 The contents of the ckeditor/init.blade.php view are as follows.

<script type="importmap">
    {
        "imports": {
            "ckeditor5": "https://cdn.ckeditor.com/ckeditor5/43.0.0/ckeditor5.js",
            "ckeditor5/": "https://cdn.ckeditor.com/ckeditor5/43.0.0/"
        }
    }
</script>
<script type="module" src="{{ url('/assets/ckeditor/main.js') }}"></script>

As I said, it works, but not on dynamic fields and it is quite cumbersome. Getting it to work easier and also with dynamic fields would also require changes to the codebase not the docs. But since it is not technically a 'bug' but a change request, and my main gripe is that the docs are now not up-to-date, I posted here.

If you'd like, I would happily post again somewere else (just tell me where).

@tabacitu tabacitu self-assigned this Aug 19, 2024
@tabacitu
Copy link
Member

Excellent write-up, thank you very much @eleven59 ! I do think we need to re-think how we implement CKEditor, yes.

So just to confirm:
A. Our standard CKEditor field is working just fine (albeit using v37).
B. You ran into this problem because you wanted a custom CKEditor build... and the instructions we provided here are no longer relevant.

So what we need to do next:

  1. Rewrite our docs to fix the steps people can take right now for custom builds.
  2. Bump our ckeditor field to use the lastest version (most likely from CDN, with import maps).
  3. In the process, figure out a way to either make it backwards-compatibly (unlikely) or to make it easier to do custom builds.

It does sound to me like this is going to be a breaking change. But we do want to ship a new "maintenance" release of PRO in the next 1-2 months, so that's a GREAT moment to do that.

Thanks again for writing this up @eleven59 . I'll ask @pxpm to give this a look next week, since we'll start spinning on wheels on the next version of PRO quite soon.

@eleven59
Copy link
Author

Hi! Thanks for picking this up, and your idea to add it to the maintenance release sounds perfect.

Re: your questions:
A. Yes, no issues with the standard field
B. Yes, the new CKEditor builder completely breaks it, and the old CKEditor builder is also inaccessible so there is no workaround either

And yes that would likely be a breaking change, but:

  • shouldn't necessarily be breaking for the standard ckeditor field if done right
  • if you do something clever with how the custom_build function accepts parameters, you could perhaps detect whether the custom build config entered in CRUD controllers is for the new or old builder, and support legacy accordingly (which would of course bloat the base install because you'd have to include two versions of ckeditor, but not by much if you get both mostly off CDNs)

Just my two cents here. Of course I fully trust you are best equipped to decide how this fits in Backpack standards and considering the quality of Backpack as a whole I have no doubts you will be able to make this work in the best possible way.

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

No branches or pull requests

3 participants