Use #attached to find the css/js of a view

As a short term, instead of doing

drupal_add_css($whatever);
return $output;

We could do

return array(
  '#markup' => $output',
  '#attached' => array(
    'css' => array(
      $whatever,
    ),
  ),
);

Would be fantastic to fix this for D8. I'm happy to help if folks have questions. In general, I think you have to pass a $build element around and let modules/plugins enrich it with css/js/markup, etc. I guess build could be right on the $view object

+++ b/lib/Drupal/views/Plugin/views/cache/CachePluginBase.phpundefined
@@ -247,12 +247,14 @@ abstract class CachePluginBase extends PluginBase {
+    $css = drupal_array_merge_deep(drupal_add_css(), isset($this->view->attached['css']) ? $this->view->attached['css'] : array());
...
+    $js = drupal_array_merge_deep(drupal_add_js(), isset($this->view->attached['js']) ? $this->view->attached['js'] : array());

These lines are really confusing already, can we instantiate $this->view->attached['whatever'] as an empty array not in this line?

+++ b/lib/Drupal/views/Tests/Plugin/CacheTest.phpundefined
@@ -83,7 +83,7 @@ class CacheTest extends PluginTestBase {
-  function testTimeCaching() {
+  function _testTimeCaching() {

@@ -126,7 +126,7 @@ class CacheTest extends PluginTestBase {
-  function testNoneCaching() {
+  function _testNoneCaching() {

?

+++ b/lib/Drupal/views/ViewExecutable.phpundefined
@@ -397,6 +397,17 @@ class ViewExecutable {
+   * An array of js/css/libraries, which belongs to the view.

JS/CSS

+++ b/lib/Views/block/Plugin/views/display/Block.phpundefined
@@ -77,7 +77,11 @@ class Block extends DisplayPluginBase {
+    $render = $this->view->render();
+    $info['content'] = array(
+      '#markup' => $render,

Is the $render variable used later in the function? If not, might as well avoid the temporary variable

+++ b/lib/Views/block/Plugin/views/display/Block.phpundefined
@@ -77,7 +77,11 @@ class Block extends DisplayPluginBase {
+    $render = $this->view->render();
+    $info['content'] = array(
+      '#markup' => $render,

Don't know how plausible it is, but the end goal should be to not render to HTML ourselves, but rather to pass a render array back to the caller that has a #theme on it. Let Drupal do the themeing later. We have #pre_render if we need to do some processing before we themeing occurs.

Okay, I'm working on #11

This required a lot of changes that would otherwise seem out of scope, but they're all positive ones:

• Use hook_library_info
• Force usage of render arrays where possible
• Remove views_add_js()

So we JUST introduced the title area, and already it's breaking things.

Awesome!

+ $vars['rows'] = drupal_render($vars['rows']);. Calls to drupal_render() are a code smell. I don't really care when they happen in Views UI such as in preview() method, but ones that happen inside of regular page processing are an issue. Specifically, the one in template_process_views_view() should be moved to the a render($content) or render($rows) in template file(s).

Yeah, that hack should definitely be revisited before commit, I'm just trying to get tests to pass for now.

This should fix the bugs, and address #16.

The rest of our CSS and JS should be turned into libraries as well, but that should be another issue.

I think that since render arrays are still "the right way" in D8, we should wait to see what happens. If anything, when the "new way" gets adopted, we'll be able to convert from render arrays to that, like the rest of core.

This uncomments those tests and cleans up a bit of extra code.

Oh, I forgot I still have to take out that one hack, reassigning.

I think this could go in as is, and #22 could be a follow-up. This already passes tests. But I leave that call to @dawehner.

Forgot interdiff

For the next reroll, change

-        foreach ($this->display_handler->getHandlers($area) as $handler) {
+        foreach ($this->$area as $handler) {

in ViewExecutable::render()

This adds at least 90 seconds to the test run, so this SHOULDN'T go in until #22 is done.
Apparently, the branch tests got slower too? They're up to 7.5 minutes...

+++ b/lib/Drupal/views/Plugin/views/area/AreaPluginBase.phpundefined
@@ -97,6 +97,15 @@ public function buildOptionsForm(&$form, &$form_state) {
+   * Perform any operations needed before full rendering.

Performs

+++ b/lib/Drupal/views/Plugin/views/area/Title.phpundefined
@@ -46,14 +46,12 @@ class Title extends AreaPluginBase {
+  function preRender(array $results) {

This should be public now

+++ b/lib/Drupal/views/ViewExecutable.phpundefined
@@ -2141,6 +2165,15 @@ class ViewExecutable {
+    $this->element =& $element;

= &$element, please

+++ b/lib/Views/block/Plugin/views/display/Block.phpundefined
@@ -75,12 +75,13 @@ class Block extends DisplayPluginBase {
-    $info['content'] = $this->view->render();
-    $info['subject'] = filter_xss_admin($this->view->getTitle());
+    $output = $this->view->render();
+    $this->view->element['#title'] = filter_xss_admin($this->view->getTitle());
     if (!empty($this->view->result) || $this->getOption('empty') || !empty($this->view->style_plugin->definition['even empty'])) {
-      return $info;
+      return $output;

This should just be reverted

+++ b/views.moduleundefined
@@ -144,6 +144,15 @@ function views_element_info() {
+  $types['view_special_block'] = array(
+    '#theme_wrappers' => array('container'),

Whaaaaat?

+++ b/views.moduleundefined
@@ -152,17 +161,38 @@ function views_element_info() {
+  dsm(123);

dpm() ;)

+++ b/views.moduleundefined
@@ -152,17 +161,38 @@ function views_element_info() {
+  $view->setElement($element);

Okay, that's kinda cool

+++ b/views.moduleundefined
@@ -152,17 +161,38 @@ function views_element_info() {
-    $element['view']['#markup'] = $view->preview($element['#display_id'], $element['#arguments']);
+    if ($element['#embed']) {
+      $element['view']['#markup'] = $view->preview($element['#display_id'], $element['#arguments']);
+    }
+    else {
+      $element['view']['#markup'] = $view->executeDisplay($element['#display_id'], $element['#arguments']);

I don't understand this

+++ b/views.moduleundefined
@@ -631,10 +661,17 @@ function views_page($name, $display_id) {
+  $element = array(
+    '#type' => 'view',
+    '#name' => $name,
+    '#arguments' => $args,
+    '#display_id' => $display_id,
+    '#embed' => FALSE,
+  );
+
+  return $element;

I think this

+++ b/views.moduleundefined
@@ -780,20 +817,15 @@ function views_block_view($delta) {
+    $element = array(
+      '#type' => 'view_special_block',
+      '#name' => $name,
+      '#display_id' => $display_id,
+      '#special_block_type' => $type,
+      '#pre_render' => array('views_pre_render_view_element', 'views_add_block_contextual_links'),
+      '#block_type' => 'special_block_' . $type,
+    );
+    return $element;

Except all of the weird "special_block" parts, I like this too

+++ b/views.moduleundefined
@@ -802,49 +834,37 @@ function views_block_view($delta) {
+  $element = array(
+    '#type' => 'view',
+    '#name' => $name,
+    '#display_id' => $display_id,
+    '#pre_render' => array('views_pre_render_view_element', 'views_add_block_contextual_links'),
+    '#block_type' => 'block',
+    '#embed' => FALSE,

What's with the views_pre_render_view_element? Why isn't that part of the regular #type => view stuff?

+++ b/views.moduleundefined
@@ -802,49 +834,37 @@ function views_block_view($delta) {
+  if (!empty($element['view']['#markup'])) {

This could use a comment.

Oh right. For now, I think we give up on late render and just do late themeing. We still get the advantage of properly using #attached. Is this doable?

Code looks good to me. I dont know much about the test fails other than it would be good if they went away :)

Bumping to major, the diffing of the global CSS before and after is really fragile, and it's going to be completely incompatible if we end up doing subrequests, and/or if drupal_add_css()/js() get refactored significantly (which seems likely at the moment).

The patch is collecting things from #attached, but it's not removing the handling of the globals yet?

Nice. We could also add ['#attached']['#block-title'] and that would solve our other problem. #block-title would be consulted when processing a top level block item.

tag

Unless @dawehner wanted to do more with this, I'm very very happy with this. Let's get it in!

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/CachePluginBase.phpundefined
@@ -230,8 +230,11 @@ function post_render(&$output) { }
+    $this->storage['css'] = drupal_map_assoc(array_keys(drupal_add_css()));
+
+    $js_start = drupal_add_js();
+    $this->storage['js'] = drupal_map_assoc(array_keys($js_start));
+    $this->storage['js']['settings'] = isset($js_start['settings']) ? $js_start['settings'] : array();

I'm confused about this bit. If we're using #attached, then we can assume that everyone else is. If contrib or custom code isn't using #attached to add css or js for context-specific stuff then it's broken - especially since we'll be doing our best to eliminate all calls to drupal_add_*() if not the functions themselves this release.

Rest of the patch looks good though - is it easy to remove the global CSS/JS handling or should that be a follow-up?

The comment in #7 is talking about node teasers. If you have node teasers, those are renderable arrays, which can have #attached set, and there's functions in common.inc to get the #attached from nested render arrays.

#51: drupal-1811828-51.patch queued for re-testing.

I agree.

I think it's perfectly fine to make progress in smaller steps.

However, let's make sure to create a follow-up issue then.
(Alternatively, we could also keep this issue open, but that most often turned out to be not a good idea.)

Catch seems to have his fingers in this particular pie.

I don't understand why #1849822: Convert (HTML) view rendering to a render array affects the global js/css diffing, that seems to be about making the view itself return a render array?

Nice work.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/CachePluginBase.php
@@ -222,7 +222,7 @@ function cache_flush() {
   /**
-   * Start caching javascript, css and other out of band info.
+   * Start caching the head.

It isn't too clear what 'the head' means

+++ b/core/modules/views/theme/theme.inc
@@ -68,13 +72,14 @@ function template_preprocess_views_view(&$vars) {
 
-  $empty = empty($vars['rows']);
+  // Render the rows render array to check for contents.
+  $rows = $vars['rows'];
+  $rows = drupal_render($rows);
+  $empty = empty($rows);

index 8e645a6..2bbb507 100644
--- a/core/modules/views/theme/views-view.tpl.php

I see that we call render($rows) in the views-view.tpl.php so i'm surprised to see a drupal_render() call in preprocess as well. Ideally we render within the template and not in preprocess.

@moshe, to answer your second point, we make a copy of the row values, and render that, to see if they're empty or not.
The original values are not rendered, and passed along.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/cache/CachePluginBase.phpundefined
@@ -246,22 +244,9 @@ function gather_headers() {
-    // Slightly less simple for CSS:
-    $css = drupal_add_css();
-    $css_start = isset($this->storage['css']) ? $this->storage['css'] : array();
-    $this->storage['css'] = array_diff_assoc($css, $css_start);
-
-    // Get javascript after/before views renders.
-    $js = drupal_add_js();
-    $js_start = isset($this->storage['js']) ? $this->storage['js'] : array();
-    // If there are any differences between the old and the new javascript then
-    // store them to be added later.
-    $this->storage['js'] = array_diff_assoc($js, $js_start);
-
-    // Special case the settings key and get the difference of the data.
-    $settings = isset($js['settings']['data']) ? $js['settings']['data'] : array();
-    $settings_start = isset($js_start['settings']['data']) ? $js_start['settings']['data'] : array();
-    $this->storage['js']['settings'] = array_diff_assoc($settings, $settings_start);
+    $attached = drupal_render_collect_attached($this->storage['output']);
+    $this->storage['css'] = $attached['css'];
+    $this->storage['js'] = $attached['js'];
   }

Yes this is more like it!

I think we are ready.

Sorry one more question. Why would the hook_library_info() implementation depend on the configuration setting for js or not? I'd think we'd check that setting when actually attaching the js? hook_library_info() isn't cached, but there's an issue to do that, and if it was then changing the configuration setting would require invalidating that cache to avoid a missing library.

Back to RTBC

#73: drupal-1811828-73.patch queued for re-testing.

Conflicted with #1818450: Introduce core token support in PluginBase class, straight reroll. Wait for green.

Thanks for the re-roll. Committed/pushed to 8.x.

An issue that was marked a duplicate of this was set to be backported to Drupal 7. I'm looking at how I can add #attached information to a view render and wondering if this in fact needs to be backported?

Filed new issue #1894736: Cannot use #attached to add general CSS/JS/Library to a View to discuss so I won't ping this issue anymore.