Now that we have twig, we can easily combine comment-block, comment-wrapper and comment into a single comments template, that contains a for loop, wrapper containers/classes and much simplified logic to hide/show title text, empty text, via preprocess functions.

Point 1: When does it ever really make sense to show a comment without it's wrapper?

Two answers:

  1. Drupal may print out a single comment, possibly without wrapper (not sure?) when previewing your comment submission.
  2. views.

I would argue that it's more work to theme a comment output, when its stripped of its wrapper container (and class). If rendering a single comment, just pass it to a unified comments template with one individual comment for the for loop, while hiding/unsetting the comments section title in a preprocess function. This will provide an avenue for consistent theming across the site, allowing you to actually make proper use of the container and wrapper class.

Point 2: The comment-wrapper and comment-block templates really don't give us anything that cannot be equally handled using preprocess and/or sub-pattern template suggestions.

  1. The section title text of the comment wrapper is shown in some places, while in other places (eg comment preview) we don't want to show this title text at all. Some sites would want to show "1 Comment" versus "2 Comments" in the title on some content types, but want to show "Reviews" etc for other content types. Overriding the default title text could be handled easily using a combination of sub-pattern template suggestions. Eg function comments_preprocess_comments__comment_preview() { /* code to unset the "Comments" section title */ }
  2. Forums: currently there is nasty logic in comment-wrapper template to remove the title for node type 'forum'. Were we to consolidate and start using sub-pattern template suggestions, this would be as easy as function forum_preprocess_comments__forum() { /* code to unset the title */ }
  3. Comment-block template provides a trivial one-liner logic statement to essentially display "No comments available" text if there are no comments. This is a single conditional that could easily be added to the top of a unified comments template that everyone would plainly understand. Eg /*pseudocode*/ if not count(comments) print empty_text else enter for loop

Related:

Comments

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Title: Consolidate comment and comment-wrapper templates » Consolidate comment, comment-wrapper, and comment-block templates

Added "comment-block" template to the summary above as well. IMHO there's no reason we need three templates here.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes

Elaborated on reasons why we could get away with consolidation.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes

Clean up html

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes

Updated summary statement first paragraph for clarity.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes

Clean up point 2

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented

I'm thinking something like this:

<section id="comments" class="{{ attributes.class }}"{{ attributes }}>

  {% if not comments %}{{ 'No comments available'|t }}{% else %}

  {% if title %}{{ title }}{% endif %}  <!-- this would take care of printing the title.prefix title text, and title.suffix -->

  {% for comments as comment %}
  <article class="{{ comment.attributes.class }} clearfix"{{ comment.attributes }}>
    {{ comment.title_prefix }}
    {% if comment.new is defined %}
      <mark class="new">{{ comment.new }}</mark>
    {% endif %}
    <h3{{ comment.title_attributes }}>{{ comment.title }}</h3>
    {{ comment.title_suffix }}

    <footer>
      {{ comment.user_picture }}
      {% set args = {'!author': comment.author, '!date': comment.created} %}
      <p class="submitted">{{ 'by !author on !date' | t(args) }}</p>
      {{ comment.permalink }}
    </footer>

    <div class="{{ comment.attributes.class }}"{{ comment.attributes }}>
      {# We hide the links now so that we can render them later. #}
      {{ hide(comment.content.links) }}
      {{ comment.content }}

      {% if comment.signature is defined %}
      <div class="user-signature">
        {{ comment.signature }}
      </div>
      {% endif %}
    </div>
    {{ comment.content.links }}
  </article>

  {% endfor %}

  {% if form %}
    <h2 class="title comment-form">{{ 'Add new comment' | t }}</h2>
    {{ form }}
  {% endif %}

  {% endif %}
</section>

At the theme layer, this consolidation would reduce 3 template files to 1 removing 6 template files total (if you include both bartik, comment module, and stark). The current three template files in stark twig theme constitute 53 lines of code across 3 files (including blank whitespace lines, not including the docblocks), and the code sample consolidation here would reduce that to 45 lines in a single file.

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Component: Twig templates conversion (front-end branch) » Theme Component Library (components branch)

Unsure, but this probably has less to do with the twig conversion, and more to do with component library...

jwilson3’s picture

jwilson3
Primary language English
Location Ecuador
commented
Issue summary: View changes

Clean up first and second point even more.

Project: » Lost & found issues

This issue’s project has disappeared. Most likely, it was a sandbox project, which can be deleted by its maintainer. See the Lost & found issues project page for more details. (The missing project ID was 1750250)