Problem/Motivation

ViewsBlockBase doesn't expose parent view and display info which makes accessing the view, display or ViewsExposedForm plugin tricky in hook_block_build_alter or hook_block_view_alter. Right now the only way to access the ViewExecutable in hook_block_build_alter or hook_block_view_alter is use the block plugin to load the view and then create the ViewExecutable using that.

Proposed resolution

Added getters to ViewsBlockBase to access view and display.

Put that information into the render array similar to \Drupal\views\Plugin\Block\ViewsBlock::build()

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#18 viewsblockbase_doesn_t-2847657-18.patch1.2 KBjibran
#18 interdiff-cf0ba8.txt1.08 KBjibran
#7 viewsblockbase_doesn_t-2847657-7.patch793 bytesjibran
#7 interdiff.txt698 bytesjibran
#5 viewsblockbase_doesn_t-2847657-5.patch782 bytesjibran
#2 viewsblockbase_doesn_t-2847657-2.patch897 bytesjibran

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Active » Needs review
Issue tags: +VDC
StatusFileSize
new viewsblockbase_doesn_t-2847657-2.patch897 bytes

Now you can do this in alter hook.

if ($block instanceof ViewsExposedFilterBlock) {
    $view = $block->getView();
    $display_id = $block->getDisplayId();
    // Custom code here.
}
dawehner’s picture

dawehner
Primary language German
commented

I am wondering why we don't just put that information into the render array and extract if from there.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

\Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase::renderExposedForm() adds it to $form_state but only returns $form.
\Drupal\views\Form\ViewsExposedForm::buildForm() can add it to the $form array but it has $form_state available.
Where do you suggest we should add it?
\Drupal\views\Plugin\Block\ViewsBlock::build() provides this info in render array keys under '#view' and '#display_id'.
Maybe \Drupal\views\Plugin\Block\ViewsExposedFilterBlock::build() can provide those keys?

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new viewsblockbase_doesn_t-2847657-5.patch782 bytes

How about something like this?

dawehner’s picture

dawehner
Primary language German
commented

This looks way better for me! Let's add a line to document what is going on.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
StatusFileSize
new interdiff.txt698 bytes
new viewsblockbase_doesn_t-2847657-7.patch793 bytes

Here we go.

dawehner’s picture

dawehner
Primary language German
commented

This looks reasonable for me ... I wonder whether its possible to test this somehow.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

I don't think it is worth testing. It is just some context metadata. Unless we want to test it for all the views render output arrays?

Status: Needs review » Needs work

The last submitted patch, 7: viewsblockbase_doesn_t-2847657-7.patch, failed testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +SprintWeekend2017, +SprintWeekendBerlin
sail3’s picture

sail3 commented

i am working on this issue with @PaulDeza

sail3’s picture

sail3 commented
Status: Needs review » Reviewed & tested by the community

We(me and @PaulDeza) have tested the patch and it is working as espected.

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.3.x-dev » 8.2.x-dev

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

I could not find a user account for @PaulDeza to add to the potential credit list; can we add a link to their profile?

@sail3, to receive issue credit for your contribution, can you describe specifically how you tested the patch, and what the results were? Thanks! 

  1. +++ b/core/modules/views/src/Plugin/Block/ViewsExposedFilterBlock.php
@@ -27,6 +27,13 @@ public function getCacheContexts() {
+    // Provide the context for render array.

    This in-line comment doesn't actually help all that much. Can we expand it a little to describe how the context is used? Also, the fact that it is a render array is clear from the code, so I don't think we need to add those last three words. The fact that it is context metadata is what's important. 

  2. +++ b/core/modules/views/src/Plugin/Block/ViewsExposedFilterBlock.php
@@ -27,6 +27,13 @@ public function getCacheContexts() {
+        '#view' => $this->view,
+        '#display_id' => $this->displayID,

    Won't these added keys need to be documented in some docblock somewhere? (Maybe that documentation doesn't exist elsewhere yet, because it's Views.)

  3. I think I can agree with @Jibran that we don't necessarily need to test this. However, maybe you could provide a reference to another such place in the views code base that we add this context? That might help illustrate it or give us an example of how to document it better. I looked at the things referenced in #4 but didn't see a clear example.

Thanks, all!

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-cf0ba8.txt1.08 KB
new viewsblockbase_doesn_t-2847657-18.patch1.2 KB

Thanks! @xjm for the feedback. Fixed everything in #17.

I looked at the things referenced in #4 but didn't see a clear example.

You should start using PHPStrom instead of emacs :D :P

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

All feedback from @xjm has been addressed, updated the IS to reflect the actual fix.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed 0db6843 and pushed to 8.4.x. Thanks!

Since this is a task only committed to 8.4.x

  • alexpott committed 0db6843 on 8.4.x 
    Issue #2847657 by jibran, xjm: ViewsBlockBase doesn't expose parent view...

Status: Fixed » Closed (fixed)

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