Add ability to insert Media inline in CKEditor widget

Created on 3 September 2020, over 4 years ago
Updated 14 March 2023, about 2 years ago

Problem/Motivation

At now Drupal Media entities can be inserted into CKEditor, using Media Library widget, only as block element. So there are no ways to insert inline image (eg small icon image into text), or document (inline link to download file inside text paragraph).

Proposed resolution

Add "Inline" variant to widget align property, and convert CKEditor widget from block to inline, if selected.

Remaining tasks

Discuss the UX, finish the patch.

User interface changes

Added "Inline" option to media align property.

API changes

None.

Data model changes

None.

Release notes snippet

TBD

Feature request
Status

Needs work

Version

10.1

Component
Media 

Last updated 2 days ago

Created by

🇦🇲Armenia murz Yerevan, Armenia

Live updates comments and jobs are added and updated live.

Missing content requested by

🇦🇺Australia dpi
over 1 year ago
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @murz
  • 🇦🇲Armenia murz Yerevan, Armenia

    Here is my patch with first quick implementations of this idea. It adds "Inline" option to align property, and converts CKEditor widget to inline, if selected, and back, if deselected. Will be good to discuss this idea, after this I ready to complete the patch.

    Now feature is implemented only in plugin.js, without duplicating changes into plugin.es6.js. After finish implementing, I will duplicate all into es6 file too.

  • 🇦🇲Armenia murz Yerevan, Armenia
  • 🇦🇲Armenia murz Yerevan, Armenia

    Here is screenshots of interface.

  • 🇩🇪Germany anruether Bonn

    Maybe you find the discussion in the entity embed issue queue helpful.

  • 🇦🇲Armenia murz Yerevan, Armenia
  • 🇺🇸United States phenaproxima Massachusetts
  • Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle .

  • 🇫🇷France sylvain lavielle

    I tested the #2 patch, but have a strange behavior : When I insert an inline document, all looks great until I click the CKEditor "source" button to edit the HTML source. When I do it, the source does not shows up and a JS error located in CK editor is thrown on the JS console. Another related side effect shows up when I save the document. The modifications made inside the WYSIWYG field are not saved, (I think, it's because CKEditor is not able to generate the modified HTML as when I click on the "source" button).

    If I chose to insert a "block" style alignment instead of the "inline" one. All works perfectly well as before.

    Is there someone having the same problem ? any idea about the "whyness" :)

  • 🇮🇳India vsujeetkumar Delhi

    Re-roll patch created, Please check.

  • 🇮🇳India vsujeetkumar Delhi

    I tested the #2 patch and while you click on source in ckeditor, In second time it will show the below error.

    "Uncaught TypeError: Cannot read property 'attributes' of null"

    @Murz So I updated the patch and try to fix the above issue, Please check and help me to fix this issue.

  • Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle .

  • 🇫🇮Finland kari.kaariainen

    The "Inline" option appears in the Edit media dialog, but it has no effect.

    If I manually make my media inline:

    <p>Lorem <drupal-media data-align="inline" data-entity-type="media" data-entity-uuid="51327ba3-eff6-4e22-a5dc-68d85cbc4ebd" data-view-mode=""></drupal-media> ipsum.</p>
    

    CKEditor breaks it into this:

    <p>Lorem</p>
    <drupal-media data-align="inline" data-entity-type="media" data-entity-uuid="51327ba3-eff6-4e22-a5dc-68d85cbc4ebd" data-view-mode=""></drupal-media>
    <p>ipsum</p>
    

    Also, the data-view-mode has appeared and is empty data-view-mode="">. Is that supposed to be like that?

  • Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle .

  • 🇫🇷France steveoriol Grenoble 🇫🇷

    CKeditor always separates insertion from a media "drupal-media" between "p" tags...

    I also try to use, even if it is less convenient for the end customer than the use of "Media Library", the entity insertion (with the "entity_embed" module with this patch Allow elements to be inline CKEditor Widgets Needs review ).
    But it's the same problem for me, the "p" tag always separates the insertions ...

    Is there anything special to do in the format configuration "admin/config/content/formats/manage/xxxxx" to make these patches work?

  • Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle .

  • I tried the patch to add inline media, but the content is not aligned inline

  • 🇺🇸United States bob.hinrichs

    Same issue as #14. The text filter on ckeditor forces p tags around the text around the media tag.

  • 🇩🇪Germany tfranz

    I think it has something to do with ckeditor 4 not respecting HTML5 (DIV-Element inside a P-Element).
    You can't even insert something like that:
    <p>Hello <div>world</div>!</p>

    Maybe it can be solved by changing the way the editor "tidies up" the source-code:
    https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dtd.html

    ... but i couldn't find out how ... :-(

    There is a discussion on SO about the same behaviour:
    https://stackoverflow.com/questions/19825802/how-to-configure-ckeditor-t...

    Tested it with the experimental module for ckeditor 5 and it didn't work either.

  • Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle .

  • 🇩🇪Germany Tilo Schumann

    For me worked to add in module/contrib/ckeditor/js/plugins/drupalmedia/plugin.js

          dtd['p']['drupal-media'] = 1;
    

    as proposed for same problem with drupalentity in:

    https://www.drupal.org/project/entity_embed/issues/2892820#comment-12849509

    No idea which side effects will occur ...

  • Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened , as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle .

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    29,418 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    30,318 pass, 1 fail
  • I'm not sure if I should open a new issue for this for CKEditor5 (in which this is still a problem), but as 10.x is only using ckeditor5 now, I think it's fine to post a fix for this for here. The following patch works for me on 9.5.x and 10.1.x.

  • 🇩🇪Germany lissy

    Thanks. The Patch did the trick for me.

  • Status changed to Needs work over 1 year ago
  • 🇳🇱Netherlands perry.franken

    This patch does the trick. The only problem i now have is when i use my arrow keys to go over the link i see a link button appears. Does anyone know how to fix this?

  • 🇩🇪Germany lissy

    Hi Perry, I think, you can change that in the CSS of Admin Theme. I have a subthem of claro.
    But I did not observe this effekt with the link button jet.

  • 🇺🇸United States glynster

    This is huge and with the patch solves many trivial issues. RTBC +1.

  • I think the patch posted above may be insufficient. It does work in some cases like for example having a media embed inside a <li> tags. But in other cases like <p> it doesn't work. In case of

    the markup inside the editor suggests that it would work as <drupal-media> is displayed inside the <p> tag, but when saving this structure and inspecting the markup, the <p> tags are closed before the opening <drupal-media>.

    For example, inside the editor the source looks like

    <p>
        This image should display inlin<drupal-media class="image_resized" style="width:284px;" data-media-width="284px" data-entity-type="media" data-entity-uuid="97248952-ca0c-4383-92ec-0e5cde61ec9c" data-align="center">&nbsp;</drupal-media>e inside the &lt;p&gt; tag
    </p>

    whereas in the rendered content after saving, it looks like this:

    <p>This image should display inline</p>
    <div
      class="image_resized align-center media media--type-image media--view-mode-cke-media-resize-medium">
      <div
        class="field field--name-field-media-image field--type-image field--label-visually_hidden">
        <div class="field__label visually-hidden">Image</div>
        <div class="field__item">
          <img loading="lazy"
            src="/sites/default/files/styles/cke_media_resize_medium/public/2023-08/pexels-clive-kim-4220967_4.jpg?itok=X2-WwcKn"
            width="500" height="750" alt="asdd"></div>
      </div>
    </div>
    inside the &lt;p&gt; tag<p></p>
    
  • Re-roll after update or core introducing CKEditor5 v39.0.0

  • 🇺🇸United States glynster

    @s_leu your patch does not apply to Drupal 10.1.2, however #23 still does and works as expected.

  • 🇺🇸United States glynster

    @s_leu your patch does apply to Drupal 10.1.3, so we are set! RTBC +1

  • 🇨🇦Canada boquistm

    I'm having trouble getting the inline option to appear, maybe just a config option I can't find. Any tips greatly apreciated.
    - Drupal 10.1.3 with patch from #30 applied
    - using 'Insert Media' button
    - clicking the document shows alignment options, but no inline option. See attached image:

  • 🇺🇸United States iamsauder

    @boquistm - I am seeing the same thing with a document link. The patch does make the ckeditor media widget act line an inline element - you can insert the widget inside another html tag and drag it around text - but the link itself is still wrapped by a <div> tag and is thus displaying as a block element in the rendered html.

    - Drupal 10.1.4 with patch from #30 applied

    Is there a patch file for the source Javascript files available to contribute to?

  • 🇺🇸United States glynster

    I think with the latest Drupal release this patch is no longer needed? Drupal 10.1.5. Also the patch does not apply either.

  • Glynster is right, the fix in 🐛 [DrupalMedia] Formatting lost when attempting to edit media within a list item in CKEditor 5 Fixed seems to make the patch here obsolete. However anybody still on D9 can use the patch here or the patch there, they both seem to work for lists.

  • 🇺🇸United States brad.bulger

    I'm not sure what you all mean when you say the patch is not needed? The basic issue of not being able to insert a media entity image inline still exists in 10.1.x-dev. The image button has an "in line" setting but not Insert Media.

  • 🇺🇸United States sclsweb

    Seconding what brad.bulger said in #37. This issue is not at all fixed in 10.1.5. For clarity, in order to consider this "fixed," these are the use cases that should be supported (am I missing any?):

    • Multiple media images can sit together on the same line, centered, and it should be possible to put a cursor in and insert text/spaces between them
    • A media image can be inserted and sit between two words in the same line of text
    • A media image can be added inside a list item WITHOUT CKEditor putting the text of the list item inside a paragraph tag inside the list item ( 🐛 [DrupalMedia] Formatting lost when attempting to edit media within a list item in CKEditor 5 Fixed fails this; it adds a paragraph tag around the text inside the list item)
    • Any inline media image should be able to be linked or un-linked without adding paragraph tags in the markup or having the media image lose its inline positioning
  • 🇺🇸United States glynster

    @brad.bulger and @sclsweb does the patch cover all those issues? As it stands now the patch no longer applies as this was updated in core: 🐛 [DrupalMedia] Formatting lost when attempting to edit media within a list item in CKEditor 5 Fixed . I assume we need a re-roll?

  • 🇺🇸United States sclsweb

    I never tested the original patch, so I'm not sure if it covered all the issues. If someone re-rolls I can test on 10.1.5.

  • 🇺🇸United States iamsauder

    I was using patch #30 and it was doing all of those things. 10.1.5 breaks the patch and doesn't fully replicate the functionality this patch provided. 10.1.5 still breaks if you drag the media item around and doesn't solve the issue of embedding a media inline with text (ex a photo with a paragraph around it like a newspaper layout)

  • 🇨🇭Switzerland lionel rousseau Geneva

    @glynster

    • Yes, multiple media images can sit together on the same line

      center-alignment can be done via text alignment (so the center-align button on the media could be removed,

      not possible to put a cursor in and insert text/spaces between them
    • Yes, A media image can be inserted and sit between two words in the same line of text:
    • Yes, A media image can be added inside a list item WITHOUT CKEditor putting the text of the list item inside a paragraph tag
    • Yes, inline media image is able to be linked or un-linked without adding paragraph tags
  • 🇺🇸United States chrisgross

    I can confirm that this issue still exists in 10.1.6 and the most recent patch does not apply.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 5.3 & MySQL 5.5
    last update over 1 year ago
    Patch Failed to Apply
  • First commit to issue fork.
  • Merge request !5566Issue 3168966: re-applied changes on top of 10.1.x → (Open) created by euk
  • Here's a patch that seems to work for us against latest 10.1.x. This is based partly on the diff from the MR put in by @ekubovsky, but we made some changes because of AJAX/JS errors we were getting from Drupal media not being set as a CKEditor object. I'm leaving this in needs work, though, because, it seems that now all media is being inserted inline, which may not be ideal. IMO, the actual solution would look something like the native CKEditor Image plugin, which glues together the ImageInline and ImageBlock plugins and can be set to be used to auto determine which plugin to use based on context. The LoE on that seems high, though, and it'd probably be best to get input from core media subsystem maintainers before proceeding.

  • 🇺🇸United States chrisgross

    #46 applies for me, but it doesn't seem to do anything. No images are inserted inline at all.

    I just tried out the CKE5 live demo on their website and media alignment just works. Shouldn't Drupal already support all of CKE5's basic native functions out of the box in order to be the default editor that every single Drupal sites uses? Maybe this is off-topic, but it seems irresponsible to have included this new version as the native editor at all if that is not the case.

  • 🇺🇸United States euk

    I guess there are two sides to this issue: CKEditor and format filter. FOr the CKEditor - that's the fix in the patch, it looks ugly in the editor still, but at least the media embed is placed inline and doesn't break the <p>. On the filter side - i believe you have to make sure our image is rendered without any block-level tags. Also Auto-p seems to be messing up with the render as well - we used additional patch to remove any new lines from the markup the media embed generates.

  • 🇺🇸United States chrisgross

    Okay, I think I understand how the patch works. Images can be now embedded "inline" insofar as they do not break lists within the editor itself and can be dragged around and placed back in lists. But there is still no true inline option for rendering images within text, which I guess is the bigger issue, as this is a standard CKE5 feature.

  • 🇺🇸United States chrisgross

    Actually, after testing this patch #46 more, it seems to create more problems than it solves. For some reason, with the current state of D10/CKE5, media embedded within lists get shoved inside additional list element, which is confusing to editors.

    Before this patch, I could theme around this by hiding the list styles and margins in my theme, but after the patch, embedded media has wildly inconsistent behavior.

    When inserting media in a list, the media gets inserted INSIDE a 'ck-list-bogus-paragraph' element. After saving the content and returning, it is now appearing AFTER the 'ck-list-bogus-paragraph' element, and so the list styles cannot be removed with CSS.

  • s_leu changed the visibility of the branch 3168966-add-ability-to to hidden.

  • s_leu changed the visibility of the branch 3168966-add-ability-to to active.

  • s_leu changed the visibility of the branch 3168966-add-ability-to to hidden.

  • Status changed to Needs review over 1 year ago
  • I had another stab at this inside the new MR as the patch I uploaded initially didn't really work for all media as reported in #33 and was an only an incomplete/partial solution.

    As mentioned in #49, the other MR wasn't cutting it either, so I hid that one for now. A complete solution is only possible by adding an additional schema model for inline media in the ckeditor5 plugin code, because a block element won't ever really work completely in all inline elements inside the CKE5 DOM unless we convert it entirely to an inline element. A complete conversion however would also remove the ability to insert media embeds as block elements, which certainly has its use-cases too. Thus a complete solution is only possible by having two different ckeditor schema models for embedded media, one for embeds that should be blocks and a dedicated one for inline usage. This is also how ckeditor5's image plugin works, see the definition of the inline image and it's counterpart, the block image.

    The new MR addresses this, adds corresponding styling and also handles this not only inside the editor but also when rendering the markup saved by the editor via the EmbedMedia filter.

    As for inline media, the media entity itself will not get rendered as that would produce a lot of block wrapper markup (<div>) that would have to be converted to flow content containable markup in order to ensure correct rendering of media inside inline elements such as for example <p>.
    So the MR applies corresponding changes to the filter in order to keep the markup at a minimum, as a result, only the source field of a media will be rendered by the filter.

    The source field will be rendered in a new dedicated view mode ckeditor_inline which can be configured per media bundle to customize how the inline media gets rendered.

    If anybody wants to add or change stuff, could you please the MR and not upload patches? That will make it much easier to keep track of changes.

    Btw. I don't know how I can see the the linting errors that cause the pipelines on gitlab to fail, if someone can point me into the right direction here, I can address this.

  • Issue was unassigned.
  • Status changed to Needs work over 1 year ago
  • @s_leu This is nice work! I haven't tested yet, but this is along the lines of what I was suggesting in #46. One quick thing I noticed:
    Instead of overwriting the build array contents with only the source field render array in \Drupal\media\Plugin\Filter\MediaEmbed::process(), have you considered creating a media--ckeditor-inline.twig.html<code> or <code>media--image--ckeditor-inline.twig.html template with inline tags instead? I think the way you have it now, cacheable metadata from the media entity won't bubble up, so if you made the changes to alt text, the change would not be reflected in the rendered embed until after a separate cache clear. Also, using a template could possibly allow people to override and insert an inline caption, for example.

    Btw. I don't know how I can see the the linting errors that cause the pipelines on gitlab to fail, if someone can point me into the right direction here, I can address this.

    It's a bit opaque, but when you click to view the failing "JavaScript linting (eslint)" pipeline, click on the "Artifacts" link in the right sidebar. Open the row for the failing test and download the "junit.xml.gz" file. Extract and view. There are junit.xml viewers online to make the results more human-readable if needed.

    Also I've hidden the patch files and set the version to 11.x-dev, because for the work to go into core, it needs to go to 11.x first and get backported back to 10, though I'm not sure it will get back ported to 10.1.

    Next steps:

    • Fix linting issues
    • Set MR target branch to 11.x
    • Add automated tests
    • Update issue summary to reflect approach taken
    • Get media subsystem maintainer sign off
  • Pipeline finished with Failed
    about 1 year ago
    Total: 295s
    #70865
  • Pipeline finished with Failed
    about 1 year ago
    #70880
  • Pipeline finished with Failed
    about 1 year ago
    Total: 1047s
    #70884
  • Another update on this: I fixed the linting errors and changed the code such that it'll render the entire media instead of only the source fields as I did prior to the latest commit in the MR.

    have you considered creating a media--ckeditor-inline.twig.html or media--image--ckeditor-inline.twig.html template with inline tags instead?

    Yes I did consider that too, but rendering the field only made more sense to me as it was less markup to convert. The concerns you mentioned are valid though, so I changed it now. I wasn't quite sure if those templates and the new preprocess hook should really live inside the ckeditor module, but having ckeditor specific templates inside the media module makes less sense I think.

    This is as far as I can go with this for now, fixing the tests, writing new ones and re-rolling against 11.x should happen in the MR.

  • Pipeline finished with Failed
    about 1 year ago
    Total: 990s
    #70911
  • Pipeline finished with Failed
    about 1 year ago
    Total: 1002s
    #70995
  • Switched the MR target branch to 11.x after cherry-picking the changes over, a rebase didn't really work any more here.

  • Pipeline finished with Failed
    about 1 year ago
    Total: 497s
    #72080
  • Pipeline finished with Failed
    about 1 year ago
    Total: 521s
    #72079
  • Pipeline finished with Failed
    about 1 year ago
    Total: 541s
    #75505
  • Pipeline finished with Failed
    about 1 year ago
    Total: 557s
    #75569
  • Pipeline finished with Canceled
    about 1 year ago
    #75976
  • Pipeline finished with Failed
    about 1 year ago
    Total: 1102s
    #75978
  • Pipeline finished with Failed
    about 1 year ago
    #77330
  • Pipeline finished with Failed
    about 1 year ago
    Total: 161s
    #77801
  • Pipeline finished with Failed
    about 1 year ago
    #77811
  • Pipeline finished with Failed
    about 1 year ago
    Total: 184s
    #77814
  • Pipeline finished with Failed
    about 1 year ago
    Total: 186s
    #88381
  • Pipeline finished with Failed
    about 1 year ago
    #88384
  • Pipeline finished with Canceled
    about 1 year ago
    Total: 215s
    #88398
  • Pipeline finished with Failed
    about 1 year ago
    Total: 693s
    #88402
  • Pipeline finished with Failed
    about 1 year ago
    Total: 712s
    #88407
  • Pipeline finished with Failed
    about 1 year ago
    Total: 676s
    #88413
  • Pipeline finished with Failed
    about 1 year ago
    Total: 587s
    #88421
  • Pipeline finished with Canceled
    about 1 year ago
    Total: 423s
    #88427
  • Pipeline finished with Failed
    about 1 year ago
    Total: 555s
    #88431
  • Pipeline finished with Failed
    about 1 year ago
    Total: 497s
    #88445
  • Pipeline finished with Failed
    about 1 year ago
    Total: 527s
    #88512
  • Pipeline finished with Canceled
    about 1 year ago
    Total: 365s
    #88531
  • Pipeline finished with Success
    about 1 year ago
    Total: 456s
    #88536
  • Pipeline finished with Success
    about 1 year ago
    Total: 577s
    #88555
  • Pipeline finished with Success
    about 1 year ago
    Total: 507s
    #88634
  • Status changed to Needs review about 1 year ago
  • This is ready for review again now. The MR is changed to target 11.x, further I fixed existing tests and added some basic test coverage of the new feature. The IS is updated as well.

    Please note that the ckeditor plugins for captions and alignment are currenctly not supported for inline media, namely these:

    1. ckeditor5_drupalMediaCaption
    2. media_mediaAlign

    I think this can happen in a follow up and doesn't have to be a blocker here as this patch is already large enough as it is now.

  • Pipeline finished with Canceled
    about 1 year ago
    Total: 192s
    #94441
  • Pipeline finished with Success
    about 1 year ago
    Total: 609s
    #94442
  • Status changed to Needs work about 1 year ago
  • 🇺🇸United States smustgrave

    There are a number of patches and about 7 branches up, can those be cleaned up/hidden please.

  • godotislate changed the visibility of the branch 3168966_cke-inline-media-11.x to hidden.

  • godotislate changed the visibility of the branch 3392621_3168966_combined_10.1.x to hidden.

  • godotislate changed the visibility of the branch 3392621_3168966_combined_11.x to hidden.

  • godotislate changed the visibility of the branch 3168966_tests to hidden.

  • godotislate changed the visibility of the branch 3168966_cke-inline-media_10.2.x to hidden.

  • godotislate changed the visibility of the branch 3168966_cke-inline-media-10.1.x to hidden.

  • Hid patches and branches other than MR 5758, which seems to have the latest work in it. Though look like it has a merge conflict, so leaving at Needs work.

  • 🇺🇸United States rex.barkdoll

    rex.barkdoll changed the visibility of the branch 3168966_cke-inline-media-10.1.x to active.

  • 🇺🇸United States rex.barkdoll

    rex.barkdoll changed the visibility of the branch 3168966_cke-inline-media-10.1.x to active.

  • 🇺🇸United States rex.barkdoll

    rex.barkdoll changed the visibility of the branch 3168966_cke-inline-media-10.1.x to hidden.

  • 🇨🇦Canada Shiraz Dindar Sooke, BC

    Hi folks,

    I'm just joining this issue now.

    I successfully applied the latest MR 5758 and rebuilt the cache but I'm not seeing any options for inline images as I see in screenshots pasted here. Nor do I see any new buttons or new configuration in the text format for the embed media filter.

    Is there something else I need to do?

    Thanks,
    Shiraz

  • Pipeline finished with Failed
    12 months ago
    Total: 175s
    #130405
  • Status changed to Needs review 12 months ago
  • Pipeline finished with Failed
    12 months ago
    Total: 331s
    #135005
  • Ok I'm not sure what's going on here in the Validatable config step of the merge request pipeline.

    The code in the MR adds a post update hook which installs a view mode config and apparently that differs to the output of the drush config:inspect --statistics command which is ran after installing the standard profile first and then again when checking out the branch and running update hooks. So at this point of course config will differ as the view mode got installed. What I don't understand here is why this would be a reason to stop the pipeline? I mean it's legitimate (and sometimes necessary) for a module to install new config in an update hook, isn't it?

    I also tried to add that config to the optional config of the standard profile, but the error causing the fail persisted. Can someone point out why this happens and what we need to do to fix this? This didn't happen a while ago when the merge request pipeline was green..

  • Pipeline finished with Failed
    12 months ago
    Total: 632s
    #135010
  • Pipeline finished with Failed
    12 months ago
    Total: 603s
    #135020
  • Pipeline finished with Success
    12 months ago
    Total: 712s
    #135027
  • 🇺🇸United States carsoncho Kansas City, MO

    carsoncho changed the visibility of the branch 3392621_3168966_combined_10.1.x to active.

  • 🇺🇸United States carsoncho Kansas City, MO

    carsoncho changed the visibility of the branch 3392621_3168966_combined_10.1.x to hidden.

  • 🇨🇦Canada Shiraz Dindar Sooke, BC

    Hi I just wanted to chime in here to say that an alternative to this patch would be to define a display mode for your image which is styled to show as an inline-block.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Hi I just wanted to chime in here to say that an alternative to this patch would be to define a display mode for your image which is styled to show as an inline-block.

    AFAIK that doesn't work, as CKe5 changes the structure of the markup such that the embedded media becomes a block element of its own, not embedded inside e.g. a paragraph.

    Example: I would need something like this:

    <p>This is some text and contains a logo <drupal-media ....DATA OF THE EMBEDDED IMAGE.../> and some more text.</p>
    

    This gets stored like this:

    <p>This is some text and contains a logo</p>
    <drupal-media ....DATA OF THE EMBEDDED IMAGE.../>
    <p>and some more text.</p>
    

    This can't be fixed with CSS.

  • 🇺🇸United States ksenzee Washington state

    AFAIK that doesn't work, as CKe5 changes the structure of the markup such that the embedded media becomes a block element of its own, not embedded inside e.g. a paragraph.

    This is exactly right. Without this patch, CKE5 will helpfully "correct" your markup to make sure the embedded media is a block element.

    Shiraz: You have to get the new optional config installed for this patch to do anything. If you run the post-update hooks manually that should take care of it.

  • Status changed to Needs work 11 months 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 necessarily 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.

  • Pipeline finished with Success
    11 months ago
    Total: 625s
    #157116
  • 🇺🇸United States ksenzee Washington state

    I very much need this feature—many thanks for all the work on it thus far. It works well on both 10.1 and 11.x. This is the first time I've ventured into the shark-infested waters of CKE5 development, so weigh my opinion accordingly, but I think this needs GHS integration. If you add arbitrary attributes to a <drupal-media> element, they’re preserved (as long as you’re using Full HTML, or another text format without filter_html enabled), because GHS supports <drupal-media>. If you add arbitrary attributes to a <drupal-media-inline> attribute, they get removed, because GHS doesn’t know about <drupal-media-inline>.

    I rebased the MR, and I’ll work on adding GHS support.

  • Pipeline finished with Failed
    11 months ago
    Total: 165s
    #157895
  • Status changed to Needs review 11 months ago
  • 🇺🇸United States ksenzee Washington state
  • Pipeline finished with Failed
    11 months ago
    Total: 508s
    #157958
  • First commit to issue fork.
  • Pipeline finished with Success
    11 months ago
    Total: 3032s
    #159647
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I gave this MR a test drive today and got a bit confused about how it is supposed to work. Here's what I did:

    • Checked out the MR
    • Installed Standard
    • Installed Media library
    • Enabled the media library button in the CKEditor on the Full HTML text format
    • Configured the Media filter to allow the user to select the new "Ckeditor Inline" view mode and allowed the user to choose the view mode (I now realise this is not how this is supposed to work)
    • And added an article... went first to the media button and added an document and tried to add it inline. This didn't work even if I added a sentence.
    • Then I realised (after some time) then if I added a sentence to the article an use the media library to add the document to that sentence it would add it inline. (Nice)

    But now that inline image is stuck to being inline and if I drag it to a new block it doesn't automatically convert to a block. Am I doing something wrong?

    I think there needs to be an easy way to flick between an inline and block media embed.

  • Pipeline finished with Success
    11 months ago
    #159699
  • I wonder if there actually might need to be 3 drupal media plugins like CKEditor5's Image plugin - where there is Image that acts as a "glue" plugin for ImageBlock and ImageInline.

  • Status changed to Needs work 11 months 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 necessarily 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.

  • Pipeline finished with Success
    11 months ago
    Total: 953s
    #162114
  • 🇺🇸United States ksenzee Washington state

    I agree that adding support for dragging between block and inline would make this more intuitive to use. I don't think the built-in Image plugin handles block-to-inline conversion perfectly (try dragging from block to inline—I can't make it work). But we could copy what they're doing and get to feature parity at least.

    Another missing feature here is the contextual toolbar that's supposed to pop up when you select a media item. Right now the toolbar doesn't appear because it's behind a conditional that doesn't recognize <drupal-media-inline> as a Drupal media item, which is because the functions in drupalMedia/src/utils.js don't recognize that <drupal-media-inline> exists. They'll need to be reworked, with a BC layer.

    Also, as s_leu mentioned in #63, the plugins for captions and alignment don't work, even if you fix the toolbar visibility so that the caption and alignment buttons appear. Keeping them disabled might actually be the right call, though: I'm not sure how a caption for an inline media item should look, from a markup perspective, and alignment doesn't apply to an inline element. The same thing applies to the view mode dropdown: if we're forcing a single view mode for all inline media, the dropdown shouldn't appear. (Do we have to force a single view mode, though? Any way we could designate some media view modes as inline and others as block?)

    I rebased the MR and tests are passing again, except for validatable config, and s_leu's questions about that in #77 are still valid.

  • Pipeline finished with Failed
    10 months ago
    Total: 507s
    #171260
  • 🇩🇪Germany lmoeni

    I tested the MR today and it works fine when I use the Images button (except the things discussed in the comments starting from #87).
    I cannot get it working with the media library though. Is there anything that I need to keep in mind while testing it with the media library?

  • 🇺🇸United States ksenzee Washington state

    @lmoeni Thanks for testing! It should work fine with the media library. If you've applied the patch to an existing site, make sure the new optional config has been installed. There are post-install hooks that should do it if you run a database update.

  • I cannot apply those changes on 10.2.2

  • First commit to issue fork.
  • Pipeline finished with Failed
    8 months ago
    Total: 203s
    #235186
  • 🇳🇱Netherlands koosvdkolk

    Unfortunately we could not wait for this issue to be solved. Our use case requires inline media only, so we wrote a small patch module, which turns all stuff into inline-block HTML.

    It consists of code altering CKEditor behavior and a text filter. Theme CSS is required for correct display in the front-end (see README in lb_ckeditor_media_fix.zip).

    Sharing it here as it might help some others.

    Please note that this module has a very narrow use case and is really meant as a band-aid!

  • Pipeline finished with Failed
    4 months ago
    Total: 247s
    #342085
  • Pipeline finished with Failed
    4 months ago
    Total: 136s
    #342089
  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've rebased MR !5758 since the ckeditor5_theme() got removed from Drupal 11.x but is required for the inline insertion here.

  • Pipeline finished with Failed
    2 months ago
    Total: 208s
    #382553
  • 🇺🇸United States euk

    euk changed the visibility of the branch 3168966_cke-inline-media_10.2.x to active.

  • 🇺🇸United States euk

    euk changed the visibility of the branch 3168966_cke-inline-media_10.2.x to active.

  • 🇺🇸United States euk

    euk changed the visibility of the branch 3168966_cke-inline-media_10.2.x to hidden.

  • 🇺🇸United States euk

    euk changed the visibility of the branch 3168966_cke-inline-media_rebased to hidden.

  • Pipeline finished with Failed
    24 days ago
    Total: 133s
    #429352
  • Pipeline finished with Failed
    24 days ago
    Total: 733s
    #429358
  • 🇺🇸United States euk

    I added Merge request !11248, which is a rebase of the MR5758 on top of the 11.x branch. Fixed a few pipeline issues, but few other pipeline issues remain as I am not sure where the tests are failing or the tests need to be adjusted.

    Also, I do not understand the changes entirely, so would be great if @ksenzee or @jurgenhaas could review the changes.

    I am going to apply this changes against 10.4.x since this is where I need them. 10.4.x path is coming soon (should I have success)

  • Pipeline finished with Failed
    24 days ago
    Total: 652s
    #429372
  • 🇺🇸United States euk

    euk changed the visibility of the branch 11.x to hidden.

Production build 0.71.5 2024