Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

Status: Active » Needs review
FileSize
1.05 KB

separated from meta patch, first draft

rovo’s picture

FileSize
8.49 KB

I've run this patch against the latest Front-end branch.

It created:
/drupal_8_twig_sandbox/core/themes/stark/templates/views-view.html.twig

I compared the output before and after the patch.

There were no errors, and appeared the same.

Attaching DOM capture of #content before and after the patch.

Is there more I should look for?

Needed to switch the Admin to Stark in order for Views to pick up the views-view.html.twig template.

Now the "H2 A" elements are not printing out in the Summaries on the generated View Page.
<h2><a href="/node/32" rel="bookmark"></a></h2>

mbrett5062’s picture

Issue tags: +VDC

Tagging.

adamdicarlo’s picture

Title: Convert views/theme/views-view-field.tpl.php to twig » Convert views/theme/views-view-field.tpl.php to Twig

Working OK for me except for image fields - however I suspect that might be due to a separate issue.

Views' PHP template:

<div class="field-content">
  <img src="http://d8/sites/default/files/field/image/blueberries.gif" width="100" height="79" alt="" typeof="foaf:Image">
</div>


Twig template:

<div class="field-content">
  <figure class="">
    <img src="" class="image-formatter">
  </figure>
</div>
dawehner’s picture

The problem mentioned in #4 seems to be unrelated to this issue though for sure worth to report! Thank you.

FluxSauce’s picture

Status: Needs review » Fixed

Committed and correctly attributed, thanks!

Status: Fixed » Closed (fixed)

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

joelpittet’s picture

Project: » Drupal core
Version: » 8.x-dev
Component: Twig templates conversion (front-end branch) » theme system
Status: Closed (fixed) » Active
Issue tags: +Twig

Moving to core queue

tlattimore’s picture

Status: Active » Needs review
FileSize
2.09 KB

This is a straight re-roll of @joelpittet's patch in #1 against 8.x HEAD and deleted the corresponding .tpl.php.

joelpittet’s picture

Status: Needs review » Needs work

Thanks @tlattimore

I am moving this back to needs work for some cleanup docs and the preprocessor.

There is a theme function for this that needs to be moved into the preprocessor.

/**
 * Display a single views field.
 *
 * Interesting bits of info:
 * $field->field_alias says what the raw value in $row will be. Reach it like
 * this: @code { $row->{$field->field_alias} @endcode
 */
function theme_views_view_field($vars) {
  $view = $vars['view'];
  $field = $vars['field'];
  $row = $vars['row'];
  return $vars['output'];
}

Also, would like to get the variable $vars converted to $variables

/**
 * Process a single field within a view.
 *
 * This preprocess function isn't normally run, as a function is used by
 * default, for performance. However, by creating a template, this
 * preprocess should get picked up.
 */
function template_preprocess_views_view_field(&$vars) {
  $vars['output'] = $vars['field']->advanced_render($vars['row']);
}
trrroy’s picture

Assigned: Unassigned » trrroy
Status: Needs work » Needs review
FileSize
3.23 KB

Comment cleanup, theme function removed and 'vars' changed to 'variables'.

steveoliver’s picture

Status: Needs review » Needs work

A few more nitpicks:

+++ b/core/modules/views/templates/views-view-field.html.twig
@@ -0,0 +1,24 @@
+ *
+ * Variables available:
+ * - view: The view object
+ * - field: The field handler object that can process the input
+ * - row: The raw SQL result that can be used
+ * - output: The processed output that will normally be used.

1. Change "Variables available" to "Available variables" -- it's what we've been doing in all the other templates.
2. Add punctuation to the ends of the sentences.

+++ b/core/modules/views/templates/views-view-field.html.twig
@@ -0,0 +1,24 @@
+ *
+ * The above will guarantee that you'll always get the correct data,
+ * regardless of any changes in the aliasing that might happen if
+ * the view is modified.
+ */
+#}
+{{ output }}

Let's add an '\n@ingroup themeable' to the bottom of this docblock.

+++ b/core/modules/views/views.theme.inc
@@ -319,28 +319,14 @@ function template_preprocess_views_view_grouping(&$vars) {
-/**
  * Process a single field within a view.
  *
  * This preprocess function isn't normally run, as a function is used by
  * default, for performance. However, by creating a template, this
  * preprocess should get picked up.
  */
-function template_preprocess_views_view_field(&$vars) {

Since the theme function is gone, the entirety of this docblock text should be:

"Prepares variables for views field templates.\n\n Default template: views-view-field.html.twig."

trrroy’s picture

Status: Needs work » Needs review
FileSize
1.82 KB
3.35 KB

Changes are applied.

Status: Needs review » Needs work

The last submitted patch, 1843746-views-view-field-13.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
546 bytes
3.89 KB

Looks good, I just think the definition in views_theme needs changing too, and this is good to go.

This only part I'm not sure about is this part: "It is not actually used in default Views ..." huh? :)

Status: Needs review » Needs work

The last submitted patch, 1843746-15.patch, failed testing.

joelpittet’s picture

Title: Convert views/theme/views-view-field.tpl.php to Twig » Convert views/templates/views-view-field.tpl.php to Twig
jastraat’s picture

Status: Needs work » Needs review
FileSize
3.98 KB

Removed the line from http://drupal.org/node/1843746#comment-7252450 since without the theme function it no longer applies.
Added @see to documentation in line with http://drupal.org/node/1823416
Add $variables param to documentation in line with http://drupal.org/node/1913208

Status: Needs review » Needs work

The last submitted patch, drupal_twig_views_field_template-1843746-18.patch, failed testing.

jastraat’s picture

@ damiankloip
Quick question about #15: could you explain a bit more about the change from pattern to template in views_theme()? I think views_view_grouping is a similar case, and I want to understand a little better.

dawehner’s picture

I'm wondering whether we should have performance testing for this one ... as fields are rendered a lot. (could be more then hundred on a table view). Previously there is the theme_ vs. template trick, should we at least test this?

joelpittet’s picture

+++ b/core/modules/views/views.module
@@ -108,7 +108,7 @@ function views_theme($existing, $type, $theme, $path) {
   $hooks['views_view_field'] = $base + array(
-    'pattern' => 'views_view_field__',
+    'template' => 'views-view-field',
     'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),

@jastraat I am almost positive that is not necessary for this conversion. Remove that change and give that a go again. Nice work so far:)

joelpittet’s picture

Status: Needs work » Needs review

I retract #22, that is the default pattern, so shouldn't be necessary and probably does need template because it had a theme_ function and template file...

This one needs another pair of eyes, please.

dawehner’s picture

So the idea of this small piece of code was the following: Use the theme function by default, as it's certainly faster then using a template. As written before, if you have like 100 or more calls to this theme function on a page, this can be problematic.

As there will be no theme functions in D8, just twig, there is no need to special case the pattern/template bit.

jastraat’s picture

@dawehner - Does that mean that the entire section for views_view_field in views_theme() can be removed? Since the theme function will no longer exist?

star-szr’s picture

Status: Needs review » Needs work

I can answer #25, the hook_theme() definition is still needed even though the output will be through a template file.

Back to needs work, the test failures need to be addressed. Thanks everyone!

star-szr’s picture

Issue tags: +Needs manual testing

Tagging.

joelpittet’s picture

Assigned: trrroy » Unassigned
Status: Needs work » Needs review
Issue tags: +needs profiling
FileSize
3.94 KB
1.18 KB

Would love to see profiling as mentioned in #21.

This should fix the errors as as Views advanced_render() returns null from time to time.

dawehner’s picture

Well, but what is the problem with maybe put out a NULL variable, is this a specific twig problem?

thedavidmeister’s picture

Status: Needs review » Needs work

#29 - Looks like it's a problem with the way that Twig is implemented in Drupal - It will throw an exception whenever a NULL variable is passed to a Twig template. Same problem we're seeing in #1961872: Convert html.tpl.php to Twig.

I've opened a couple of tickets to see what can be done about it, if anything #1975442: drupal_render() should always return a string., #1975462: Allow and test for NULL and integer 0 values in Twig templates..

Regardless of whether this is resolved within Twig itself, can we not put the workaround inside the template? I really think this will be confusing to front end developers, as they'll have no idea when something coming in could possibly be NULL - if we break the seal we'll see {% if foo is defined %} appearing *everywhere* in contrib/custom templates.

Just change + $variables['output'] = $variables['field']->advanced_render($variables['row']); to something that enforces a string data type like:

  $variables['output'] = (string) $variables['field']->advanced_render($variables['row']);
bdone’s picture

Issue tags: -Needs manual testing

Aside from performance, manual testing of #28 looks good:

Before:

<div class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/11">Commodo Inhibeo Letalis Nostrud Nunc Typicus</a></span>  </div>  
  <div class="views-field views-field-field-image">    <span class="views-label views-label-field-image">Image: </span>    <div class="field-content"><a href="/node/11"><img typeof="foaf:Image" src="http://d8.localhost/sites/default/files/field/image/imagefield_9dafD4.jpg" width="298" height="269" alt="Elit eum lenis melior." title="Jumentum nutus praesent sed sudo."></a></div>  </div>  
  <div class="views-field views-field-body">    <span class="views-label views-label-body">Body: </span>    <div class="field-content">Brevitas magna quidne quis ratis. Enim iustum ullamcorper. Duis luptatum magna persto tation vereor.

Ad exerci gilvus lobortis loquor roto usitas vindico. Abbas accumsan consequat haero jus olim quidne torqueo tum wisi. Caecus dolor elit magna persto vulputate. Abbas imputo nostrud praesent singularis tincidunt. Acsi si sino vero. Abluo ad dignissim hendrerit inhibeo interdico rusticus singularis velit. Capto eum gemino imputo quia tum.
</div>  </div>  </div>

After:

<div class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/11">Commodo Inhibeo Letalis Nostrud Nunc Typicus</a></span>  </div>  
  <div class="views-field views-field-field-image">    <span class="views-label views-label-field-image">Image: </span>    <div class="field-content"><a href="/node/11"><img typeof="foaf:Image" src="http://d8.localhost/sites/default/files/field/image/imagefield_9dafD4.jpg" width="298" height="269" alt="Elit eum lenis melior." title="Jumentum nutus praesent sed sudo."></a></div>  </div>  
  <div class="views-field views-field-body">    <span class="views-label views-label-body">Body: </span>    <div class="field-content">Brevitas magna quidne quis ratis. Enim iustum ullamcorper. Duis luptatum magna persto tation vereor.

Ad exerci gilvus lobortis loquor roto usitas vindico. Abbas accumsan consequat haero jus olim quidne torqueo tum wisi. Caecus dolor elit magna persto vulputate. Abbas imputo nostrud praesent singularis tincidunt. Acsi si sino vero. Abluo ad dignissim hendrerit inhibeo interdico rusticus singularis velit. Capto eum gemino imputo quia tum.
</div>  </div>  </div>
star-szr’s picture

Assigned: Unassigned » star-szr

Doing some profiling here and likely posting a revised patch to lose the if statement.

dawehner’s picture

Yeah we do have a preprocess function, which allows us totally to fix this NULL problem. I guess "" is a proper value for that.

thedavidmeister’s picture

#33 - As in #1961872: Convert html.tpl.php to Twig, #1975442: drupal_render() should always return a string., if there's a function that's supposed to be "rendering" values - in this case Views advanced_render() - we probably want to make sure that it returns a string reliably rather than NULL. This would hopefully be a better fix than working around the Twig engine in the preprocess or the template.

Incidentally,

Views advanced_render() returns null from time to time.

advanced_render() sometimes calls drupal_render(), which sometimes returned NULL until yesterday. Is there any other way for Views advanced_render() to return NULL? If not, this problem is already fixed.

dawehner’s picture

FileSize
717 bytes

Maybe this could fix this 100%, though i'm not sure.

dawehner’s picture

Maybe this could fix this 100%, though i'm not sure.

thedavidmeister’s picture

Status: Needs work » Needs review

testbot

Status: Needs review » Needs work

The last submitted patch, drupal-1843746.patch, failed testing.

thedavidmeister’s picture

Status: Needs work » Needs review
FileSize
730 bytes

@dawehner - how about return (string) $value;? then yeah, from the looks of it, it should be impossible for Views to return anything non-stringy from advanced_render().

Status: Needs review » Needs work

The last submitted patch, 1843746-39.patch, failed testing.

dawehner’s picture

Yeah, this could work. I guess we have to manually adapt one of the tests?

thedavidmeister’s picture

#41 - or drupal_render($value);? anyway, we're totally hijacking this thread now. We should make a new issue for the Views render.

We should just re-roll #28 without the "is not null" and see what happens.

thedavidmeister’s picture

Status: Needs work » Needs review
dawehner’s picture

We should just re-roll #28 without the "is not null" and see what happens.

I agree.

star-szr’s picture

I'm not sure we need the 'if' at all, will post a patch tonight and some preliminary profiling numbers.

star-szr’s picture

Assigned: star-szr » Unassigned
Issue tags: -needs profiling
FileSize
407 bytes
3.93 KB

It does make sense to keep the if block, at least with the current state of things.

Profiled with @Fabianx's xhprof-kit, and thanks to his help and patience I put together and profiled a scenario that renders 200 fields in a table view.

Comparisons:

  1. Theme function implementation (HEAD).
  2. PHPTemplate implementation, HEAD plus this change:
    diff --git a/core/modules/views/views.module b/core/modules/views/views.module
    index 8e2a378..d09b87c 100644
    --- a/core/modules/views/views.module
    +++ b/core/modules/views/views.module
    @@ -108,7 +108,7 @@ function views_theme($existing, $type, $theme, $path) {
     
       // Default view themes
       $hooks['views_view_field'] = $base + array(
    -    'pattern' => 'views_view_field__',
    +    'template' => 'views-view-field',
         'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),
       );
       $hooks['views_view_grouping'] = $base + array(
    
  3. Twig template implementation (patch attached to this comment).

Theme function vs. PHPTemplate implementation:

=== baseline-views-field-theme-200-fields..views-field-tpl compared (51774c26bf054..51774cdfb1214):

ct  : 108,070|113,470|5,400|5.0%
wt  : 437,956|462,870|24,914|5.7%
cpu : 406,820|430,145|23,325|5.7%
mu  : 15,827,696|15,836,624|8,928|0.1%
pmu : 16,039,152|16,050,992|11,840|0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51774c26bf054&...

Theme function vs. Twig template implementation:

=== baseline-views-field-theme-200-fields..views-field-twig compared (51774c26bf054..51774d289642f):

ct  : 108,070|117,830|9,760|9.0%
wt  : 437,956|472,152|34,196|7.8%
cpu : 406,820|440,780|33,960|8.3%
mu  : 15,827,696|15,851,768|24,072|0.2%
pmu : 16,039,152|16,057,240|18,088|0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51774c26bf054&...

After looking at the profiling results and discussing with @Fabianx, we already have at least a few ideas for optimization:

  • Around 5ms: #1938430: Don't add a default theme hook class in template_preprocess() will remove the calls to drupal_html_class().
  • Around 4ms: Add static caching to Drupal\Core\Template\TwigEnvironment::loadTemplate(). (Issue to come)
  • Around 2ms: Use drupal_static_fast for $default_attributes (in addition to $default_variables) in template_preprocess().

Total - somewhere around 11ms, which should at least make the Twig implementation quicker than the PHPTemplate one. Longer term maybe we can get rid of template_preprocess() altogether or slim it down some more.

I'm also wondering (completely untested theory) if we could lose the whitespace operators here if that would mean better performance.

Scenario profiled:

  • 50 nodes created via devel generate, then disabled devel and devel generate
  • Stark theme
  • APC class loader enabled in settings.php
  • Created a table view of nodes displaying 50 rows with 4 columns: node title linked to the node, nid, date posted, and image
  • Total of 200 view fields rendered on the page

The site homepage was used for profiling, with one of the 50 nodes set as the default homepage and the table view added as a block to the homepage.

star-szr’s picture

Issue tags: +needs profiling

I'm also wondering (completely untested theory) if we could lose the whitespace operators here if that would mean better performance.

I couldn't detect any difference in performance at all after removing the whitespace modifiers. Good to know.

star-szr’s picture

Issue tags: -needs profiling

Oops, that'll teach me to reuse a tab :)

joelpittet’s picture

I am quite sure we don't need the if statement if we get this guy in #1975462: Allow and test for NULL and integer 0 values in Twig templates. If I remember right it's why I put that statement in the first place. Also, that should help a bit with the performance a bit as well. Cottser maybe you can roll the two for a test?

star-szr’s picture

@joelpittet/#49 - after doing some tests it looks like #1975462: Allow and test for NULL and integer 0 values in Twig templates. will either have no impact on performance or a small negative impact for this particular scenario. The numbers are quite close though and it's tough to pin them down. After adding #1975462: Allow and test for NULL and integer 0 values in Twig templates. and removing the if block from views-view-field.html.twig there are 75 more function calls in the scenario described in #46.

dawehner’s picture

Thanks for this great benchmark. So was far as I understand this performance regression is too much at the moment, so we should better wait until the points you listed went it. Hopefully this will lower then the costs.

star-szr’s picture

Well, currently #1938430: Don't add a default theme hook class in template_preprocess() is postponed on conversion. Maybe we need to rethink that now if this can't be committed without it.

Edit: Or we do all the other optimizations we can and keep #1938430: Don't add a default theme hook class in template_preprocess() postponed?

thedavidmeister’s picture

Status: Needs review » Needs work

#1975462: Allow and test for NULL and integer 0 values in Twig templates. just landed. Does this mean we can remove the "if output" and just leave {{ output }} to improve performance?

star-szr’s picture

Yes, but doing so didn't seem to have a measurable effect on performance from what I remember. Which is both good and bad :)

thedavidmeister’s picture

#54 - sure. well... remove it anyway for readability?

star-szr’s picture

Yes, we should still remove it because it's unnecessary.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
3.9 KB
532 bytes

Removed twig if;)

Status: Needs review » Needs work

The last submitted patch, 1843746-57-views-view-field.patch, failed testing.

thedavidmeister’s picture

Is that failing because of whitespace?

joelpittet’s picture

Status: Needs work » Needs review
FileSize
3.93 KB
556 bytes

Yeah @thedavidmeister that seemed to be the issue.

thedavidmeister’s picture

Status: Needs review » Needs work
+ * - field: The field handler object that can process the input.
+ * - row: The raw SQL result that can be used.

How does "field handler object" and "raw SQL result" fit in with our rules against discussing PHP data types in template comments?

+ *   - field: @todo.
+ *   - row: @todo.

See http://drupal.org/node/1757550#comment-7358798

shanethehat’s picture

Assigned: Unassigned » shanethehat
star-szr’s picture

We should also split off the theme_ function removal into its own issue along the same lines as #1843752: Convert views/templates/views-view-grouping.tpl.php to twig and #1912606: Remove theme_views_view_grouping function. Thanks!

Edit: In the end we will probably either keep the theme_ function (still allowing template overrides as we do now), or we may convert it to use a markup utility function - see #1986116: Improve performance by replacing very small and simple templates and theme function with "Markup Utility Functions" for discussion. So for now, maybe we don't need the other issue created, since it's a trivial patch to create.

shanethehat’s picture

Assigned: shanethehat » Unassigned
Status: Needs work » Needs review
FileSize
3.23 KB

New patch, only replaces the template and modifies the documentation. Leaves theme function alone.

I've removed the paragraph about the preprocess not usually running because it is run when the theme function is used.

thedavidmeister’s picture

Status: Needs review » Needs work
Issue tags: +Novice
+ * Prepares variables for views field templates.
+ * 
+ * Default template: views-view-field.html.twig.
+ * 
+ * @param ar

trailing spaces here.

+ * - field: The field handler object for the current field..

Double period at the end of this sentence.

+ * When fetching output from the row, this construct should be used:

Unnecessary comma.

+ * The above will guarantee that you'll always get the correct data,
+ * regardless of any changes in the aliasing that might happen if
+ * the view is modified.

These comments can be "reflowed" to use up the full 80 characters available.

Kevin Morse’s picture

Status: Needs work » Needs review
FileSize
9.05 KB

This should address those issues. I also fixed a few other of the worst offenders in views.theme.inc with regards to comments going past 80 characters.

thedavidmeister’s picture

Status: Needs review » Needs work

I also fixed a few other of the worst offenders in views.theme.inc

I appreciate the good intentions but we really have to keep these views conversion patches from creeping in scope. There's a separate issue for every one of those preprocess functions that you modified the docs for at #1757550: [Meta] Convert core theme functions to Twig templates.

Could we get a re-roll with just the docs updates for views-view-field?

star-szr’s picture

Issue tags: +needs profiling

Tagging for profiling.

shanethehat’s picture

Status: Needs work » Needs review
FileSize
3.25 KB
2.94 KB

Fixes the docs issues from #65, and also puts the template in the templates directory!

thedavidmeister’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice
joelpittet’s picture

Assigned: Unassigned » joelpittet

grabbing for profiling

joelpittet’s picture

Issue tags: -needs profiling

Scenario

View table with 50 rows and 5 fields = 250 fields rendered. A views-view-field.tpl.php in 8.x and views-view-field.html.twig both in Stark on their respective branches.

View block full content node in the content region.

=== 8.x..8.x compared (519bd405e5fe3..519bd4c5b5ac8):

ct  : 109,395|109,395|0|0.0%
wt  : 808,806|811,711|2,905|0.4%
cpu : 768,192|769,879|1,687|0.2%
mu  : 41,201,936|41,201,936|0|0.0%
pmu : 41,460,888|41,460,888|0|0.0%

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

=== 8.x..1843746-views-view-field compared (519bd405e5fe3..519bd4feea4c7):

ct  : 109,395|114,683|5,288|4.8%
wt  : 808,806|814,682|5,876|0.7%
cpu : 768,192|774,967|6,775|0.9%
mu  : 41,201,936|41,251,104|49,168|0.1%
pmu : 41,460,888|41,438,176|-22,712|-0.1%

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

joelpittet’s picture

Added the warning back in for that the template is not being used and is for reference as the theme_ function is used for performance.

No changes from performance teston #72

Also, reverted $variables to $vars because that is being done in. #1963942: Change all instances of $vars to $variables

star-szr’s picture

Still RTBC, ready for review. Thanks @joelpittet!

Status: Reviewed & tested by the community » Needs work
Issue tags: -Twig, -VDC

The last submitted patch, 1843746-73-views-view-field.patch, failed testing.

Fabianx’s picture

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

#73: 1843746-73-views-view-field.patch queued for re-testing.

star-szr’s picture

Status: Needs review » Reviewed & tested by the community
joelpittet’s picture

Assigned: joelpittet » Unassigned
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/views/templates/views-view-field.html.twigundefined
@@ -0,0 +1,30 @@
+{% spaceless %}
+  {{ output }}
+{% endspaceless %}

Why are we using the spaceless functionality here? If we need it then we to doc why...

joelpittet’s picture

Ok I replied but I guess the internet was too slow or something...

@alexpott I put that in because the twig template didn't remove the last newline character and failed whitespace related tests.

I wanted to get the markup as close as possible. So the real purpose in this case is to remove the ending \n character.

I'm sure there could be another way to do this, either leave the newline and change the whitespace tests or possibly something better but spaceless does the trick.

tlattimore’s picture

After running some tests locally I am not sure we really need spacelacess here at all to get it to pass the tests. Here is a patch that removes spaceless stuff from this instance - we'll see if the testbot approves. Figures crossed!

tlattimore’s picture

Status: Needs work » Needs review

Oops. Tagging for review.

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

The last submitted patch, 1843746-views-view-field-81.patch, failed testing.

tlattimore’s picture

Status: Needs work » Needs review

#81: 1843746-views-view-field-81.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1843746-views-view-field-81.patch, failed testing.

tlattimore’s picture

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

#81: 1843746-views-view-field-81.patch queued for re-testing.

tlattimore’s picture

Thats nice! Passed tests without spaceless. Anything else here you're concerned about, @Alexpott? RTBC?

star-szr’s picture

Status: Needs review » Reviewed & tested by the community

Great, thanks @tlattimore and @joelpittet! Back to @alexpott.

joelpittet’s picture

Nice @tlattimore. It seems that the twig templates are behaving like the docs indicated and remove the last newline as does the PHP template.

http://twig.sensiolabs.org/doc/templates.html#whitespace-control

alexpott’s picture

Title: Convert views/templates/views-view-field.tpl.php to Twig » [READY] Convert views/templates/views-view-field.tpl.php to Twig
Status: Reviewed & tested by the community » Closed (duplicate)
alexpott’s picture

Title: [READY] Convert views/templates/views-view-field.tpl.php to Twig » Convert views/templates/views-view-field.tpl.php to Twig
Status: Closed (duplicate) » Fixed

Committed c68896f 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.