Problem/Motivation

theme_indentation() still exists and we want to remove all theme functions before 8.0.0. #1982208: Replace indentation theme hook/indentation.html.twig with data attributes is the ideal outcome but is a tad risky just before release.

Proposed resolution

Remove theme_indentation() and just use the Twig template.

Remaining tasks

  • Review
  • Commit

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Cottser created an issue. See original summary.

star-szr’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.67 KB

Numbers are looking very reasonable to me. This is 100 calls and also includes contextual_preprocess() for each template (@todo create issue to try to optimize that).

Scenario:

  • Created a menu with 100 links via devel_generate (see screenshot below for settings I used).
  • Allowed anonymous users to administer menus and menu links.
  • Set homepage to (for example) /admin/structure/menu/manage/devel-frakikavup
=== 8.0.x..8.0.x compared (560d7dac0c098..560d810c7abba):

ct  : 1,259,291|1,259,291|0|0.0%
wt  : 1,826,926|1,828,491|1,565|0.1%
mu  : 43,346,464|43,346,464|0|0.0%
pmu : 59,225,776|59,225,776|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=560d7dac0c098&...

=== 8.0.x..remove-indentation-theme-function compared (560d7dac0c098..560d8088818a3):

ct  : 1,259,291|1,262,339|3,048|0.2%
wt  : 1,826,926|1,840,941|14,015|0.8%
mu  : 43,346,464|43,372,176|25,712|0.1%
pmu : 59,225,776|59,253,664|27,888|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=560d7dac0c098&...

star-szr’s picture

Missing screenshot for devel_generate settings:

star-szr’s picture

And because it's worth mentioning: I had XDebug off, Twig debug off, render cache off.

Also, based on the xhprof breakdown most of the change seems to just be fluctuation from other things.

star-szr’s picture

lauriii’s picture

+++ b/core/includes/theme.inc
@@ -1097,23 +1097,6 @@ function template_preprocess_item_list(&$variables) {
-    $output .= '<div class="js-indentation indentation">&nbsp;</div>';

+++ b/core/modules/system/templates/indentation.html.twig
@@ -11,10 +11,4 @@
 {% for i in 1..size if size > 0 %}<div class="js-indentation indentation">&nbsp;</div>{% endfor %}

I was just wondering that these two has different classes. Should we change them to be the same or?

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Ignore last comment. Classes are the same in the template and in the theme function so they didn't change. RTBC for me

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: remove-2578567-2.patch, failed testing.

RainbowArray’s picture

Status: Needs work » Reviewed & tested by the community

Rando migrate fail. Unrelated.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 00de5e5 on 8.0.x
    Issue #2578567 by Cottser: Remove theme_indentation() and use Twig...

Status: Fixed » Closed (fixed)

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