Add ability to insert Media inline in CKEditor widget

Created on 3 September 2020, almost 4 years ago
Updated 10 July 2024, 3 days 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

Instead of manual control via the align properties as initially proposed, the current approach in the MR is doing the using the same approach as ckeditor5's image plugin is build upon, see the definition of the inline image and it's counterpart, the block image.

So as the image plugin in ckeditor core, there will be a new model for media which is embedded inline and media which is embedded as block content, which model is used is depending on the parent element of the media. So for example a <div>parent will result in a block model, a <p> parent will cause the editor to use the inline model.

On the rendering side, this needs dedicated templates to ensure that media embedded inline is also being rendered using markup that is based on phrasing content and can be contained inside flow content.
To use these templates view mode, there is also a new view mode ckeditor_inline which enables advanced control over what is rendered in case of media embedded media.

Remaining tasks

User interface changes

API changes

  1. Clientside: New element inside the drupalMedia ckeditor plugin
  2. Serverside: New event MediaBuildEmbedEvent that allows altering the media build before it is rendered and embedded. The currently existing '#embed' key in the build is not providing enough context to make advanced changes in the build.

Data model changes

None.

Release notes snippet

TBD

โœจ Feature request
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Mediaย  โ†’

Last updated about 1 hour ago

Created by

๐Ÿ‡ฆ๐Ÿ‡ฒArmenia Murz Yerevan, Armenia

Live updates comments and jobs are added and updated live.
  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

  • 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.

Missing content requested by

๐Ÿ‡ฆ๐Ÿ‡บAustralia dpi
8 months 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 about 1 year ago
  • last update about 1 year ago
    29,418 pass, 1 fail
  • last update about 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 12 months 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.

  • last update 8 months ago
    Patch Failed to Apply
  • First commit to issue fork.
  • 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 7 months 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 7 months 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
    6 months ago
    Total: 295s
    #70865
  • Pipeline finished with Failed
    6 months ago
    #70880
  • Pipeline finished with Failed
    6 months 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
    6 months ago
    Total: 990s
    #70911
  • Pipeline finished with Failed
    6 months 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
    6 months ago
    Total: 497s
    #72080
  • Pipeline finished with Failed
    6 months ago
    Total: 521s
    #72079
  • Pipeline finished with Failed
    6 months ago
    Total: 541s
    #75505
  • Pipeline finished with Failed
    6 months ago
    Total: 557s
    #75569
  • Pipeline finished with Canceled
    6 months ago
    #75976
  • Pipeline finished with Failed
    6 months ago
    Total: 1102s
    #75978
  • Pipeline finished with Failed
    6 months ago
    #77330
  • Pipeline finished with Failed
    6 months ago
    Total: 161s
    #77801
  • Pipeline finished with Failed
    6 months ago
    #77811
  • Pipeline finished with Failed
    6 months ago
    Total: 184s
    #77814
  • Pipeline finished with Failed
    5 months ago
    Total: 186s
    #88381
  • Pipeline finished with Failed
    5 months ago
    #88384
  • Pipeline finished with Canceled
    5 months ago
    Total: 215s
    #88398
  • Pipeline finished with Failed
    5 months ago
    Total: 693s
    #88402
  • Pipeline finished with Failed
    5 months ago
    Total: 712s
    #88407
  • Pipeline finished with Failed
    5 months ago
    Total: 676s
    #88413
  • Pipeline finished with Failed
    5 months ago
    Total: 587s
    #88421
  • Pipeline finished with Canceled
    5 months ago
    Total: 423s
    #88427
  • Pipeline finished with Failed
    5 months ago
    Total: 555s
    #88431
  • Pipeline finished with Failed
    5 months ago
    Total: 497s
    #88445
  • Pipeline finished with Failed
    5 months ago
    Total: 527s
    #88512
  • Pipeline finished with Canceled
    5 months ago
    Total: 365s
    #88531
  • Pipeline finished with Success
    5 months ago
    Total: 456s
    #88536
  • Pipeline finished with Success
    5 months ago
    Total: 577s
    #88555
  • Pipeline finished with Success
    5 months ago
    Total: 507s
    #88634
  • Status changed to Needs review 5 months 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
    5 months ago
    Total: 192s
    #94441
  • Pipeline finished with Success
    5 months ago
    Total: 609s
    #94442
  • Status changed to Needs work 5 months 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
    4 months ago
    Total: 175s
    #130405
  • Status changed to Needs review 3 months ago
  • Pipeline finished with Failed
    3 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
    3 months ago
    Total: 632s
    #135010
  • Pipeline finished with Failed
    3 months ago
    Total: 603s
    #135020
  • Pipeline finished with Success
    3 months ago
    Total: 712s
    #135027
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States carsoncho

    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 Seattle area

    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 3 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
    3 months ago
    Total: 625s
    #157116
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States ksenzee Seattle area

    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
    3 months ago
    Total: 165s
    #157895
  • Status changed to Needs review 3 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States ksenzee Seattle area
  • Pipeline finished with Failed
    3 months ago
    Total: 508s
    #157958
  • First commit to issue fork.
  • Pipeline finished with Success
    2 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
    2 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 2 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
    2 months ago
    Total: 953s
    #162114
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States ksenzee Seattle area

    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
    2 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 Seattle area

    @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

Production build 0.69.0 2024