At the moment there are 3 views_ui_pre_render_* functions that live in views_ui/includes/admin.inc These are then added as pre_render callbacks in views plugins. This doesn't make sense, as everything else to do with buildForm etc... lives with the classes. These should be moved in the appropriate views plugin classes and called from there as methods.

This is reliant on #1993312: Change pre_render, post_render, and after_build callbacks to accept callables, so waiting on that.

Files: 
CommentFileSizeAuthor
#20 interdiff-1993384-20.txt5.93 KBdamiankloip
#20 1993384-20.patch11.59 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,037 pass(es).
[ View ]
#16 1993384-16.patch11.5 KBgrisendo
PASSED: [[SimpleTest]]: [MySQL] 59,396 pass(es).
[ View ]
#8 1993384-8.patch10.97 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1993384-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 interdiff-1993384-8.txt3.7 KBdamiankloip
vdc.views-ui-pre-render.patch9.97 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 55,576 pass(es), 195 fail(s), and 142 exception(s).
[ View ]

Comments

damiankloip’s picture

Issue tags:+VDC

.

damiankloip’s picture

Status:Postponed» Needs review
tim.plunkett’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/PluginBase.phpundefined
@@ -331,4 +331,52 @@ public function globalTokenForm(&$form, &$form_state) {
+   * Pre render: Move form elements into details for presentation purposes.
+   *
+   * Many views forms use #tree = TRUE to keep their values in a hierarchy for
+   * easier storage. Moving the form elements into fieldsets during form building
+   * would break up that hierarchy. Therefore, we wait until the pre_render stage,
+   * where any changes we make affect presentation only and aren't reflected in
+   * $form_state['values'].

I know these were copy paste, but we can clean them up a bit. If anything, we have to rewrap them to fit

Also, do we have any desire to remove the need for these? They just seem like FAPI abuse.

dawehner’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/PluginBase.phpundefined
@@ -331,4 +331,52 @@ public function globalTokenForm(&$form, &$form_state) {
+   * Pre render: Move form elements into details for presentation purposes.

Seems a little odd as coding standard.

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

The last submitted patch, vdc.views-ui-pre-render.patch, failed testing.

dawehner’s picture

Status:Needs work» Needs review

vdc.views-ui-pre-render.patch queued for re-testing.

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

The last submitted patch, vdc.views-ui-pre-render.patch, failed testing.

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new3.7 KB
new10.97 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1993384-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I think we need to keep these, as they are specifically pre render callback so they can alter the form but we don't alter the $form_state['values'] structure.

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

The last submitted patch, 1993384-8.patch, failed testing.

damiankloip’s picture

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

#8: 1993384-8.patch queued for re-testing.

dawehner’s picture

Status:Needs review» Needs work

So let's move them further up.

damiankloip’s picture

Yeah, let's do that.

dawehner’s picture

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

#8: 1993384-8.patch queued for re-testing.

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

The last submitted patch, 1993384-8.patch, failed testing.

dawehner’s picture

Issue tags:+Needs reroll

.

grisendo’s picture

Issue summary:View changes
Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new11.5 KB
PASSED: [[SimpleTest]]: [MySQL] 59,396 pass(es).
[ View ]

Re-rolled

Status:Needs review» Needs work

The last submitted patch, 16: 1993384-16.patch, failed testing.

grisendo’s picture

Status:Needs work» Needs review

16: 1993384-16.patch queued for re-testing.

grisendo’s picture

Green now! Broken head before?

damiankloip’s picture

Status:Needs work» Needs review
StatusFileSize
new11.59 KB
PASSED: [[SimpleTest]]: [MySQL] 58,037 pass(es).
[ View ]
new5.93 KB

Thanks grisendo! Looks good. One more thing; I think we should not use $this in the pre render calls, everywhere else we are just doing get_class($this) nowadays.

So maybe just like this?

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Ship it.

Xano’s picture

20: 1993384-20.patch queued for re-testing.

Xano’s picture

20: 1993384-20.patch queued for re-testing.

damiankloip’s picture

Assigned:damiankloip» Unassigned

I don't need to be assigned to this now.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 004fa12 and pushed to 8.x. Thanks!

[Edit] Removed a documentation fix I said I did when I did not.

Status:Fixed» Closed (fixed)

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