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

Files: 
CommentFileSizeAuthor
#3 indentation-profiling-generate-settings.png37.16 KBCottser
#2 remove-2578567-2.patch1.67 KBCottser
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,240 pass(es), 1 fail(s), and 0 exception(s). View

Comments

Cottser created an issue. See original summary.

Cottser’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.67 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,240 pass(es), 1 fail(s), and 0 exception(s). View

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&...

Cottser’s picture

Missing screenshot for devel_generate settings:

Cottser’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.

Cottser’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.

mdrummond’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.