Currently media_token_to_markup() is wrapping a media item with additional fields using a prefix/suffix and applying some default classes via drupal_attributes($attributes). This makes it difficult to add additional classes via its hook, hook_media_token_to_markup_alter()
Posting a patch that wraps the element via FAPIs container item.

Comments

DaneMacaulay’s picture

Patch

DaneMacaulay’s picture

arthurf’s picture

I like this for the reduction of markup and the simplicity of it.

I'm not sure if this is now the place to start talking about removing the conditional on the markup if fields are present. I think it would make sense for all images inserted by media use the same markup.

DaneMacaulay’s picture

I'd be interested in exploring new options here. If we ditch the conditional we loose the ability to float left or right from within WYSIWYG.

I'm using the hook in a project to float the wrapper div left or right depending on the WYSIWYG setting 'align' defined in the image properties form in ckeditor.

Incidently, im grabbing the width that ckeditor sets on the image and applying that to my container so that i can float the text below.
http://austinpost.org/neighborhood-news-bites/test

Another possibility is that we could store a float value, left or right, as a formatter setting... But that would then be persistent across all displays of that image and loose the flexibility of positioning within wysiwyg.

This is a new case, but my initial impulse to wrapping this entity was to try field_groups, need to look into why but this just outputs nothing.

DaneMacaulay’s picture

Heres an example of using ckeditors declarations from the image properties form to apply styles to the image's parent container:

<?php
function mymodule_media_token_to_markup_alter(&$element, $tag_info, $settings){
  if(!empty($settings['attributes']['style'])){
   $css_overrides = media_parse_css_declarations($settings['attributes']['style']);
    if(array_key_exists('float', $css_overrides)){
      $element['content']['#attributes']['style'][] = 'float: ' . $css_overrides['float'] . ';';
    }
    if(array_key_exists('width', $css_overrides)){
      $element['content']['#attributes']['style'][] = 'width: ' . $css_overrides['width'] . ';';
      $custom_width = TRUE;
    }
  }
?>
arthurf’s picture

I think doing something like this in media_token_to_markup() might be viable:

      $attributes = array(
        'class' => array(
          'media',
          'media-element-container',
          'media-' . $element['content']['file']['#view_mode'],
        ),
        'style' => ! empty($tag_info['attributes']['style']) ? $tag_info['attributes']['style'] : '';        
      );

I think this would work to handle the left/right correctly. We would need to remove the width/height from the $tag_info['attributes']['style'] I'm guessing.

DaneMacaulay’s picture

yep, though it might be necessary in some cases to pass the width to the wrapper as well, because to float text below the image we need a fixed width parent.
in most cases we can style width by the class:
'media-' . $element['content']['file']['#view_mode']
but if the image proportions are changed via wyswig image properties then we have whitespace.

the rest of my hook function, if wysiwyg doesnt set the width i fall back to width of image as it is

<?php
function mymodule_media_token_to_markup_alter(&$element, $tag_info, $settings){
  if(!empty($settings['attributes']['style'])){
   $css_overrides = media_parse_css_declarations($settings['attributes']['style']);
    if(array_key_exists('float', $css_overrides)){
      $element['content']['#attributes']['style'][] = 'float: ' . $css_overrides['float'] . ';';
    }
    if(array_key_exists('width', $css_overrides)){
      $element['content']['#attributes']['style'][] = 'width: ' . $css_overrides['width'] . ';';
      $custom_width = TRUE;
    }
  }
  if(!empty($element['content']['#attributes']['class'])){
    $element['content']['#attributes']['class'][] = 'ap-media-image';
    if(!$custom_width){
      $element['content']['#attributes']['style'][] = 'width: ' . $settings['width'] . 'px;';
    }
  }
?>
ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

i am gonna commit #2, lets talk about inline styles in another issue (#1885432: Figure out a way to keep markup output consistent in media embeds)
#2 is a no-brainer for now

ParisLiakos’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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