git commit -m "Issue #1898034 by Cottser, jenlampton, joelpittet, Shawn DeArmond, idflood, Hydra, artofeclipse, rich.yumul, chrisjlee, gnuget, c4rl, thund3rbox: block.module - Convert PHPTemplate templates to Twig."

Task

Convert PHPTemplate templates to Twig templates.

Remaining

  • Patch needs review
  • Manual testing (steps below)
Template path Conversion status
core/modules/block/templates/block-admin-display-form.tpl.php converted
core/modules/block/templates/block.tpl.php converted

Testing steps

Verify that various blocks are being output through block.html.twig.

#1757550: [Meta] Convert core theme functions to Twig templates
#1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch
#1938898: Convert block theme tables to table #type

CommentFileSizeAuthor
#135 interdiff.txt870 bytesHydra
#135 1898034-135-twig-block.patch24.8 KBHydra
#132 1898034-132-twig-block.patch25.48 KBjoelpittet
#132 interdiff.txt2.41 KBjoelpittet
#123 interdiff.txt705 bytesHydra
#123 1898034-123.patch24.74 KBHydra
#118 interdiff.txt2.27 KBHydra
#118 1898034-118.patch24.97 KBHydra
#110 1898034-110.patch25.1 KBgnuget
#110 interdiff.txt7.17 KBgnuget
#106 1898034-106.patch26.25 KBstar-szr
#106 interdiff.txt1.96 KBstar-szr
#103 1898034-103-twig-block.patch26.14 KBgnuget
#103 drupal-1898034-103.patch-interdiff.txt8.71 KBgnuget
#100 1898034-100-twig-block.patch17.65 KBjoelpittet
#100 interdiff.txt1.46 KBjoelpittet
#96 1898034-96-twig-block-reroll.patch17.53 KBjoelpittet
#90 drupal_twig_block-1898034-90.patch16.42 KBidflood
#90 interdiff.txt1.29 KBidflood
#86 drupal_twig_block-1898034-86.patch16.52 KBidflood
#86 interdiff.txt757 bytesidflood
#84 drupal_twig_block-1898034-84.patch16.52 KBidflood
#84 interdiff.txt1.71 KBidflood
#82 1898034-82.patch16.89 KBpatrickfgoddard
#74 drupal-1898034-74.patch16.89 KBc4rl
#74 drupal-1898034-74.patch-interdiff.txt8.49 KBc4rl
#71 twig-convert_block-1898034-70.patch25.41 KBjenlampton
#71 interdiff.txt1.86 KBjenlampton
#67 twig-convert_block-1898034-67.patch25.58 KBjenlampton
#67 interdiff.txt1.18 KBjenlampton
#64 twig-block-reroll-plus-1898034-64.patch25.4 KBjenlampton
#64 interdiff.txt1.78 KBjenlampton
#63 twig-block-reroll-1898034-63.patch25.51 KBjenlampton
#59 twig-convert_block-1898034-59.patch26.75 KBShawn DeArmond
#57 twig-convert_block-1898034-56.patch27.21 KBjenlampton
#54 twig-block-1898034-54.patch26.37 KBjenlampton
#54 interdiff.txt1.88 KBjenlampton
#52 1898034-52.patch26.03 KBstar-szr
#52 interdiff.txt822 bytesstar-szr
#47 1898034-47.patch26.85 KBShawn DeArmond
#47 interdiff.txt611 bytesShawn DeArmond
#45 1898034-45.patch26.86 KBShawn DeArmond
#41 1898034-41.patch26.86 KBstar-szr
#41 interdiff.txt6.26 KBstar-szr
#40 1898034-40.patch20.6 KBstar-szr
#40 interdiff.txt1.58 KBstar-szr
#39 1898034-39.patch20.33 KBstar-szr
#39 interdiff-34-39.txt1.5 KBstar-szr
#36 twig-block-1898034-36.patch19.44 KBjoelpittet
#36 interdiff.txt788 bytesjoelpittet
#34 1898034-34-reroll-only.patch19.54 KBstar-szr
#34 1898034-34.patch19.44 KBstar-szr
#34 interdiff.txt2.86 KBstar-szr
#32 1898034-32.patch19.16 KBchrisjlee
#32 interdiff.txt679 byteschrisjlee
#29 1898034-29.patch19.17 KBstar-szr
#29 interdiff.txt3.12 KBstar-szr
#22 1898034-22.patch20.4 KBstar-szr
#22 interdiff.txt1.17 KBstar-szr
#20 1898034-20.patch20.44 KBstar-szr
#20 interdiff.txt5.45 KBstar-szr
#18 1898034-18.patch18.38 KBstar-szr
#18 interdiff.txt2.04 KBstar-szr
#16 1898034-16.patch18.41 KBstar-szr
#16 interdiff.txt4.16 KBstar-szr
#13 1898034-13.patch14.25 KBstar-szr
#13 interdiff.txt9.97 KBstar-szr
#11 1898034-11.patch9.48 KBstar-szr
#11 interdiff.txt5.55 KBstar-szr
#4 1898034-block-twig.patch5.11 KBchrisjlee
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

c4rl’s picture

Issue tags: +Twig

Tagging

rcaracaus’s picture

Assigned: Unassigned » rcaracaus
chrisjlee’s picture

Assigned: rcaracaus » chrisjlee

going to attempt this process. my first twig patch.

chrisjlee’s picture

Status: Active » Needs review
FileSize
5.11 KB

attached is the patch. hope i did this correctly.

Status: Needs review » Needs work
Issue tags: -Twig

The last submitted patch, 1898034-block-twig.patch, failed testing.

chrisjlee’s picture

Status: Needs work » Needs review

#4: 1898034-block-twig.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Twig

The last submitted patch, 1898034-block-twig.patch, failed testing.

mlncn’s picture

Assigned: chrisjlee » mlncn

Taking a look at why the bot is hating on chrisjlee's patch.

mlncn’s picture

Assigned: mlncn » Unassigned

Sorry didn't get far on this (rookie mistake: trying to run the entire test suite through the web ui. 9 hours?)

Perhaps it needs in custom_block.module

use Drupal\Core\Template\Attribute;
star-szr’s picture

Assigned: Unassigned » star-szr

This shouldn't touch custom_block.module since that will be handled in #1898038: custom_block.module - Convert theme_ functions to Twig.

Working on a revised patch.

star-szr’s picture

Status: Needs work » Needs review
FileSize
5.55 KB
9.48 KB

I have more plans here (mostly we need to change the preprocess functions), but for now I'd like to see what the failures are if we keep custom_block_add_list in custom_block_theme() and remove the .tpl.php files.

Status: Needs review » Needs work

The last submitted patch, 1898034-11.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
FileSize
9.97 KB
14.25 KB

Another work in progress patch.

Docs are lagging behind and I'm adding @todos in the patch that need to be fixed before this is committable. Mostly posting to see what the tests look like though, there should be less failures now.

block-admin-display-form handling adapted from #1818678-11: Convert core/modules/block/block-admin-display-form.tpl.php to twig.

I compared the markup before and after, at a glance the even/odd classes don't quite match up but other than that looks pretty decent.

star-szr’s picture

Status: Needs review » Needs work
jenlampton’s picture

Also keep an eye on #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors we may not need even/odd/zebra stuff for much longer.

star-szr’s picture

Status: Needs work » Needs review
FileSize
4.16 KB
18.41 KB

Adding patch from @steveoliver in #1868212: Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect (theme_table) to fix the exceptions.

To fix the other test failures, I've done the following:

  1. Modified the whitespace in the expected output of \Drupal\block\Tests\BlockStorageUnitTest::renderTests() to match the Twig output.
  2. Removed an assertion from \Drupal\block\Tests\BlockTemplateSuggestionsUnitTest::testBlockThemeHookSuggestions() now that the 'content' class is being defined in the block.html.twig template instead of template_preprocess_block().
star-szr’s picture

Status: Needs review » Needs work

And back to CNW, @todos to fix!

star-szr’s picture

Issue summary: View changes

Add draft commit message

star-szr’s picture

Status: Needs work » Needs review
FileSize
2.04 KB
18.38 KB

Oops. Fixed up a @todo as well.

star-szr’s picture

Status: Needs review » Needs work
star-szr’s picture

Status: Needs work » Needs review
FileSize
5.45 KB
20.44 KB

Summary of changes:

  1. Updated subject to label for #1591806: Change block "subject" so that it's called a (admin_)label like everything else on the theme layer.
  2. Updated docblock for template_preprocess_block() to match pending standards.
  3. Use attributes.id instead of block_html_id in block.html.twig, use the Attribute object in template_preprocess_block().
  4. Removed last drupal_render() call from template_preprocess_block().

Would love to know if the following line is correct or not. #block_config doesn't seem to reliably contain 'label', at least not in the block template suggestions test.

$variables['label'] = check_plain($variables['elements']['#block']->label);
star-szr’s picture

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,50 @@
+ * @todo Update include path once http://drupal.org/node/1777532 is resolved.

Can we remove this? The issue is #1777532: Extend Twig_Loader_Filesystem instance to get .twig templates paths with hierarchy for theme dependencies in the sandbox which is still open but I don't see how it relates to the block template specifically.

star-szr’s picture

FileSize
1.17 KB
20.4 KB

One more tweak, move the default "block" class to block.html.twig.

star-szr’s picture

#22 will fail. Trying to figure out some things regarding the Attribute() class and whitespace operators in #drupal-twig.

Status: Needs review » Needs work

The last submitted patch, 1898034-22.patch, failed testing.

star-szr’s picture

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,50 @@
+<div id="{{ attributes.id }}" class="block {{- attributes.class }}"{{ attributes }}>
...
+  <div class="content {{- content_attributes.class }}"{{ content_attributes }}>

Neither of these lines work as expected. You end up with <div id="block-test-block" class="blockblock-block-test"> and <div class="contentfoobar-class">. So something is wrong, Attribute should be inserting a leading space before each class - see Attribute::__toString(). Is Twig or our Twig implementation stripping this out? Or am I doing something else wrong?

jenlampton’s picture

You are accidentally stripping out the space with the Twig whitespace controller: {{- you need to remove the minus sign in both cases.

star-szr’s picture

When I remove the minus signs, I get <div class="content "> when content_attributes.class is empty. Kinda ugly, so I was attempting to strip that out.

star-szr’s picture

I'll roll a new patch to remove the whitespace controllers (edit: and update the tests that verify the markup output), and remove the patch from #1868212: Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect (theme_table), since that is now a core issue with a patch and is affecting #1898416: filter.module - Convert theme_ functions to Twig as well so should be its own issue.

We'll get a lot of exceptions in the tests, so unfortunately #1868212: Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect (theme_table) will need to get resolved first before tests will pass here.

star-szr’s picture

Issue summary: View changes

Updating commit message

star-szr’s picture

Status: Needs work » Needs review
FileSize
3.12 KB
19.17 KB

Revised patch to make the changes described in #28. Expecting the exceptions from #13 to come back.

Status: Needs review » Needs work

The last submitted patch, 1898034-29.patch, failed testing.

star-szr’s picture

This is blocked on #1868212: Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect (theme_table), reviews/feedback still welcome though! Reviewing it again, I noticed a couple docs tweaks, I'll roll a patch over the next few days unless someone else wants to.

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,50 @@
+ * Default theme implementation to display a container.

This needs to change to "display a block".

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,50 @@
+ * @see template_preprocess_container()

This needs to be updated as well.

chrisjlee’s picture

FileSize
679 bytes
19.16 KB

Just docs changes as asked in #31

star-szr’s picture

Awesome, thanks @chrisjlee!

star-szr’s picture

Status: Needs work » Needs review
FileSize
2.86 KB
19.44 KB
19.54 KB

First patch - reroll for #1875260: Make the block title required and allow it to be hidden.
Second patch and interdiff - docs tweaks and the following changes:

  1. Changed
    $variables['label'] = check_plain($variables['elements']['#block']->label);
    

    to the much more sensible:

    $variables['label'] = check_plain($variables['block']->label);
    
  2. Removed the @todo mentioned in #21, I don't think it needs to be in block.html.twig.

I'm not sure about the block variable actually containing the "The full block entity" or not…

Status: Needs review » Needs work

The last submitted patch, 1898034-34.patch, failed testing.

joelpittet’s picture

FileSize
788 bytes
19.44 KB
+++ b/core/modules/block/block.module
@@ -550,11 +554,10 @@ function template_preprocess_block(&$variables) {
   $variables['content'] = $variables['elements']['#children'];
 
+  $variables['attributes'] = new Attribute;

This should probably be for those Attribute exceptions...
+ $variables['attributes'] = new Attribute(array());

Just a guess but I saw some something about that someplace.

Here's a reroll with that to see. (Tried testing local but clean tests failed on block)

joelpittet’s picture

Status: Needs work » Needs review

Engage!

Status: Needs review » Needs work

The last submitted patch, twig-block-1898034-36.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
FileSize
1.5 KB
20.33 KB

Thanks @joelpittet. I just overhauled #1868212: Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect (theme_table). As it turns out the exceptions here should actually get fixed by #1938898: Convert block theme tables to table #type.

Revised patch based on #34, interdiff is from #34. This should fix the 2 fails by relaxing the XPath on the language switcher test a bit.

Also restores the following change reverted in #34, Drupal\block\Tests\BlockTemplateSuggestionsUnitTest still doesn't like $variables['block']->label.

$variables['label'] = check_plain($variables['elements']['#block']->label);
star-szr’s picture

Issue summary: View changes

blocker

star-szr’s picture

Issue summary: View changes

Updating blocking issues

star-szr’s picture

FileSize
1.58 KB
20.6 KB

Actually, instead of waiting for either of those issues, this minor tweak should get us green again (adds an empty class array to avoid the Attribute exceptions).

I expect #39 to have one fail along with the exceptions (label again). I'm not sure why this is needed, but this change should address the failure by always ensuring there is a value for label. Would love to know if there's a better way or if the problem is somehow in Drupal\block\Tests\BlockTemplateSuggestionsUnitTest. We may be able to avoid this whole mess by just using block.label in the block template but that seems less friendly to themers.

star-szr’s picture

FileSize
6.26 KB
26.86 KB

This just replaces all instances of:

/**
 * Implements hook_preprocess_HOOK() for block.tpl.php.
 */

with:

/**
 * Implements hook_preprocess_HOOK() for block.html.twig.
 */
star-szr’s picture

Issue summary: View changes

Removing blocking issues entirely, we can get a green patch with a minor tweak.

star-szr’s picture

Issue summary: View changes

Add conversion summary table

star-szr’s picture

Issue summary: View changes

Linkify #type table issue and add to related

star-szr’s picture

Issue tags: +Needs manual testing

Tagging for manual testing, basic steps for testing are up in the summary.

Shawn DeArmond’s picture

Assigned: star-szr » Shawn DeArmond

I'm going to review this.

Shawn DeArmond’s picture

Status: Needs review » Needs work

Problem: #1033116: Rename template.php to THEMENAME.theme to eliminate ambiguity around "the template file" vs. "template files"

Basically, template.php doesn't exist anymore.

This patch needs to be re-rolled.

Shawn DeArmond’s picture

Status: Needs work » Needs review
FileSize
26.86 KB

Okay, here's the re-rolled patch. While the testbot is doing its thing, I'll test it manually too.

Shawn DeArmond’s picture

Status: Needs review » Needs work

Found a problem: It doesn't look like HTML special chars are being correctly output.

Original Drupal 8:

    <h2 class="">Who&#039;s new</h2>

With this patch:

      <h2 class="">Who&amp;#039;s new</h2>
Shawn DeArmond’s picture

Status: Needs work » Needs review
FileSize
611 bytes
26.85 KB

Okay, I found a check_plain() in template_preprocess_block(). Re-rolled patch attached.

I also compared the HTML of the block output between this patch and current D8 of the following blocks:
Who's online (where I discovered the issue. now fixed.)
Who's new
Book navigation
User Login
Powered by Drupal

Shawn DeArmond’s picture

Assigned: Shawn DeArmond » Unassigned

Someone else should look at this, I suppose.

Status: Needs review » Needs work
Issue tags: -Needs manual testing, -Twig

The last submitted patch, 1898034-47.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +Twig

#47: 1898034-47.patch queued for re-testing.

star-szr’s picture

Assigned: Unassigned » star-szr
Status: Needs review » Needs work

Thanks @Shawn DeArmond!

Rerolling and updating to account for #1968322: Remove unused $id and $zebra variables from templates.

star-szr’s picture

Assigned: star-szr » Unassigned
Status: Needs work » Needs review
FileSize
822 bytes
26.03 KB

This also removes docs about all the other standard variables (is_front, logged_in, is_admin) from the template, I don't think we need to document the standard variables in every template :)

jenlampton’s picture

Assigned: Unassigned » jenlampton

reviewing

jenlampton’s picture

Assigned: jenlampton » Unassigned
Issue tags: -Needs manual testing
FileSize
1.88 KB
26.37 KB

in block.html.twig we can remove the @todo about block ID if we don't print the block ID specially in the template file. If the ID will go away in the long run let's not treat it specially now. Nothing breaks now if we make this change now, and the change that removes the ID later won't need to touch this template then either.

The docblock for the block preprocess function had some incorrect information in it, and was overly verbose as compared to all the other preprocess function docblocks. Templates are now located in a 'templates' directory within the modules file, and there is no theme_block function.

Markup looks great, only a few whitespace changes.

Manual testing results:
before:

<div id="block-who-s-new"  class="block block-user contextual-region" role="complementary">
    <h2 class="">Who&#039;s new</h2>
  <ul class="contextual-links"><li class="block-configure odd first last"><a href="/admin/structure/block/manage/bartik.who_s_new/configure?destination=node/2">Configure block</a></li></ul>
  <div class="content">
    <div class="item-list"><ul><li class="odd first last"><a href="/user/1" title="View user profile." class="username" lang="" about="/user/1" typeof="sioc:UserAccount" property="foaf:name" datatype="">root</a></li></ul></div>  </div>
</div>

after:

<div id="block-who-s-new" class="block block-user contextual-region" role="complementary">
  
      <h2 class="">Who&#039;s new</h2>
    <ul class="contextual-links"><li class="block-configure odd first last"><a href="/admin/structure/block/manage/bartik.who_s_new/configure?destination=node/2">Configure block</a></li></ul>

  <div class="content ">
   <div class="item-list"><ul><li class="odd first last"><a href="/user/1" title="View user profile." class="username" lang="" about="/user/1" typeof="sioc:UserAccount" property="foaf:name" datatype="">root</a></li></ul></div>
  </div>
</div>

Also created a few follow-up issues:
#1972120: Remove CSS ID on blocks
#1972122: Remove the DIV tag around block content

Status: Needs review » Needs work
Issue tags: -Twig

The last submitted patch, twig-block-1898034-54.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

Updating remaining, adding testing steps

jenlampton’s picture

Issue summary: View changes

rm sand

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +Twig

#54: twig-block-1898034-54.patch queued for re-testing.

jenlampton’s picture

pulled and created new patch. trying again.

Status: Needs review » Needs work

The last submitted patch, twig-convert_block-1898034-56.patch, failed testing.

Anonymous’s picture

Issue summary: View changes

commit

Shawn DeArmond’s picture

Status: Needs work » Needs review
FileSize
26.75 KB

Somehow a diffed line from views.module got mixed in here.

Try this one.

Status: Needs review » Needs work

The last submitted patch, twig-convert_block-1898034-59.patch, failed testing.

Shawn DeArmond’s picture

Doh. There's also some stuff from theme.inc that doesn't apply.

@jenlampton, I think you had some other stuff in there when you rolled your patch.

Maybe try applying #52 again and making the whitespace changes to that.

jenlampton’s picture

Assigned: Unassigned » jenlampton

I've made a mess. will clean up, sorry guys.

jenlampton’s picture

Status: Needs work » Needs review
FileSize
25.51 KB

Okay, this is a re-roll of #52 accounting for changes in HEAD.
Should pass tests.

jenlampton’s picture

Assigned: jenlampton » Unassigned
FileSize
1.78 KB
25.4 KB

And here's a new patch with the same interdiff as before.
(hopefully should also pass tests)

Status: Needs review » Needs work

The last submitted patch, twig-block-reroll-plus-1898034-64.patch, failed testing.

jenlampton’s picture

Assigned: Unassigned » jenlampton

now that's a test I can work with :)

jenlampton’s picture

Assigned: jenlampton » Unassigned
Status: Needs work » Needs review
FileSize
1.18 KB
25.58 KB

Okay, this test checked to see if .content was being added appropriately in preprocess. but we aren't adding it in preprocess anymore, it gets added directly in the template. So we can safely remove this whole test now.

Status: Needs review » Needs work

The last submitted patch, twig-convert_block-1898034-67.patch, failed testing.

star-szr’s picture

Yeah, I originally removed that unneeded 'test-class' assertion in #16. Looks like this is getting closer though :) rock on!

I think to fix the tests we have to adjust for the 'id' attribute being printed towards the end of the wrapper div instead of at the beginning.

+++ b/core/modules/block/lib/Drupal/block/Tests/BlockTemplateSuggestionsUnitTest.phpundefined
@@ -8,6 +8,7 @@
+use Drupal\Core\Template\Attribute;

We didn't have this before and tests passed, is it needed?

jenlampton’s picture

Assigned: Unassigned » jenlampton

@Cottser, Ah no, we don't need that. That was left over from other monkeying around.

Manually printing the block ID at the beging of the template made the tests pass. I think we need to change the tests to account for printing them anywhere, will take another stab at it.

jenlampton’s picture

Assigned: jenlampton » Unassigned
Status: Needs work » Needs review
FileSize
1.86 KB
25.41 KB

Tests passing locally for me now. Fingers crossed!

joelpittet’s picture

Issue tags: +Needs manual testing

re-tagging. @Shawn DeArmond Giver heck

c4rl’s picture

Status: Needs review » Needs work

Given that conversion of the block administrative table would be the duty of #1938898: Convert block theme tables to table #type, we should not convert this per similar feedback given on #1898454-17: system.module - Convert PHPTemplate templates to Twig. Correct?

c4rl’s picture

Status: Needs work » Needs review
FileSize
8.49 KB
16.89 KB

Per my comments in #73, here's #71 without the administrative form converted.

Shawn DeArmond’s picture

Assigned: Unassigned » Shawn DeArmond

manually testing...

Shawn DeArmond’s picture

Tested the following blocks:

  • User Login
  • Search Form
  • Who's Online
  • Main Navigation
  • And a custom block

Also:

  • Added random text to block.html.twig to verify that the blocks are indeed being run through the Twig template.
  • Tested the admin block list

Note: If this issue no longer includes the admin block list, it should not be a "Testing Step" to test "output through block-admin-display-form.html.twig" You might want to change that on OP.

Can this be RTBC?

Shawn DeArmond’s picture

Assigned: Shawn DeArmond » Unassigned

Not It!

Shawn DeArmond’s picture

Issue summary: View changes

Tweak commit message

c4rl’s picture

Per #76, I've revised the issue summary to not include the administrative form.

jenlampton’s picture

Status: Needs review » Reviewed & tested by the community

Gave this baby another quick re-test, and it's lookin' good!
Nice job guys :)

jenlampton’s picture

Issue summary: View changes

Removed admin from summary.

c4rl’s picture

Title: Convert block module to Twig » block.module - Convert PHPTemplate templates to Twig

Per #1757550-44: [Meta] Convert core theme functions to Twig templates, retitling to indicate this issue applies to .tpl.php templates rather than theme_ functions (though there are no theme_ functions in this module).

c4rl’s picture

Issue summary: View changes

Updated issue summary.

c4rl’s picture

Issue summary: View changes

Updated issue summary.

patrickfgoddard’s picture

Assigned: Unassigned » patrickfgoddard
Status: Reviewed & tested by the community » Needs work

Re-rolling.

patrickfgoddard’s picture

Assigned: patrickfgoddard » Unassigned
Status: Needs work » Needs review
FileSize
16.89 KB

Re-rolled patch applied. I ran the block tests locally, and passed.

Interdiff not included due to not understanding to create on a reroll.

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Thanks @thund3rbox, reroll looks great!

+++ b/core/modules/block/block.moduleundefined
@@ -516,39 +517,40 @@ function block_rebuild() {
-  $variables['attributes']['class'][] = drupal_html_class('block-' . $variables['block']->module);
+  $variables['attributes'] = new Attribute;
+  $variables['attributes']['class'] = array();
...
-  // Add default class for block content.
-  $variables['content_attributes']['class'][] = 'content';
+  $variables['attributes']['class'][] = drupal_html_class('block-' . $variables['block']->module);
+++ b/core/modules/block/block.moduleundefined
@@ -6,6 +6,7 @@
+use Drupal\Core\Template\Attribute;

Now that #1982024: Lazy-load Attribute objects later in the rendering process only if needed is in let's revert all these changes please and remove the 'block' and 'content' classes being added to block.html.twig:

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,41 @@
+<div class="block {{ attributes.class }}"{{ attributes }}>

Can be just <div{{ attributes }}>

+++ b/core/modules/block/templates/block.html.twigundefined
@@ -0,0 +1,41 @@
+  <div class="content {{ content_attributes.class }}"{{ content_attributes }}>

Can be just <div{{ content_attributes }}>

idflood’s picture

Status: Needs work » Needs review
FileSize
1.71 KB
16.52 KB

Here is a reroll with the changes defined in #83 based on the patch in #82. I've quickly tested the output of the blocks and it looks fine.

I will try to compare against a clean drupal 8 to see if there is a difference.
With this patch there is just one difference. The "content" div doesn't have the "content" class.

Status: Needs review » Needs work

The last submitted patch, drupal_twig_block-1898034-84.patch, failed testing.

idflood’s picture

This adds back the "content" class and should make the tests pass.

idflood’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal_twig_block-1898034-86.patch, failed testing.

star-szr’s picture

I suspect this is something as simple as the order of the classes within the rendered output. We just need to run that failing test locally and change the expected output in the test to match what is being output now.

Edit: any takers? :) For instructions on running tests locally, see http://drupal.org/node/519364 and http://drupal.org/node/30036.

idflood’s picture

Status: Needs work » Needs review
FileSize
1.29 KB
16.42 KB

The following patch should fix the tests. Only two little modifications.

It was expecting 'class="content "' and now it is 'class="content"' (without the space at the end).
The other one is in the block title. It as '<h2 class="">' and now it is '<h2>'.
steveoliver’s picture

#90: drupal_twig_block-1898034-90.patch queued for re-testing.

thedavidmeister’s picture

Status: Needs review » Needs work

+ {{ title_prefix }}
{{ title_attributes }}
+ {{ title_suffix }}
+ <div{{ content_attributes }}>

Undocumented variables that are used in the Twig template.

+ * - contents: The contents of this block.

Variable name is "content" not "contents".

+ * - attributes: Remaining HTML attributes for the containing element.
+ * - attributes.id: A valid HTML ID and guaranteed unique.
+ * - attributes.class: Classes that can be used to style contextually through

Generally we're documenting properties of variables in Twig as:

- things: These are some things. Each thing contains:
  - foo: Description of thing.foo.
  - bar: Description of thing.bar.
thedavidmeister’s picture

Issue tags: -Needs manual testing

Do we really need manual testing for this HTML with the epic $expected[] going on inside the tests?

star-szr’s picture

star-szr’s picture

Issue tags: +needs profiling

Tagging for profiling.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Novice, -Needs reroll
FileSize
17.53 KB

Ok, I did a manual re-roll because my mergetool is a pain. Though this should fix also the concerns in #92
And also a small 1 space in the line:

  <div{{ content_attributes }}>
    {{ content }}
  </div>

which had 3 spaces instead of 4 before {{ content }}

I added also the new docs for 'plugin_id' and 'configuration' which were new to the template file. And still left out the helper variables that were dropped above.

star-szr’s picture

Assigned: Unassigned » steveoliver

This was RTBC in #79, assigning to @steveoliver for review but feel free to jump in @jenlampton :)

thedavidmeister’s picture

+ <h2{{ title_attributes }}>{{ label }}</h2>
+ <div{{ content_attributes }}>

title_attributes and content_attributes still undocumented. Was that intentional?

steveoliver’s picture

Assigned: steveoliver » Unassigned
Status: Needs review » Needs work

@joelpittet: wanna add docs for title_attributes and content_attributes? I'd then consider this RTBC after passing perf tests.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
1.46 KB
17.65 KB

Try that on for size:)

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Needs manual testing

Given that we need to convert all .tpl.php files in one patch, I don't think we can wait for #1938898: Convert block theme tables to table #type. I think we need to convert it here, as we had in #71. Can we pull that conversion back in and test it and see if anything needs doing?

star-szr’s picture

Just a thought but we might be able to bring that conversion back in by reverse-applying the interdiff in #74.

git apply 1898034-100-twig-block.patch
git apply -R drupal-1898034-74.patch-interdiff.txt
gnuget’s picture

Status: Needs work » Needs review
FileSize
8.71 KB
26.14 KB

Done.

(I did the reverse of #74 as Cottser suggest)

Status: Needs review » Needs work
Issue tags: -Needs manual testing, -needs profiling, -Twig

The last submitted patch, 1898034-103-twig-block.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +needs profiling, +Twig

#103: 1898034-103-twig-block.patch queued for re-testing.

star-szr’s picture

FileSize
1.96 KB
26.25 KB

Thanks @gnuget!

This should get things pretty solid as far as the markup output - I compared the markup on admin/structure/block using DaisyDiff and visual diff so it should be identical from a DOM perspective - some attributes changed order in the markup but that's about it.

We could do more of a straight template conversion here instead of handling things in preprocess as well, e.g. <table> markup in the template. Thoughts?

steinmb’s picture

Trying to get the hang of profiling. a big thanx to Fabianx for help and xhprof-kit. So, this is #106 running on a clean installed D8 with 5 blocks enabled on the front page.

=== 1898034..8.x compared (5193f4e792db1..519539bf90da4):

ct  : 166,748|174,030|7,282|4.4%
wt  : 988,709|1,063,604|74,895|7.6%
cpu : 953,098|1,019,747|66,649|7.0%
mu  : 15,851,712|16,870,560|1,018,848|6.4%
pmu : 16,133,272|17,182,592|1,049,320|6.5%

<a href="http://d8.dev/xhprof-kit/xhprof/xhprof_html/index.php?source=drupal-perf&url=%2F&run1=5193f4e792db1&run2=519539bf90da4&extra=1898034..8.x">Profiler output</a>

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage
star-szr’s picture

Status: Needs review » Needs work

Those profiling results seem a bit odd @steinmb. It's comparing this issue as the baseline to 8.x where we generally want to use 8.x as the baseline. But even then, I don't see how 8.x would be adding 7,282 function calls, if anything the Twig version would be more likely to have more function calls. Not that many more though :)


And regarding the block-admin-display-form conversion, I asked this question in IRC today:

We could do more of a straight template conversion here instead of handling things in preprocess as well, e.g. <table> markup in the template. Thoughts?

And we landed on yes, let's do table markup in the template. More of a straight conversion and the template would be much more useful than {{ table }}{{ children }}.

This would be a good task for a new or existing Twig contributor to take on: do a .tpl.php to .html.twig conversion of block-admin-display-form.tpl.php and likely remove most or all of the preprocess changes in template_preprocess_block_admin_display_form() from the patch in #106. The goal for conversion is to get the markup to match with the .tpl.php version (attributes/classes in a different order or whitespace differences are fine). If this interests you, please assign the issue to yourself. Any questions, ask in #drupal-twig on IRC or post here. Thanks!

gnuget’s picture

Assigned: Unassigned » gnuget
gnuget’s picture

Status: Needs work » Needs review
FileSize
7.17 KB
25.1 KB

This patch add the changes of #108

And seems to the markup is identical.

star-szr’s picture

Looks fairly reasonable to me at a glance, thanks @gnuget!

intergalactic overlords’s picture

I have checked html output. Both block admin page and block display html output is ok.

thedavidmeister’s picture

Issue tags: -Needs manual testing
TrevorBradley’s picture

This is my first time at profiling, so go easy on me!

Clean D8.x install, switched to Stark theme, created 5 test blocks (called "Test Block 1" to "Test Block 5" with simple content) added them to the Content area, created a Simple Page node and made it the default page and verified that it rendered properly.

For this profiling I made sure not to touch my laptop while the bbenches were running, as we were getting weird numbers with other apps running.

EDIT: this is against the 1898034-110 patch

trevor@qclinux:~/drupal$ ubench 51997c7b1a0cd 
=== 8.x..8.x compared (51997c7b1a0cd..51997cec15e52):

ct  : 36,208|36,208|0|0.0%
wt  : 369,208|370,539|1,331|0.4%
cpu : 188,000|188,000|0|0.0%
mu  : 11,385,264|11,385,264|0|0.0%
pmu : 11,527,056|11,527,056|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51997c7b1a0cd&run2=51997cec15e52&source=drupal-perf-trevorbradley&extra=8.x..8.x

=== 8.x..user-1898034-110 compared (51997c7b1a0cd..51997d3722709):

ct  : 36,208|36,316|108|0.3%
wt  : 369,208|356,249|-12,959|-3.5%
cpu : 188,000|188,000|0|0.0%
mu  : 11,385,264|11,385,304|40|0.0%
pmu : 11,527,056|11,525,648|-1,408|-0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51997c7b1a0cd&run2=51997d3722709&source=drupal-perf-trevorbradley&extra=8.x..user-1898034-110

TrevorBradley’s picture

Issue summary: View changes

Formatting

thedavidmeister’s picture

Assigned: gnuget » Unassigned

-3.5% for Twig? That's awesome. Can we get confirmation from somebody else though?

The numbers are probably fine but we usually see a slowdown of about 0.3-0.5% with Twig rather than a speedup so I'd personally like to see if that result is repeatable :)

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Novice
+++ b/core/modules/block/block.admin.incundefined
@@ -120,12 +124,22 @@ function template_preprocess_block_admin_display_form(&$variables) {
-  $variables['form_submit'] = drupal_render_children($variables['form']);
+  unset($children['blocks'], $children['block_regions']);
+  $variables['form_submit'] = drupal_render_children($children);

Now that #1920886: drupal_render() should check if it's rendering a 'render element' and if so call drupal_render_children() (inline) instead is in, we can remove all these lines (and the building of form_submit) from the preprocess function and just print {{ form }} in the .html.twig. See #1898454: system.module - Convert PHPTemplate templates to Twig for a very similar example.

Template and template docs will need to be updated as well.

+++ b/core/modules/block/templates/block-admin-display-form.html.twigundefined
@@ -0,0 +1,56 @@
+ * Each data in block_listing[region] contains:
+ * - data->region_title: Region title for the listed block.
+ * - data->block_title: Block title.
+ * - data->region_select: Drop-down menu for assigning a region.
+ * - data->weight_select: Drop-down menu for setting weights.
+ * - data->operations: Block operations.
+ * @see template_preprocess()
+ * @see template_preprocess_block_admin_display_form()

No arrow operators '->' in Twig templates, just dots.

Also we need a blank line before the two @see lines :)

Hydra’s picture

Assigned: Unassigned » Hydra

I'll take this one

Hydra’s picture

Status: Needs work » Needs review
FileSize
24.97 KB
2.27 KB

Okay, here it is, removed render_children and did some template docs.

Hydra’s picture

Assigned: Hydra » Unassigned

Okay, here it is, removed render_children and did some template docs.

jenlampton’s picture

Issue tags: -Novice, -needs profiling, -Twig

removing novice tag since it needs profiling.

cosmicdreams’s picture

cosmicdreams’s picture

+++ b/core/modules/block/block.admin.incundefined
@@ -84,15 +85,18 @@ function block_admin_edit(Block $entity) {
+  $children = $variables['form'];

As a result of not using drupal_render_children anymore, we should get rid of this variable.

We are going to be using drupal_render to render a renderable instead. So we need the renderable, not the renderable's children.

+++ b/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.phpundefined
@@ -135,11 +135,13 @@ protected function renderTests() {
+    $expected[] = '  ';
...
+    $expected[] = '    ';

Why the extra whitespace? I don't get why that's important.

+++ b/core/modules/block/lib/Drupal/block/Tests/BlockStorageUnitTest.phpundefined
@@ -159,12 +161,14 @@ protected function renderTests() {
+    $expected[] = '    ';
+    $expected[] = '';

Why does one line have whitespace and the other does not?

Hydra’s picture

FileSize
24.74 KB
705 bytes

Yay thx for the nice review!
Okay, first of all, the variable is obsolete, thx for pointing it!
About the extra white space, as far es I know, the output of the twig html is slightly different then the output of the tpl.php files related to whitespaces. We don't really care about whitespace in the output markup, so we just adjust the test to work with the new output. Test went out green so we should not care about this ;)

TrevorBradley’s picture

Assigned: cosmicdreams » Unassigned

Just a note that I'm here in the DrupalCon code sprint room willing to reprofile this patch as soon as it passes through the approval queue.

Also, Scott at Acquia (sorry, don't recall drupal name) suggested I also need to test the twig template for the admin page. He stated it was critical that node.html.twig was also loaded and suggested the following setup:

1) Make /admin/structure/block the default home page.
2) Create a single node with content.
3) Create a view that displays full node bodies, and insert this view as a block to be displayed on all pages.

Resulting in an /admin/structure/block home page that renders node/1 as a block on the page.

Still planning on going forward and testing this with the new patch, but let me know if Scott's idea needs review. :)

TrevorBradley’s picture

Alright, same test as yesterday - one node, 5 test blocks, all put on the same home page. Not doing Admin yet.

=== 8.x..8.x compared (519a986b87e71..519a9aa88c710):

ct  : 36,235|36,235|0|0.0%
wt  : 360,430|358,322|-2,108|-0.6%
cpu : 180,000|184,000|4,000|2.2%
mu  : 11,338,272|11,338,272|0|0.0%
pmu : 11,479,184|11,479,184|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a986b87e71&run2=519a9aa88c710&source=drupal-perf-trevorbradley&extra=8.x..8.x

=== 8.x..block-1898034-123 compared (519a986b87e71..519a9ae44e0a5):

ct  : 36,235|36,343|108|0.3%
wt  : 360,430|353,929|-6,501|-1.8%
cpu : 180,000|184,000|4,000|2.2%
mu  : 11,338,272|11,338,288|16|0.0%
pmu : 11,479,184|11,478,032|-1,152|-0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a986b87e71&run2=519a9ae44e0a5&source=drupal-perf-trevorbradley&extra=8.x..block-1898034-123
cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

Code Reviewed and it looks great.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1898034-123.patch, failed testing.

Hydra’s picture

Status: Needs work » Needs review

#123: 1898034-123.patch queued for re-testing.

TrevorBradley’s picture

Got a problem.

Attempted to test the block admin twig file and hit a snag.

I set up the following from scratch:

Set up a blank system
Switched theme to stark
Created a single node
Created a view to view all nodes and display as a block.
Added this block to the content area of every page.
Made admin/structure/block the default home page
Set up permission so that anonymous can administer blocks.

bbranch works find, the first half of bbranches works fine 8.x vs 8.x works fine, but it would start to struggle comparing 8.x to my git branch. I was getting the following errors in the apache error log:

[Mon May 20 16:50:54 2013] [error] [client 127.0.0.1] PHP Fatal error:  Class 'Drupal\\views\\ViewsData' not found in /home/trevor/drupal/sites/default/files/php/service_container/service_container_prod_.php/f47ebd5ae387a7266105310b7ef6624f9ecd76a68ad09473b7e60db6d607975d.php on line 2377

Then I noticed that if I attepmted to view the website, I'd get a white screen of death, associated with the same error.

I can clear the error in the patch branch, but only if I delete the sites/default/files/php directory. If I do that, the page renders correctly without error. If I then switch back to the 8.x branch, I get the *same* error, that doesn't go away unless I also clear the sites/default/files/php directory.

Unless bbranches will delete the php directory as it switches between branches, I'm not sure I can do the performance testing here. (I'll check with Scott though)

EDIT: Looks like this was a rebasing issue..... test results coming up...

TrevorBradley’s picture

Block Admin Test results...

=== 8.x..8.x compared (519abc45361d5..519abc8e6489b):

ct  : 58,650|58,650|0|0.0%
wt  : 234,177|235,142|965|0.4%
cpu : 232,000|232,000|0|0.0%
mu  : 14,555,544|14,555,544|0|0.0%
pmu : 14,799,144|14,799,144|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519abc45361d5&run2=519abc8e6489b&source=drupal-perf-trevorbradley&extra=8.x..8.x

=== 8.x..block-1898034-123 compared (519abc45361d5..519abcdcdf015):

ct  : 58,650|65,195|6,545|11.2%
wt  : 234,177|256,958|22,781|9.7%
cpu : 232,000|252,000|20,000|8.6%
mu  : 14,555,544|14,579,552|24,008|0.2%
pmu : 14,799,144|14,870,744|71,600|0.5%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519abc45361d5&run2=519abcdcdf015&source=drupal-perf-trevorbradley&extra=8.x..block-1898034-123

Hydra’s picture

This doesn't look right, somebody else should defenetly check this out.

joelpittet’s picture

I got similar results at about 7% wall time on the block admin page.

This is with my little space patch and moving row_class to attributes. And this profile is on this uploaded patch. So similar results even when I moved things around a bit.

@FabianX can you have a look?

=== 8.x..8.x compared (519b182c941bf..519b18dba5252):

ct  : 60,658|60,658|0|0.0%
wt  : 567,742|567,361|-381|-0.1%
cpu : 531,891|531,201|-690|-0.1%
mu  : 39,477,040|39,477,040|0|0.0%
pmu : 39,723,256|39,723,256|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519b182c941bf&...

=== 8.x..1898034-twig-block compared (519b182c941bf..519b1908c212c):

ct  : 60,658|68,940|8,282|13.7%
wt  : 567,742|608,439|40,697|7.2%
cpu : 531,891|572,949|41,058|7.7%
mu  : 39,477,040|39,669,632|192,592|0.5%
pmu : 39,723,256|39,969,872|246,616|0.6%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519b182c941bf&...

star-szr’s picture

Issue tags: +Twig

Great work on this folks! Fixing/updating tags.

P.S. I think I am "Scott from Acquia" referenced above but I'm not from Acquia, I just sprinted at their offices :)

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +needs profiling

Adding back profiling tag so we can profile the block output again for 5 blocks.

+++ b/core/modules/book/book.moduleundefined
@@ -957,7 +957,7 @@ function book_preprocess_block(&$variables) {
- * Processes variables for book-all-books-block.tpl.php.
+ * Processes variables for book-all-books-block.html.twig.

@@ -967,7 +967,7 @@ function book_preprocess_block(&$variables) {
- * @see book-all-books-block.tpl.php
+ * @see book-all-books-block.html.twig

Please remove these hunks from the patch, they are causing an unnecessary conflict when rolling the megapatch since this whole docblock is updated in #1898050: book.module - Convert PHPTemplate templates to Twig.

Thanks :)

Hydra’s picture

Status: Needs work » Needs review
FileSize
24.8 KB
870 bytes

Okay let's get rid of it.

geoffreyr’s picture

Assigned: Unassigned » geoffreyr

Profiling.

geoffreyr’s picture

Assigned: geoffreyr » Unassigned
Issue tags: -needs profiling

Ran this against the front page with a bunch of default Drupal blocks added. This doesn't cover the admin block UI. (Right now, I don't know how to hook up Fabianx's xhprof scripts up to auth and then use the admin pages.)

=== 8.x..8.x compared (519c60f26cf68..519c619634b1e):

ct  : 109,012|109,012|0|0.0%
wt  : 737,388|739,276|1,888|0.3%
cpu : 683,788|686,649|2,861|0.4%
mu  : 9,636,664|9,636,664|0|0.0%
pmu : 9,893,532|9,893,532|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519c60f26cf68&...

=== 8.x..1898034 compared (519c60f26cf68..519c6200dd578):

ct  : 109,012|109,802|790|0.7%
wt  : 737,388|744,267|6,879|0.9%
cpu : 683,788|690,829|7,041|1.0%
mu  : 9,636,664|9,898,012|261,348|2.7%
pmu : 9,893,532|10,154,800|261,268|2.6%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519c60f26cf68&...

star-szr’s picture

0.6% looks good to me :)

I worked a bit too much on this one to RTBC myself - can someone please re-review?

thedavidmeister’s picture

0.9%, yeah?

star-szr’s picture

@thedavidmeister - Nope, baseline wt was off by 0.3% so if we adjust for that, only 0.6% :)

Hydra’s picture

@geoffreyr block ui should be available if you set permissions for anonymous user for "Administer blocks", and seth admin/structure/blocks as your frontpage

geoffreyr’s picture

Assigned: Unassigned » geoffreyr

Righto, will give this another run.

geoffreyr’s picture

Assigned: geoffreyr » Unassigned

Scenario: admin/structure/block on the homepage, various sidebar blocks. Significant difference here.

=== 8.x..8.x compared (519d44ffb3845..519d45a7cd8f5):

ct  : 115,325|115,325|0|0.0%
wt  : 804,456|806,795|2,339|0.3%
cpu : 740,244|740,707|463|0.1%
mu  : 10,287,492|10,287,492|0|0.0%
pmu : 10,643,256|10,643,256|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519d44ffb3845&...

=== 8.x..1898034 compared (519d44ffb3845..519d45fd795a4):

ct  : 115,325|125,647|10,322|9.0%
wt  : 804,456|860,084|55,628|6.9%
cpu : 740,244|795,091|54,847|7.4%
mu  : 10,287,492|10,380,288|92,796|0.9%
pmu : 10,643,256|10,823,304|180,048|1.7%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519d44ffb3845&...

thedavidmeister’s picture

Status: Needs review » Needs work
Fabianx’s picture

Status: Needs work » Reviewed & tested by the community

Back to RTBC. (I also re-reviewed the whole patch.)

The admin-overview profiling, while important is not a blocker for this patch.

@alexpott said to me in Person that he does not care about 40ms for an admin overview page, which is still very much work-in-progress and changing anyway.

alexpott’s picture

Title: block.module - Convert PHPTemplate templates to Twig » [READY] block.module - Convert PHPTemplate templates to Twig
Status: Reviewed & tested by the community » Closed (duplicate)
alexpott’s picture

A folllow-up will be created to work on the block ui

alexpott’s picture

Issue summary: View changes

Updated issue summary.

alexpott’s picture

Title: [READY] block.module - Convert PHPTemplate templates to Twig » block.module - Convert PHPTemplate templates to Twig
Status: Closed (duplicate) » Fixed

Committed bd5769d and pushed to 8.x. Thanks!

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary (commit message).