Problem/Motivation

If a CKEditor plugin integrates with Media to provide a custom attribute, it can no longer cause a preview re-render for the drupal-media element. In CKEditor 4 this was possible by altering the editor_media_dialog form and a little magic.

In CKEditor5 you must implement your own toolbar button or dropdown. Re-rendering of the preview is explicitly registered in https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/modules/cked...

The converter is locally scoped and the plugin does not allow adding additional attributes to listen for refresh.

Steps to reproduce

Write a CKEditor5 plugin to manipulate drupal-media, see it never refresh

Proposed resolution

Provide an API for adding additional attributes OR export the converter for other plugins to use.\

laurii pointed to https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-ima..., the problem for contrib is the quirky import (relative paths)

however, using the exported converter in a build could lead to an outdated copy, if drupal fixes it. it'd be better as a runtime dependency, which is hard. so using properties on the plugin which other plugins register may be best (copying notes from Slack with laurii)

Remaining tasks

Decide on approach, and do it

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3300246

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mglaman created an issue. See original summary.

mglaman’s picture

Issue summary: View changes
mglaman’s picture

Issue summary: View changes
mglaman’s picture

I quick tried this locally:

Modify DrupalMediaEditing to add

    this.converterAttributes = [
      'drupalMediaEntityUuid',
      'drupalElementStyleViewMode',
      'drupalMediaEntityType',
      'drupalMediaAlt',
    ];

My plugin in its init then has to do:

    const mediaEditing = editor.plugins.get('DrupalMediaEditing');
    mediaEditing.converterAttributes.push('drupalElementStyleMediaEmbedCode')
    console.log(mediaEditing.converterAttributes)
    mediaEditing._defineConverters();

It has to re-apply the converter listeners for the new attribute since everything is running in `init`. That's one reason why exporting `converter` would be better. My plugin has DrupalMediaEditing as a dependency, so init is messy.

The weird part is that everything breaks when I use:

        this.converterAttributes.forEach((attribute) => {
          dispatcher.on(attribute, converter);
        });

The media element never renders, only when I keep

        dispatcher.on(
          'attribute:drupalElementStyleMediaEmbedCode:drupalMedia',
          converter,
        );
        dispatcher.on('attribute:drupalMediaEntityUuid:drupalMedia', converter);
        dispatcher.on(
          'attribute:drupalElementStyleViewMode:drupalMedia',
          converter,
        );
        dispatcher.on('attribute:drupalMediaEntityType:drupalMedia', converter);
        dispatcher.on('attribute:drupalMediaAlt:drupalMedia', converter);
mglaman’s picture

It didn't work because I failed to build the proper event name 😅. Opening an MR based on the following:

  constructor(editor) {
    super(editor);

    this.attrs = {
      drupalMediaAlt: 'alt',
      drupalMediaEntityType: 'data-entity-type',
      drupalMediaEntityUuid: 'data-entity-uuid',
    };
    this.converterAttributes = [
      'drupalMediaEntityUuid',
      'drupalElementStyleViewMode',
      'drupalMediaEntityType',
      'drupalMediaAlt',
    ];
  }

and

        this.converterAttributes.forEach((attribute) => {
          dispatcher.on(`attribute:${attribute}:drupalMedia`, converter);
        });

This allowed our plugin, in its constructor, to add another attribute:

  constructor(editor) {
    super(editor);

    const mediaEditing = editor.plugins.get('DrupalMediaEditing');
    mediaEditing.converterAttributes.push('drupalElementStyleMediaEmbedCode')
  }

lauriii’s picture

Status: Active » Reviewed & tested by the community

The MR looks all good for me. The plugin is still marked as @private which I think is what we still expect. We can potentially officially declare this as an API later on.

Would be useful to have a 9.5.x and 9.4.x versions of the patch.

mglaman’s picture

StatusFileSize
new79.31 KB
new79.31 KB

Patches for 9.5.x and 9.4.x, if needed.

lauriii’s picture

Issue tags: +Needs reroll
wim leers’s picture

Status: Reviewed & tested by the community » Needs work

For the reroll needed since #9.

Ankit.Gupta’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new79.22 KB

Reroll the patch #8 with Drupal 10.0.x

wim leers’s picture

Title: Allow other CKEditor5 plugins to rerender drupal-media when their attribute changes » Allow other CKEditor 5 plugins to rerender drupal-media when their attribute changes
Priority: Normal » Major
Issue tags: +Contributed project blocker

Looks like #8 did not work because the CKEditor 5 version in those 2 branches was slightly different than the one in 10.0.x at that point in time.

Let's get this back on track.

First, the MR should work against 10.0.x again — it's >1.5 month out of date. CKEditor 5 has been updated since.

$ git merge origin/10.0.x
# CONFLICT in core/modules/ckeditor5/js/build/drupalMedia.js ⚠️
# Let's fix it:
cd core
yarn install
yarn build:ckeditor5
git add modules/ckeditor5/js/build/drupalMedia.js
git commit

The result is 77d2321866df0c3539d0b6bd8c4244a910de9bc7, let's see how that works out 🤓

wim leers’s picture

StatusFileSize
new79.3 KB
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

I failed to follow through here — the MR is green and the rerolled patch for 9.x is green too.

I didn't do anything besides rebasing and rerolling, so restoring the RTBC from #7.

  • lauriii committed 3a4d030 on 10.1.x
    Issue #3300246 by mglaman, Wim Leers: Allow other CKEditor 5 plugins to...

  • lauriii committed 2bdfff0 on 10.0.x
    Issue #3300246 by mglaman, Wim Leers: Allow other CKEditor 5 plugins to...

  • lauriii committed ebeffc8 on 9.5.x
    Issue #3300246 by mglaman, Wim Leers: Allow other CKEditor 5 plugins to...

  • lauriii committed 9e0f4e4 on 9.4.x
    Issue #3300246 by mglaman, Wim Leers: Allow other CKEditor 5 plugins to...
lauriii’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed the MR 3a4d030 and pushed to 10.1.x and cherry-picked to 10.0.x. Thanks!
Committed #13 ebeffc8 and pushed to 9.5.x and cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.