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

CommentFileSizeAuthor
#3 indentation-profiling-generate-settings.png37.16 KBstar-szr
#2 remove-2578567-2.patch1.67 KBstar-szr

Comments

Cottser created an issue. See original summary.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new remove-2578567-2.patch1.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

star-szr
he/him
Primary language English
commented
StatusFileSize
new indentation-profiling-generate-settings.png37.16 KB

Missing screenshot for devel_generate settings:

star-szr’s picture

star-szr
he/him
Primary language English
commented

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

star-szr
he/him
Primary language English
commented
Parent issue: » #2348381: [META-0 theme functions left] Convert/refactor core theme functions
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented 
+++ 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

lauriii
he/him
Primary language Finnish
Location Finland
commented
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

rainbowarray
she/her
Primary language English
Location Twin Cities, Minnesota
commented
Status: Needs work » Reviewed & tested by the community

Rando migrate fail. Unrelated.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
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.