As follow up to #1934420: Allow area handlers to return a render array and as general concept, we should handle the view preview as render array too.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

There might be an issue here as the preview is used with an AJAX command, and that will expect the result to be rendered. This could be moved somewhere else, just a warning :)

damiankloip’s picture

Assigned: Unassigned » damiankloip

Working on this one.

damiankloip’s picture

Status: Active » Needs review
FileSize
21.5 KB

We may have to do something like this, which I'm not sure about.

olli’s picture

Just a few notes:

-    $output = $view->preview();
+    $preview = $view->preview();
+    $output = drupal_render($preview);

Lots of these, what about

     $output = $view->preview();
+    $output = drupal_render($output);

instead?

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.php
@@ -202,20 +202,24 @@ public function optionsSummary(&$categories, &$options) {
-    return new Response($this->view->render(), 200, array('Content-type' => $this->getMimeType()));
+   $output = $this->view->render();
+    return new Response(drupal_render($output), 200, array('Content-type' => $this->getMimeType()));

Indentation.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/View.php
@@ -81,14 +81,10 @@ function render($empty = FALSE) {
         // Current $view->preview() does not return a render array, so we have
         // to build a markup out if it.

Obsolete.

damiankloip’s picture

FileSize
20.46 KB

Thanks for the review! Much appreciated.

Here are those changes, let's see how this gets on. In a bit of a rush so if there are some missing I will have to fi them later :)

damiankloip’s picture

FileSize
16.77 KB

And the interdiff..

damiankloip’s picture

FileSize
2.02 KB
20.5 KB

Spotted a couple of things.

Status: Needs review » Needs work

The last submitted patch, 1938380-7.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
20.76 KB
2.62 KB

Fixing this bunch of silly mistakes :)

damiankloip’s picture

Thanks! I did say I ran out of time :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Forgot to set the patch rtbc

tim.plunkett’s picture

RTBC+1

webchick’s picture

#9: drupal-1938380-9.patch queued for re-testing.

xjm’s picture

+++ b/core/modules/rest/lib/Drupal/rest/Plugin/views/display/RestExport.phpundefined
@@ -202,20 +202,24 @@ public function optionsSummary(&$categories, &$options) {
   /**
    * Overrides \Drupal\views\Plugin\views\display\DisplayPluginBase::render().
    */
   public function render() {
-    $output = $this->view->style_plugin->render();
+    $build = array();
+    $build['#markup'] = $this->view->style_plugin->render();
 
+    // Wrap the output in a pre tag if this is for a live preview.
     if (!empty($this->view->live_preview)) {
-      return '<pre>' . $output . '</pre>';
+      $build['#prefix'] = '<pre>';
+      $build['#suffix'] = '</pre>';
     }
 
-    return $output;
+    return $build;

So Alex and I talked at length about this method, since it was setting a value in #markup rather than returning a "true" render array. However, in this particular case, the contents of the #markup isn't actually markup at all--it's a string representation of the REST response, getting wrapped in a <pre> tag for display to the user.

I did notice that we are calling drupal_render() in StylePluginBase::render_grouping_sets() -- Is that a requirement, or is that something we're planning to clean up later? Out of scope here in any case.

So we agree that this is commit-ready once it finishes testing.

dawehner’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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