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.

Comments

jibran created an issue. See original summary.

jibran’s picture

Status: Active » Needs review
Issue tags: +VDC
FileSize
897 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

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

jibran’s picture

\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

How about something like this?

dawehner’s picture

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

jibran’s picture

dawehner’s picture

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

jibran’s picture

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

Status: Needs work » Needs review
dawehner’s picture

sail3’s picture

i am working on this issue with @PaulDeza

sail3’s picture

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

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

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

Status: Needs work » Needs review
FileSize
1.08 KB
1.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

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’s picture

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.