Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Editing alignment doesn't "take" until source button pushed.
https://www.drupal.org/files/issues/2019-06-26/alignment%20issue%20-%207...
Here's the embed code:
<drupal-entity data-embed-button="media" data-entity-embed-display="entity_reference:media_thumbnail" data-entity-embed-display-settings="{"image_style":"thumbnail","image_link":""}" data-entity-type="media" data-entity-uuid="dc89fa65-1f64-45ad-89d6-9775683ac357" data-langcode="en"></drupal-entity>
If you search for the xpath '//drupal-entity[@data-align]' it doesn't appear until you click the source button twice to refresh the contents of the CKEditor.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3064287-4.patch | 647 bytes | Wim Leers |
| |||
| alignment issue - 720.mov | 7.91 MB | oknate | |
Comments
Comment #2
oknateComment #3
oknateIt seems like it's ignoring the data-align attribute because it's initially missing.
Comment #4
Wim LeersBingo.
But! The server-side preview is updated.
It's because of #3037316: Show an outline around the <drupal-entity> element within CKEditor that this does not work! It's the
<drupal-entity>tag whosedata-aligndetermines the CSS that applies, and it not being updated also results in the problem you demonstrated in the screencast. Try starting withdata-align=center, then it seems that using the dialog to change it torightworks, but the wrong CSS applies, because in the DOM it's stilldrupal-entity[data-align=center].This is a consequence of keeping
<drupal-entity>around rather than stripping it entirely. CKEditor's Widget API doesn't let us do that unfortunately.Comment #5
oknateManually tested, and confirmed the fix. We could add test coverage, but I don't have time this morning. Perhaps in a follow-up issue.
Comment #7
Wim LeersI don't think test coverage is necessary in this case; that'd amount to visual regression testing. We could test the updating of those attributes, yes, but that's testing a side effect, not the actual problem.
So +1 to landing this as-is.
🚢