At the moment the viewExecutable also has a lot of properties like (live_preview) which simply should only be used
on the edit UI.

The idea is to create another object and use the data from there. It might be another object or some kind of decorator
or even just an extended class. This is not 100% clear.

CommentFileSizeAuthor
#11 views-1788232-12.patch18.65 KBtim.plunkett
#10 views-1788232-10.patch15.85 KBtim.plunkett
#7 views-1788232-7.patch132.61 KBdawehner
#5 views-1788232-5.patch5.49 KBdawehner
#3 views-1788232-2.patch5.49 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

I think one interim possibility would be an array property like $view->adminUI to put all of those properties in.

They are:

editing
display_errors
changed_display
build_time
render_time
execute_time
locked
changed
temporary_options
stack
live_preview
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review

The easiest approach, extend the existing class and use this one in the loader function,
even simple editing works, this throws some kind of notices, which are not easy to find.


Notice: Only variable references should be returned by reference in Drupal\views\ViewExecutable->__get() (line 421 of sites/all/modules/views/lib/Drupal/views/ViewExecutable.php).
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
views-1788232-2.patch5.49 KB

.

Status: Needs review » Needs work

The last submitted patch, views-1788232-2.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
views-1788232-5.patch5.49 KB

Let's see whether the patch now works.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Wow, I can't believe that works. Awesome!

+++ b/lib/Drupal/views/ViewUI.phpundefined
@@ -0,0 +1,114 @@
+   * @todo Group with other UI-only properties.

We can get rid of these comments now!

+++ b/views_ui.moduleundefined
@@ -292,7 +293,11 @@ function views_ui_edit_page_title($view) {
+  $original_view = NULL;
+  if ($storage) {
+    $original_view = new ViewUI($storage);

Might as well make this a ternary

+++ b/views_ui.moduleundefined
@@ -317,6 +322,7 @@ function views_ui_cache_load($name) {
   else {
+
     return $view;

Keep this out in the next reroll

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
views-1788232-7.patch132.61 KB
  • Added types to admin.inc
  • Updated based on the review.
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Title: Group ui related properties on the viewExecutable » Move UI properties from ViewExecutable to a new ViewUI class

Status: Needs review » Needs work

The last submitted patch, views-1788232-7.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
views-1788232-10.patch15.85 KB

Whew! That took a while to debug.

WizardPluginBase::instantiate_view() needed a ViewUI, not a ViewExecutable.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Unassigned » dawehner
FileSize
views-1788232-12.patch18.65 KB

More type hinting. I'm good with this now.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Fixed

Thank you! Committed and pushed

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