Problem/Motivation

Feedback was brought up in #1898416-118: filter.module - Convert theme_ functions to Twig that some of the markup and logic in the filter.module theme functions/templates could be improved. The purpose of that issue was just to convert the existing markup and logic to Twig templates, this issue is to discuss changing it.

(The numbers do not match up with the original review as some of the points have been addressed or were not relevant to this follow-up issue.)

  1. +++ b/core/modules/filter/templates/filter-tips.html.twig
    @@ -0,0 +1,52 @@
    +  {% if multiple %}
    +    <div class="compose-tips">
    +  {% endif %}
    ...
    +    {% if multiple %}
    +      </div>
    +    {% endif %}
    ...
    +  {% if multiple %}
    +    </div>
    +  {% endif %}

    As a themer, the conditional output of the wrapper elements (and lack thereof) always resulted in a unexpected bad surprise when testing the design as anonymous user.

    I wonder whether we want to remove these 'multiple' conditionals as part of this conversion to (1) simplify the template and (2) prevent nasty surprises?

  2. +++ b/core/modules/filter/templates/filter-tips.html.twig
    @@ -0,0 +1,52 @@
    +    {% if multiple %}
    +      <div{{ tip.attributes }}>
    +      <h3>{{ tip.name }}</h3>
    +    {% endif %}

    The 'multiple' condition here looks wrong to me -- the wrapping container + heading is only used for the long filter tips; i.e., the condition should be 'long' instead.

    That said, it would be great if we could split these two completely different representations (short/list vs. long/page) into two separate templates. Having them in a single never really made sense to me.

Proposed resolution

Discuss the points from the review here and make updates as necessary.

Remaining tasks

Discuss
Patch

User interface changes

TBD

API changes

TBD

Comments

Cottser’s picture

Issue summary: View changes
Cottser’s picture

Issue summary: View changes

Removing point about the paragraph tags, they are gone from #1898416: filter.module - Convert theme_ functions to Twig now, and removing the item_list consolidation so that #1778624: rework theme_filter_tips to use the new Attributes, and call theme('item_list) while we're at it can handle it.

Cottser’s picture

Issue summary: View changes

Fixing code tags.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.