Problem/Motivation

In media.module we have:

/**
 * Prepares variables for media templates.
 *
 * Default template: media.html.twig.
 *
 * @param array $variables
 *   An associative array containing:
 *   - elements: An array of elements to display in view mode.
 *   - media: The media item.
 *   - name: The label for the media item.
 *   - view_mode: View mode; e.g., 'full', 'teaser', etc.
 */
function template_preprocess_media(array &$variables) {
  $variables['media'] = $variables['elements']['#media'];
  $variables['view_mode'] = $variables['elements']['#view_mode'];
  $variables['name'] = $variables['media']->label();

  // Helpful $content variable for templates.
  foreach (Element::children($variables['elements']) as $key) {
    $variables['content'][$key] = $variables['elements'][$key];
  }
}

However inside media.html.twig we have:

{#
/**
 * @file
 * Default theme implementation to present a media item.
 *
 * Available variables:
 * - media: The media item, with limited access to object properties and
 *   methods. Only method names starting with "get", "has", or "is" and
 *   a few common methods such as "id", "label", and "bundle" are available.
 *   For example:
 *   - entity.getEntityTypeId() will return the entity type ID.
 *   - entity.hasField('field_example') returns TRUE if the entity includes
 *     field_example. (This does not indicate the presence of a value in this
 *     field.)
 *   Calling other methods, such as entity.delete(), will result in
 *   an exception.
 *   See \Drupal\Core\Entity\EntityInterface for a full list of methods.
 * - name: Name of the media item.
 * - content: Media content.
 * - title_prefix: Additional output populated by modules, intended to be
 *   displayed in front of the main title tag that appears in the template.
 * - title_suffix: Additional output populated by modules, intended to be
 *   displayed after the main title tag that appears in the template.
 * - view_mode: View mode; for example, "teaser" or "full".
 * - attributes: HTML attributes for the containing element.
 * - title_attributes: Same as attributes, except applied to the main title
 *   tag that appears in the template.
 *
 * @see template_preprocess_media()
 *
 * @ingroup themeable
 */
#}
<div{{ attributes }}>
  {#
    In the 'full' view mode the entity label is assumed to be displayed as the
    page title, so we do not display it here.
  #}
  {{ title_prefix }}
  {% if label and view_mode != 'full' %}
    <h2{{ title_attributes }}>
      {{ label }}
    </h2>
  {% endif %}
  {{ title_suffix }}

  {{ content }}
</div>

Proposed resolution

Use name instead of label in the twig template.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 2931057-2.patch530 bytesmarcoscano
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

seanB’s picture

Issue tags: +Need tests

The fact this went unnoticed makes me believe we need to have tests for this. The patch in #2775131-32: Media entities should support contextual links introduces the same change. So we need to make sure that this change is removed from the other issue.

chr.fritsch’s picture

The fact why we solved #2912298: Make media name available on manage display was that media items are often embedded into other entities. And for embedded media items we often don't need the name to be shown.

So if I understand this correctly here, this would kind of revert #2912298: Make media name available on manage display. I think this is what @Berdir also pointed out in https://www.drupal.org/project/drupal/issues/2775131#comment-12381209

marcoscano’s picture

Assigned: Unassigned » marcoscano
Status: Needs review » Needs work

I'll be working to add tests here, and I agree with @seanB it probably makes more sense to fix this here (with all possible tests), and only then continue the work on #2775131: Media entities should support contextual links and on #2930788: Do not show name by default in media displays.

seanB’s picture

So if I understand this correctly here, this would kind of revert #2912298: Make media name available on manage display.

I think this means that #2912298: Make media name available on manage display was never properly fixed. I do see your point though. We should remove the H2 instead of fixing the variable name. And add tests of course...

marcoscano’s picture

Assigned: marcoscano » Unassigned
Status: Needs work » Closed (won't fix)

After discussing this on slack with @seanB and @chr.fritsch, it seems that it doesn't really make much sense to fix what this issue is intended for, because we ultimately don't want to show the media name inside the template, unless the user deliberately includes it to the content region. Because of that, instead of "fixing" the template, the reasoning was to directly remove all name logic and hardcoding from the media template.

Under that context, this issue has no meaning anymore, so I'm closing this in favor of #2930788: Do not show name by default in media displays, where we can do this and add all missing tests.