data-align and data-caption don't work with entity_embed

I am currently using the following code in a custom module to make data-align work in entity_embed-7.x-3.x (with the alignment filter from editor.module enabled). It works only for image:image and entityreference:entityreference_view_entity display plugins. I don't think there is a way to generalize this without knowing about the array structure of the different display plugins?

Anyway, I think something similar could be done properly within entity_embed at least for the known formatters / display plugins.

/**
 * Implements hook_entity_embed_alter().
 */
function mycustom_entity_embed_alter(&$build, $entity, $context) {
	if ($context['data-entity-embed-display'] == 'image:image') {
		if (!empty($context['data-align'])) {
			$build['#item']['attributes']['data-align'] = $context['data-align'];
		}
	}
	if ($context['data-entity-embed-display'] == 'entityreference:entityreference_entity_view') {
		if (!empty($context['data-align'])) {
			$build['file'][$context['data-entity-id']]['#attributes_array']['data-align'] = $context['data-align'];
		}
	}
}

/**
 * Implements hook_preprocess_file_entity().
 */
function mycustom_preprocess_file_entity(&$vars) {
	if (isset($vars['elements']['#attributes_array'])) {
		$vars['attributes_array'] = array_merge($vars['attributes_array'], $vars['elements']['#attributes_array']);
	}
}

I am currently working on this. I've been walking through the process of embedding the entities and noticed that the data-align attribute is being lost when embedding and there is nothing in the filter to add the class based on this. I will work on updating this so the data-align attributed doesn't get removed and we add the correct classes.

I've made some progress on this. The data-align property is getting stripped when we embed the entity because we do not allow it. That is a consequence of whitelisting attributes rather than using a wildcard. So we need to add 'data-align' and class as attributes to the drupal-entity tag. We also need to implement the filter so that it reads the data-align property and adds it to the class. I've done this locally and it aligns the entity within ckeditor itself. I need to refine the filter so it works when rendering the entity.

Then I think we will have to do the same thing for the data-captions. But will also have to add this to the form as it is not currently there.

We should wildcard *any* data attributes.

@Dave Reid, I am happy to make that change, but the discussion here: #2554687: <drupal-entity> tag not being automatically added when an Entity Embed button is added to active editor toolbar mention to explicitly define the attributes that would be allowed. Thoughts?

Let's have #2554687: <drupal-entity> tag not being automatically added when an Entity Embed button is added to active editor toolbar addressed first.

This is just a detail. We can continue working on this. Replacing data-align with data-* is a matter of minutes. No need to postpone this on #2554687: <drupal-entity> tag not being automatically added when an Entity Embed button is added to active editor toolbar IMO.

I agree, I can work on that one next, but I pretty deep into this ticket. I can finish completing this ticket, then once that one is fixed, we should be in better shape.

@slashrsm @Dave Reid The class attribute is getting lost when the renderEntityEmbed() function gets called in EntityHelperTrait.php. its set in the context in the function but when we call renderEntityEmbedDisplayPlugin, there is no 'class' in the '#attributes' of the $build array. It gets lost in the build function in the FieldFormaterEntityEmbedDisplayBase class.

Looking for suggestions on how to correctly fix this. I see two options.
1. In the build function, pull $this->attributes and look for a class and add it to the build array if it exists
or
2. Add it to the $build array after we call renderEntityEmbedDisplayPlugin.

I've tested adding this class in manually during this process to make sure that if we get the class added, it does indeed align the tag properly, and it does. Just need to find the correct way of making sure the class gets added during the rendering process.

Thoughts?

Thank you for looking into this @kmoll. We are relying on filter_align to handle data-align, right? I see two scenarios in this case:

1. filter_align runs before entity_embed filter. filter_align converts data-align to the appropriate class and entity_embed must preserve it. If I understand your comment correctly you're dealing with this situation. I think we'll need to add the class after display plugins have done their job. We don't have control over them and they will very likely remove it if we add before they run.
2. entity_embed filter runs before filter_align. entity_embed must preserve data-align attribute so filter_align can later convert it.

Do we want to support both situations or only one?

@slashrsm well that is the thing. So I moved the align filter to be after the embed filter. It works as long as we maintain the 'data-align' attribute which we aren't currently doing, so that will need to be updated for that. But if the user doesn't have the align filter enabled, or after than aligning doesn't work. So we either need to document that fact that the align filter needs to be enabled and placed after the embed filter and rely on users actually doing that. Or we have to handle adding the align class ourselves.

I think its one or the other. I don't think we add logic to check if the filter exists. I have a hesitation to make it seem through our plugin that we support alignment but then require the user to enable another filter in order for that to work. I also don't like duplicating functionality. So I'm a bit torn on which way to go.

Also, the align filter description and title make it seem like its specifically for images. If we rely on the align filter to do this work for us, we should make an issue and a PR to fix the wording of the filter so users know its not just to align images.

I have a hesitation to make it seem through our plugin that we support alignment but then require the user to enable another filter in order for that to work. I also don't like duplicating functionality. So I'm a bit torn on which way to go.

You are right... I still think it is better to require another filter than duplicating functionality. Of course we need to clearly document that (README and github.com/drupal-media/d8-guide?).

I don't have strong opinions about whether align filter should be executed before or after embedding. I think it shouldn't be that complicated to support both.

Also, the align filter description and title make it seem like its specifically for images. If we rely on the align filter to do this work for us, we should make an issue and a PR to fix the wording of the filter so users know its not just to align images.

True, but when looking at the code it seems that it should work with any HTML element. At least that :). I agree we should update the filter's description to make that clear.

@slashrsm ok, I will go with requiring the filter and we can just clearly define that in the documentation. I will take a second look to see that we can allow the filter to run either before or after our embed filter. But I was running into issues with our rendering of the entity not maintaining classes that were set on the DOM element before our filter ran. But I will take a look.

For data-caption, although this ticket states that we need to support that too, I think that is a different process that we will need to support within entity_embed. Do we want to break that out into a separate ticket?

@slashrsm, its possible to support the filter being either before or after the embed filter runs, but we need to take into consideration the attributes that exist depending on if the align filter has run first. If the align filter runs first, the data-align attribute is removed and the class is populated with the correct align class. If it hasn't run, the data-align attribute is still present, and needs to be preserved for the align filter.

The build() function in FieldFormatterEntityEmbedDisplayBase.php does not retain either the data-align or the class attribute. So we either need to handle both of those scenarios in the renderEntityEmbed() function after we get the build array where we can add in either the data-align attribute or the class attribute depending on if the align filter has run. Or We need to alter the build() function to handle those cases. Not sure what is the better option here, or if it makes a big difference or not. Thoughts?

I've created a patch with updating the build array after the build function is called. Let me know what you think

If that method works for you, I will create PR with the changes. If you prefer we support the attributes in a different way, lets discuss and I can update it.

@slashrsm I was able to remove the code in the plugin.js file that handled the class. That is handled by the align filter once I added the code to support it in the trait.

https://github.com/drupal-media/entity_embed/pull/206

Marked as needs-review again, the code was updated since the patch and is reflected in the PR.

The PR works for me so far. I can't provide a more detailed review tough. I am not familiar enough with entity_embed.

Tested, squashed and merged. Thanks @kmoll!

Does not work for me. data-align ignored in WYSIWYG and no class in content view.

Use Full HTML and Default filter processing order:

-Display embedded entities
-Align images
-Caption images
-Track images uploaded via a Text Editor

I tested this on fresh install with Bartik yesterday and I can assure it worked. Are you testing this on a custom theme? Did you check if "align-left", "align-right" or "align-center" class appears on embedded entity when displaying it?

Okay, did I something wrong?

My Steps:

1. Fresh install
2. Bartik Theme
3. Modules installed: entity_embed and embed
4. Create new embed button
   • Label: File Image
   • Embed type: Entity
   • Entity type: File
   • Allowed Entity Embed Display plugins: Image
5. Add embed button to WYSIWYG Full HTML profile
6. Activate Display embedded entities in Full HTML
7. This is the Code shown after add an image and click Source in in WYSIWYG
   <drupal-entity alt="test" data-align="left" data-embed-button="file" data-entity-embed-display="image:image" data-entity-embed-settings="{&quot;image_style&quot;:&quot;&quot;,&quot;image_link&quot;:&quot;&quot;}" data-entity-id="2" data-entity-label="File Image" data-entity-type="file" data-entity-uuid="[...]" title=""></drupal-entity>

But it is not aligned in WYSIWYG or Content view.

This is the Code in Content view:
<img src="/sites/default/files/styles/medium/public/2016-02/muc1.jpg?itok=[...]" alt="test" typeof="foaf:Image" class="image-style-medium">

Full HTML
Filter processing order:
- Display embedded entities
- Align images
- Caption images
- Track images uploaded via a Text Editor

Thank you in advance!

I've tested the latest dev version, and image alignment + captions do not work for me either - we've tested on two site installs here with no luck, so I also tested against a clean core install.

My steps to reproduce:

1. Install Drupal with the Standard profile, keeping Bartik as the main theme & Seven as the admin theme
2. Enable entity embed related modules: Dropzone JS, Entity Browser, Embed, File Browser, Entity Embed
3. Create an 'Image' text editor embed button
   • Embed type: Entity
   • Entity type: File
   • Allowed plugins: Image
   • Browser: Browser for files
   • Do not display entity after selection
4. Add the button to the WYSIWYG editor for Basic HTML,
   • Enable 'Display embedded entities' filter
   • Ensure 'Align images' and 'Caption images' filters are enabled
   • Added '<drupal-entity data-entity-type data-entity-uuid data-entity-id data-view-mode data-entity-embed-display data-entity-embed-settings data-align data-caption>' to the allowed HTML tags
5. Created content, uploading an image using the embed button. Selected align left, and added a caption.

In the editor, the 'data-align="left"' and 'data-caption="Test caption"' properties were both added to the embed code. However, neither of those made it through to the page display.

I used a debugger to step through the code, and it appears that '#attributes' isn't actually used when rendering, when it gets set here in EntityHelperTrait::renderEntityEmbed():

if (isset($context['data-align'])) {
  $build['#attributes']['data-align'] = $context['data-align'];
}

It actually uses '#item_attributes' during rendering further down the line. So I changed the code to $build['#item_attributes']['data-align'] = $context['data-align'];, and it works for classes, captions & alignment.

Please try the attached patch?

Cheers

I can confirm that the most recent Dev and the patch from Smaz, #36 works. Instantly. Thanks Smaz!

I will take a look at this again, it was (and should) work with [#attributes]. I will see if I can re-produce the error.

This latest patch doesn't seem to work for me in the current dev release... The caption is output as text, but it's not wrapped in a <figure>
If I don't enable the limit HTML filter it works fine (even without this patch), I've added the exact <drupal-entity> tag listed in #36

I've figured out what I was doing wrong.

The order of filters is really important to get right, and it would be great if it was documented better. For me the winning order is:

1. Limit allowed HTML tags
2. Caption images
3. Align images
4. Display embedded entities

And the string for allowed HTML Tags should be:
<drupal-entity alt data-caption data-align data-entity-type data-entity-uuid data-entity-id data-view-mode data-entity-embed-display data-entity-embed-settings data-embed-button data-entity-label>

According to mariagwyn comment it's cool. RTBC'ed.

I am changing the status back on this. I feel like I should change it back to fixed though, but still willing to discuss this and find out what the issue is here so set is as 'needs review'. I've ensured latest dev versions of this. I created embed buttons and added images and entities to content. I have set up the filters to match those listed above. My embed code was

And this was added to the wysiwyg field properly and was rendered properly in the page view of the node. I've tried with nodes aligning left, and aligning right.

@smaz Since you are using entity browser to add images, can you be more specific with the plugins you used in setting up your EB and if you set up views to select this or what other items you added here?

#attributes is the correct way to add attributes. I am not aware of having to use '#item_attributes', and have never seen that used before. AFAIK, that is not the correct way to add attributes, and I do not recommend applying the patch.

My last test was with a media entity. But I did create a new button that used an Image with File Entity. It used autocomplete rather than Entity Browser, but the embed code should be the same, unless the bug is with Entity Browser. The Image was rendered correctly and all the attributes were preserved.

ok, I was finally able to reproduce and I see what is happening here. I was adding a caption in. When a caption is added, it adds a wrapper tag, which is what the attributes are applied to, so everything works. When only an image is added, it doesn't add that wrapper tag, and the core image_formatter uses the #items_attributes. I do see that array element there in the renderEntityEmbed function.

But we still don't want to replace the #attributes element with #item_attributes all together, as the #attributes tag is what works with all other entities and elements. I suggest we just put a check in to see if the #theme is 'image_formatter' and add the #item_attributes if needed.

Created a PR:

https://github.com/drupal-media/entity_embed/pull/210

Based on #46 and interdiff is against it. Tests should pass now.

Committed #51. Thank you all!

@kmoll: Do you know if the data-align was fixed for centering? We can align left and right but can only align centre if there is a caption.

Fixing tags.

Do you know if the data-align was fixed for centering? We can align left and right but can only align centre if there is a caption.

Any updated about it? data-align center is still broken.

@ashopin @drholera

About this:

Do you know if the data-align was fixed for centering? We can align left and right but can only align centre if there is a caption.

My workaround was implementing the hook_entity_embed_alter(&$build, $entity, &$context) to add the class 'text-align-center' to the final $build array:

function mymodule_entity_embed_alter(&$build, $entity, &$context) {
  if (isset($build['#attributes']['data-align']) && $build['#attributes']['data-align'] == 'center') {
    $build['#attributes']['class'][] = 'text-align-center';
  }
}

A css rule for the class 'text-align-center' is included in the css file 'core/themes/stable/css/system/components/align.module.css' which is intended to be loaded in every page (sitewide). This is the existing rule:

.text-align-center {
  text-align: center;
}

This rule ensures that the image is displayed centered.

Hence, in my case, implementing hook_entity_embed_alter(&$build, $entity, &$context) to add this class to the embedded media image worked very well.

If for some reason you are not including this css file in your theme, make sure you define a similar rule in your sitewide css.

Oh, great, it works for me! Thank you!

In my case, I wanted the image itself to receive the bootstrap class 'center-block' for centering, not the container.

function mymodule_entity_embed_alter(&$build, $entity, &$context) {
    if (
        isset($context['data-entity-embed-display']) &&
        $context['data-entity-embed-display'] == 'image:image' &&
        isset($build['#attributes']['data-align']) &&
        $build['#attributes']['data-align'] == 'center' &&
        isset($build['entity'])
    ) {
        $build['entity']['#item_attributes']['class'][] = 'center-block';
    }
}

@ashopin @drholera @camilo.escobar

It looks like data-align center is still an issue if you do not have a caption. If you do have a caption, everything works.
I have created a patch based on #58.

I created a follow up issue for #61:
#3058870: data-align should work without caption