Problem/Motivation

Add support for Drupal\ckeditor\Plugin\CKEditorPlugin\DrupalImageCaption which allows captioning and aligning images.

Proposed resolution

Remaining tasks

User interface changes

CommentFileSizeAuthor
#7 drupal-rendered.png426.76 KBzrpnr
#7 inline-ui-editing.png81.26 KBzrpnr
#7 inline-ui.png112.11 KBzrpnr

Issue fork ckeditor5-3201646

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

lauriii created an issue. See original summary.

zrpnr’s picture

Status: Active » Postponed

We may want to postpone this until we discuss with @reinmar, on the call this morning he said CK was planning to rework the image plugins.

Reinmar’s picture

The feature branch for inline images is here: https://github.com/ckeditor/ckeditor5/tree/i/2052-inline-images

The epic ticket is here: https://github.com/ckeditor/ckeditor5/issues/2052. It seems mostly dead as GH doesn't show sub tickets there, but I asked the team to start maintaining it.

I think it's worth to first check how the upgrade of DrupalImage will look like. Then we could review if it makes sense for you to rely on ImageCaption.

wim leers’s picture

Status: Postponed » Needs work

@zrpnr I think this means this is unblocked — we need your analysis here 😊

zrpnr’s picture

Thanks for the update @Reinmar, I'm still looking into this but it seems like the InlineImage is going to be really useful. It's more like how images are currently handled in Drupal. One small but tricky issue we ran into earlier was with saving just the img alone, without it being wrapped in a figure tag.

@Wim Leers could explain this better I'm sure- but Drupal uses a data attribute for an image caption, then this attribute is converted to markup when the page renders. If we're trying to preserve how Drupal currently renders images, then we'd want to save an InlineImage that has a data-caption attribute.

Since the ImageCaption plugin only applies to ImageBlock and adds an actual caption element, I'd expect to need to write a custom plugin that adds `data-caption` to the InlineImage model, and some way to edit that caption in the Editor.
We also would want to re-use this feature on elements since they follow the same pattern.

zrpnr’s picture

Issue summary: View changes
Status: Needs work » Active
StatusFileSize
new112.11 KB
new81.26 KB
new426.76 KB

Created a plugin at https://github.com/zrpnr/ckeditor5-data-caption
This adds the data-caption attribute to both image and imageInline, and uses a balloon UI like the alt text button to edit the caption, since Drupal only accepts a string value and not markup.

selecting an inline image:
inline image caption button

editing the caption:
inline image caption form

rendered display:
rendered page showing images with captions

I rebuilt all the js dll files from the i/2052-inline-images branch, and opened a new branch for the ckeditor5-drupal-image project as well so it could support imageInline.
https://github.com/zrpnr/ckeditor5-drupal-image/pull/2

I'm not sure what to do about the html filter in Drupal it was hiding the rendered entirely, when I was testing with CK4 as well.

zrpnr’s picture

After talking with @reinmar and @Wim Leers I realized I was mistaken about the purpose of imageInline, and that Drupal is more likely better served by regular block images.
CKEditor 5 is able to detect whether the image has been inserted "inline" or as a block and creates the appropriate element. For maintaining existing markup it's best that Drupal supports both.

For captions, the CKEditor 5 plugin does not support imageInline, and I think although Drupal can technically render a caption we shouldn't add that in the UI. I updated the data-caption plugin to only be enabled for block images, not images that are inserted in between text in a paragraph for example. The toolbar button will be disabled for imageInline.

To make this work for drupal-media, I added a toolbar like imageToolbar for drupalMedia
https://github.com/zrpnr/ckeditor5-drupal-media/pull/1

There may be a better way to handle dependencies but I ran into an issue where data-caption.js loaded on the page before drupalMedia so I kept the schema changes in each respective plugin- drupalimage and drupalmedia and left data-caption handling just the UI.

wim leers’s picture

For captions, the CKEditor 5 plugin does not support imageInline, and I think although Drupal can technically render a caption we shouldn't add that in the UI.

+1

Drupal 8/CKEditor 4 also only ever did this for block-level images.

I updated the data-caption plugin to only be enabled for block images, not images that are inserted in between text in a paragraph for example. The toolbar button will be disabled for imageInline.

👏

This is a net improvement over Drupal 8/CKEditor 4!

To make this work for drupal-media, I added a toolbar like imageToolbar for drupalMedia
https://github.com/zrpnr/ckeditor5-drupal-media/pull/1

There may be a better way to handle dependencies but I ran into an issue where data-caption.js loaded on the page before drupalMedia so I kept the schema changes in each respective plugin- drupalimage and drupalmedia and left data-caption handling just the UI.

Isn't that out of scope here? The issue summary clearly says Add support for Drupal\ckeditor\Plugin\CKEditorPlugin\DrupalImageCaption which allows captioning and aligning images.

Captioning embedded media is a different beast — also in Drupal 8/CKEditor 4.

wim leers’s picture

… which makes me ask: what is the next step here? Is it a thorough review of the merge request? Or are there still things you need to address?

(Asking because this is not marked as Needs review.)

bnjmnm made their first commit to this issue’s fork.

bnjmnm’s picture

I updated the MR to use the custom data-caption plugin from @zrpnr's repo.

This works within CKEditor 5's UI, but the captions are not part of the rendered page. Wasn't quite sure how to go about fixing that, so I thought I'd write a test showing that captions added to an uploaded image do not show up on the rendered page. Unfortunately, I ran into a snag on that as well figuring out how to upload an image within CKEditor 5 in a FunctionalJavascript test. (I've done this plenty of times in Media Library, but this is different as it's not using a Form API file field).

wim leers’s picture

but the captions are not part of the rendered page. Wasn't quite sure how to go about fixing that

Huh?! 🤪 Do you mean it won't show up in the filtered output? If that's indeed not happening:

  1. Are you sure you configured the filter_html filter to allow data-caption?
  2. filter_caption must run after filter_html — see core/profiles/standard/config/install/filter.format.basic_html.yml for an example

well figuring out how to upload an image within CKEditor 5 in a FunctionalJavascript test

Does that mean we do not have test coverage for this yet? Because in the end it is "just" a controller to send data to in a HTTP request?

bnjmnm’s picture

The following from #14 isn't entirely accurate upon re-read

This works within CKEditor 5's UI, but the captions are not part of the rendered page

The controls to add a caption are available, but the value I enter into it either isn't stored or is stored but not retreived. If I save content with an image with alt text and a caption, then return to edit it, the alt text is still there, but not the caption, so I don't think the value is there at all, as opposed to being stripped on render. It's quite possible this is a simple implementation detail I'm just not familiar with.

Does that mean we do not have test coverage for this yet? Because in the end it is "just" a controller to send data to in a HTTP request?

There is \Drupal\Tests\ckeditor5\Functional\ImageUploadTest, but there isn't a FunctionalJavascript that tests uploading within the editor as a user would do. For the purposes of this issue, i just want to get it to the point where a test can add captions (and alt text for that matter) in the UI the way a user would, so if there's a way to provide that without going through the UI steps that's fine too.

wim leers’s picture

but not retreived.

By what? By \Drupal\filter\Plugin\Filter\FilterCaption?

Or are you talking solely about CKEditor 5?

Does the caption make it into the database?

See \Drupal\Tests\filter\Kernel\FilterKernelTest::testCaptionFilter() for how it's supposed to behave.

but there isn't a FunctionalJavascript that tests uploading within the editor as a user would do

I see!

so if there's a way to provide that without going through the UI steps that's fine too.

There is:

Node::create([
  'body' => [
    'format' => …,
    'value => '<img src="llama.jpg" data-caption="Loquacious llama!" />',
  ],
])->save()

And then going to the edit form for that node :)

lauriii’s picture

Status: Active » Needs review

This is ready for review. I created new PR since the approach has changed significantly because CKEditor refactored their image plugin, and made it much easier for us to implement this.

The implementation is based very closely on the example that CKEditor team provided us https://github.com/ckeditor/ckeditor5/compare/ci/763. 🎉

wim leers’s picture

Status: Needs review » Needs work

Tested this thoroughly. Works beautifully! 🤩

We will need a follow-up to make formatting inside the caption work though — but that's an upstream bug that we were warned about. Quoting @Reinmar from Slack:

The PoC of drupalimage implementation is still in progress. I heard it’s close to be finished but then we found that in CKE4 inline formatting inside captions can be stored in data-caption attributes. This is not that straightforward now.


I've addressed my own remarks. The handful of commits I pushed refactor the caption support out into a separate CKEditor 5 definition that is enabled conditionally when the filter_caption is enabled. This is important because it allows us to have CKEditor 5 automatically enable the data-caption attribute only when actually necessary, and not always.

I'm pretty sure you'll like those commits, @lauriii, but please do double check!


Finally: the issue title says "caption and align". But this issue/MR only is doing the "caption" part. Perhaps it makes sense to spin that off into an issue of its own too?

lauriii’s picture

Title: Add support for image caption and align » Add support for image caption
Status: Needs work » Needs review

Opened separate issue for the image align: #3222755: Add support for image align (<img data-align>).

wim leers’s picture

Title: Add support for image caption » Add support for image caption (<img data-caption>)
Related issues: +#3222755: Add support for image align (<img data-align>)
wim leers’s picture

Status: Needs review » Reviewed & tested by the community

I did a final pass. Once https://git.drupalcode.org/project/ckeditor5/-/merge_requests/62/diffs#n... is addressed, this is good to go! 🥳

  • lauriii committed fd9d6f6 on 1.0.x
    Issue #3201646 by lauriii, Wim Leers, zrpnr, bnjmnm, Reinmar: Add...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Thank you all! Merged 🎉

wim leers’s picture

wim leers’s picture

Status: Fixed » Closed (fixed)

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

hungry_mind’s picture

Hello, I just want to add captions to CKEditor 5 images in Drupal 9.5.9. that are inserted with the "Insert image" button. Is this functionality built-in in Drupal 9? Is there a summary of a solution somewhere? Is there a module that I can just go ahead and install without having to go through all the comments in posts such as this?

Thanks
ok sorry, I hadn't realized there was a caption option when clicking on the image. Solved.