@TODO

CommentFileSizeAuthor
#9 views-area-render-array-1934420-9.patch13.49 KBderhasi
#9 interdiff.txt6.45 KBderhasi
#7 views-area-render-array-1934420-7.patch10.09 KBderhasi
#3 views-area-render-array-1934420-3.patch10.05 KBderhasi
#2 views-area-render-array-1934420-2.patch10.37 KBderhasi
#1 views-area-render-array-1934420-1.patch2.05 KBderhasi

Comments

derhasi’s picture

derhasi commented
Status: Active » Needs work
StatusFileSize
new views-area-render-array-1934420-1.patch2.05 KB

Here's a first approach for allowing the return of render arrays and passing it forward to the them as render array. String return values are handled as #markup.

In that patch no existing area plugin has been rewritten to use a render array.

I'll do some work on trying to convert the given plugins to render array, before setting it to needs review.

derhasi’s picture

derhasi commented
Status: Needs work » Needs review
StatusFileSize
new views-area-render-array-1934420-2.patch10.37 KB

There's another patch, that forces plugins to use a render array as render value. The given core plugins were adjusted to suit the return value.

derhasi’s picture

derhasi commented
StatusFileSize
new views-area-render-array-1934420-3.patch10.05 KB

Arg, sorry, there was a wrong "Implements" statement ...

derhasi’s picture

derhasi commented
Issue tags: +SprintWeekend2013
dawehner’s picture

dawehner
Primary language German
commented

This minor points and this seems to be RTBC!

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/AreaPluginBase.phpundefined
@@ -159,7 +159,13 @@ public function preRender(array $results) {
+   *   Drupal render array.

I guess it should be a sentence with a verb in that case.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/Broken.phpundefined
@@ -27,7 +27,15 @@ public function adminLabel($short = FALSE) {
+  function render($empty = FALSE) {

Yeah let's fix it here and mark it as public.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/area/View.phpundefined
@@ -78,15 +78,21 @@ function render($empty = FALSE) {
+            '#markup' => $view->preview($display_id, $this->view->args),

Is there a follow up for preview() to not return a rendered output?

+++ b/core/modules/views/tests/views_test_data/lib/Drupal/views_test_data/Plugin/views/area/TestExample.phpundefined
@@ -40,11 +40,13 @@ public function buildOptionsForm(&$form, &$form_state) {
+   * Implements Drupal\views\Plugin\views\area\AreaPluginBase::render().

Nitpick alarm: Missing "\"

amateescu’s picture

amateescu commented
Status: Needs review » Needs work

Sounds like 'needs work' to me.

derhasi’s picture

derhasi commented
Status: Needs work » Needs review
StatusFileSize
new views-area-render-array-1934420-7.patch10.09 KB

Reworked the patch and opened #1938380: Let ViewExecutable->preview() return a render array.

Status: Needs review » Needs work

The last submitted patch, views-area-render-array-1934420-7.patch, failed testing.

derhasi’s picture

derhasi commented
Status: Needs work » Needs review
Issue tags: +VDC
StatusFileSize
new interdiff.txt6.45 KB
new views-area-render-array-1934420-9.patch13.49 KB
  • Made the tests test on the new render arrays. Render methods will allways return at least an empty array().
  • Made Entity Area Plugin work with render array.
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Awesome!

xjm’s picture

xjm
she/her
Primary language English
commented

#9: views-area-render-array-1934420-9.patch queued for re-testing.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Quick fix

Worst issue summary ever. Good thing the issue is simple and the VDC team writes clean code. ;)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Allow area handlers to return a render array » Change notice: Allow area handlers to return a render array
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 8.x-3.x-dev
Component: views.module » Code
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

This seems pretty straight-forward to me. Agreed on the issue summary. :)

Committed and pushed to 8.x. This looks like an API change between Views 7 and Views 8, so marking needs change notice and moving to the views queue. (I think I remember this being the protocol)

dawehner’s picture

dawehner
Primary language German
commented

See http://drupal.org/node/1954686

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Change notice: Allow area handlers to return a render array » Allow area handlers to return a render array
Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module
Status: Active » Fixed
Issue tags: -Needs change record

Looks good to me.

derhasi’s picture

derhasi commented

@dawehner, in the case of area plugins, those MUST return a render array. Should we change that or update the change notification? Also the change notification talks about plugins in general, is that intended?

dawehner’s picture

dawehner
Primary language German
commented

Yes that's intended, this is one big change in VDC. Feel free to polish the change notice.

derhasi’s picture

derhasi commented

Ok. So I updated the change notice.

damiankloip’s picture

damiankloip commented

That reminds me, I created #1960014: Empty area handlers need to be rendered for tables earlier based on this issue.

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