Trigger event when Text Editor is attached

Created on 6 November 2022, over 1 year ago
Updated 8 April 2024, 3 months ago

Problem/Motivation

It would be helpful if a javascript event was triggered when a CKEditor5 instance is created. Currently, it is possible for other modules to interact with editor instances using the Drupal.CKEditor5Instances map, but because the editor is created asynchronously, there's not a good way to know when the instance is ready to interact with. By triggering an even during the promise resolution, it would be possible for other code to react to it once it's ready.

My specific use case is that I have a form where some fields are disabled to start until the user clicks a checkbox toggle to enable them. I can do that using ckeditor's enableReadOnlyMode and disableReadOnlyMode methods, but I'm not able to have it initially set to read only on page load.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs work

Version

11.0 πŸ”₯

Component
EditorΒ  β†’

Last updated 20 days ago

Created by

πŸ‡ΊπŸ‡ΈUnited States JeffM2001

Live updates comments and jobs are added and updated live.
  • API addition

    Enhances an existing API or introduces a new subsystem. Depending on the size and impact, possibly backportable to earlier major versions.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • last update about 1 year ago
    Custom Commands Failed
  • πŸ‡ΊπŸ‡ΈUnited States Tim Bozeman

    What do y'all think about this patch? It tweaks Drupal.editors[editor].attach so it expects a promise and then dispatches an editor:attached event.

    I was doing a really odd work around before finding this issue.

            const observer = new MutationObserver((mutationsList) => {
              mutationsList.forEach((mutation) => {
                if (mutation.attributeName === 'data-ckeditor5-id') {
                  var waitForEditor = setInterval((ckeditor5Id, textArea) => {
                    const editor = Drupal.CKEditor5Instances.get(ckeditor5Id);
                    if (editor) {
    
                      // Do stuff ...
    
                      clearInterval(waitForEditor);
                      observer.disconnect();
                    }
                  }, 50, mutation.target.dataset.ckeditor5Id, mutation.target);
                }
              });
            });
            observer.observe(field, {attributes: true});
    
  • last update about 1 year ago
    Patch Failed to Apply
  • πŸ‡ΊπŸ‡ΈUnited States Tim Bozeman

    Custom commands...

  • last update about 1 year ago
    28,526 pass
  • πŸ‡ΊπŸ‡ΈUnited States Tim Bozeman

    😬

  • Status changed to Needs work about 1 year ago
  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • last update about 1 year ago
    Patch Failed to Apply
  • πŸ‡«πŸ‡·France nod_ Lille

    should be 11.x even

    +++ b/core/modules/editor/js/editor.js
    @@ -297,7 +297,18 @@
    +      const editorAttachPromise = Drupal.editors[format.editor].attach(
    +        field,
    +        format,
    +      );
    +      editorAttachPromise.then(
    +        (editor) => {
    +          $(document).trigger('editor:attached', [editor]);
    +        },
    +        (error) => {
    +          console.log(`Failed to attach editor.\n${error}`);
    +        },
    +      );
    

    I'd do away with the editorAttachPromise variable and simply call .then() after the .attach().

    Also it would be better to use a catch instead of the second parameter of the then method.

  • Status changed to Needs review about 1 year ago
  • last update about 1 year ago
    29,486 pass
  • πŸ‡ΊπŸ‡ΈUnited States Tim Bozeman

    Wow, that was quick! Thanks for reviewing! Yes indeed, that is more concise. πŸ‘πŸ»

  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Could we add some test coverage for this new feature?

    Also could some of the missing pieces of the issue summary be updated just to help the review

    Thanks!.

  • First commit to issue fork.
  • Pipeline finished with Failed
    7 months ago
    Total: 232s
    #52790
  • πŸ‡ΊπŸ‡¦Ukraine dinazaur

    I just opened MR, so it is easier to manage, review, and update the code. It contains same changes as the patch in #13. I just could not apply the patch for the latest D10, so re-rolled it in MR.

  • πŸ‡ΊπŸ‡ΈUnited States Tim Bozeman

    It looks like some extra white space was added when re-rolling #13 into an MR.

  • First commit to issue fork.
  • Pipeline finished with Failed
    7 months ago
    Total: 196s
    #62001
  • Removed the whitespaces mentioned in #18 and rebased the branch on 11.x

  • πŸ‡ΊπŸ‡ΈUnited States nessthehero

    Was able to spin up an instance using the MR and write a custom module that ties into this event. Seems to work great!

    ((Drupal, $) => {
    
      Drupal.behaviors.testEditorAttachment = {
        attach() {
    
          console.log('behavior attached');
    
          $(document).on('editor:attached', function (event, editor) {
            if (typeof editor !== "undefined") {
              console.log('Event works!', event, editor);
    
              editor.setData('<p>Hello world!</p>');
            }
          });
    
        }
      };
    
    })(Drupal, jQuery);
    

    It's wishful thinking, but it'd be nice if this used native web Event Listeners rather than jQuery events, to reduce another need for jQuery in the large scheme of things, but it works as expected.

    I won't mark as RTBC since this needs tests, but this is definitely something I could use in my current projects.

    Would it be possible to roll this for 10.1? Is there a reason it must wait til 11?

  • πŸ‡§πŸ‡·Brazil igoragatti

    Re-rolled patch to work with Drupal 10.1.7

  • last update 5 months ago
    29,722 pass
  • last update 5 months ago
    Patch Failed to Apply
  • πŸ‡ΊπŸ‡ΈUnited States jtwalters

    The current patch direction does not account for editors that do not return a Promise. This results in a JS error, like that below:

    Uncaught TypeError: Drupal.editors[format.editor].attach(...).then is not a function

    Example: Try creating an Ace Editor text format and then switching to it, and then try switching back to a CKEditor 5 type format.

  • πŸ‡ΊπŸ‡ΈUnited States jtwalters

    Re-rolled for Drupal 10.2.3 with Promise.resolve() change per #22

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 4 months ago
    29,722 pass
  • heddn Nicaragua

    Can we get these changes back into the MR? Core doesn't really use a patch workflow any more.

  • Status changed to Needs review 3 months ago
  • heddn Nicaragua

    Fixed up MR and merged #24 back into it. I've hid all the patches.

  • Pipeline finished with Failed
    3 months ago
    #140834
  • Status changed to Needs work 3 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Thanks for turning to an MR.

    Moving to NW for the tests and issue summary update.

Production build 0.69.0 2024