In #1535868: Convert all blocks into plugins, we did a straight port of the Views block integration.
With plugins and derivatives and all the new stuff, we'll want to revamp our code.

Specific things to target:

  • The naming of \Drupal\views\Plugin\views\display\DisplayPluginBase::viewSpecialBlocks()
  • The $type parameter of viewSpecialBlocks(), which is currently always 'exp' for exposed blocks
  • Maybe make views_add_block_contextual_links() a method, or find a way to move that to contextual.module
CommentFileSizeAuthor
#16 vdc-1879256-16.patch3.51 KBtim.plunkett
#16 interdiff.txt1.07 KBtim.plunkett
#13 drupal-1879256-13.patch2.45 KBdawehner
#10 interdiff.txt735 bytesdawehner
#10 drupal-1879256-10.patch6.62 KBdawehner
#9 vdc-1879256-9.patch6.61 KBtim.plunkett
#7 drupal-1879256-7.patch6.61 KBdawehner
#7 interdiff.txt6.61 KBdawehner
#5 drupal-1879256-5.patch3.74 KBdawehner
#5 interdiff.txt638 bytesdawehner
#3 drupal-1879256-3.patch3.3 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: +Blocks-Layouts
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Postponed » Active
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
FileSize
drupal-1879256-3.patch3.3 KB

There really seems no use-case for anything else then exposed forms, and if someone needs it, nothing prevents him from doing that in any kind of other custom code.

tim.plunkett’s picture

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

There's still a reference to special_block above views_add_block_contextual_links()

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt638 bytes
drupal-1879256-5.patch3.74 KB

Good point, thank you!

I would be really surprised if there would be test-coverage for these kind of blocks.

tim.plunkett’s picture

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

I think the only thing left is #3 from the OP, making views_add_block_contextual_links() a method on ViewsBlock

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
interdiff.txt6.61 KB
drupal-1879256-7.patch6.61 KB

Oh totally forgot that, did I talked about working at night before ;)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: +Block plugins
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
vdc-1879256-9.patch6.61 KB

Rerolled.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1879256-10.patch6.62 KB
interdiff.txt735 bytes

Fixed some small bit.

As you didn't made any other suggestions is that RTBC?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Yes, I believe it is!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/views/lib/Drupal/views/Plugin/block/block/ViewsBlock.phpundefined
@@ -78,9 +78,35 @@ public function build() {
+    // Do not add contextual links to an empty block.
+    if (!empty($output)) {
+      // Contextual links only work on blocks whose content is a renderable
+      // array, so if the block contains a string of already-rendered markup,
+      // convert it to an array.
+      if (is_string($output)) {

I'm surprised that Views has to check this for its own blocks, can they really be both?

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined
@@ -2640,20 +2640,21 @@ public function getSpecialBlocks() {
+    if (arg(0) == 'admin' && arg(1) == 'structure' && arg(2) == 'views') {
+      return;

Couldn't this use menu_get_item()?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1879256-13.patch2.45 KB

Good points, here is a new patch.

According to b261004a4825e1b6eb1398e849ef3729acd4de97 this went in, so this just changes this specific parts of it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Oh hell yes, this patch removes two of my least favorite parts of Views :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal-1879256-13.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
interdiff.txt1.07 KB
vdc-1879256-16.patch3.51 KB

Ahh, when the drupal_render() is removed, it's not built fully.
This "fixes" it, but seems like overkill (those methods have an isset at the top of each in case they've run before, so its not too bad).

@dawehner, I hope you have better ideas :)

Status: Needs review » Needs work
Issue tags: -VDC, -Blocks-Layouts, -Block plugins

The last submitted patch, vdc-1879256-16.patch, failed testing.

dawehner’s picture

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

#16: vdc-1879256-16.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC, +Blocks-Layouts, +Block plugins

The last submitted patch, vdc-1879256-16.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Novice

This has some good idea in there in general.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Issue tags: -Novice +Needs issue summary update

Seems like this issue has taken a different turn since the original approach; can we update the summary? Also I'm not sure what the next step is here. I don't think it's novice since the last thing is @tim.plunkett saying doesn't know what to do. :)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

  • webchick committed b261004 on 8.3.x 
    Issue #1879256 by tim.plunkett, dawehner: Refactor viewSpecialBlocks...

  • webchick committed b261004 on 8.3.x 
    Issue #1879256 by tim.plunkett, dawehner: Refactor viewSpecialBlocks...

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

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

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Issue summary: View changes
Issue tags: -Blocks-Layouts

Not actively part of the Blocks-Layouts work.

  • webchick committed b261004 on 8.4.x 
    Issue #1879256 by tim.plunkett, dawehner: Refactor viewSpecialBlocks...

  • webchick committed b261004 on 8.4.x 
    Issue #1879256 by tim.plunkett, dawehner: Refactor viewSpecialBlocks...

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.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

quietone credited webchick.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Version: 8.9.x-dev » 8.0.x-dev
Status: Needs work » Closed (outdated)
Issue tags: +Bug Smash Initiative

This was committed somewhere between comments #11 and #12. AFAICT all the items in the Issue Summary were completed in the original commit. That leaves the following work. Looking at the latest patch I believe all that work has happened in other issues, such as #788900: Deprecate and remove usages of arg().

Therefore, closing as outdated. If this is incorrect, create a new issue explaining what still needs to be done and add this one as a related issue.

Thanks!

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Issue tags: -Bug Smash Initiative

Not a bug, removing bug smash tag.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Status: Closed (outdated) » Fixed

I am full of errors at the moment. Closing as fixed.

If there really is something that didn't get done here, create a new issue and add this as related.

Thanks!

Status: Fixed » Closed (fixed)

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