Problem/Motivation

It can be quite difficult to get Content Blocks revisioning with the entity to which they are attached. This is complicated by the fact that scenarios in which content blocks are heavily used can clutter the block placement UI with an enormous number of blocks that were likely never intended for reuse.

Proposed resolution

We need a new way of interacting with Content Blocks that does not save them in a traditional entity approach. A null save that allows us to get the result of this save and serialize it into the block configuration. Serialized content blocks would be treated as a revisionable string in panelizer's normal operation which would mean that content blocks used this way would automatically revision with the rest of the "config" blocks because the storage would all be on the panelizer field.

Remaining tasks

We need to really clean up the following patch, update schema, make certain our UI stuff all makes sense and test out weird stuff like dates and file/image fields. Also, it's best if this field hides the configuration block's label form elements and just overrides the normal label with the title of the content block entity. We do that already for panelizer's IPE integration but need to figure out how to do it more generically.

User interface changes

A new section under IPE's block placement will be "custom" (I'd like to probably change this to inline). Within that section you'll find "Inline ..." blocks. These are what you want to test with. New ones should appear there automatically when you create new content block types.

API changes

This code depends on Inline Entity Form module. Be certain to download it if you want to play with this.

Data model changes

We introduce a new block type (inline block). So the existing data model's not changed, we're just abusing it a bit. :-D

Initial credit

I didn't do this first pass at a patch on my own, phenaproxima was instrumental in helping to make this happen as was tedbow.

Comments

EclipseGc created an issue. See original summary.

DamienMcKenna’s picture

Oh, I like the idea! This is very similar to what Paragraph Panes does.

DamienMcKenna’s picture

It might help to add a test method that includes a file field.

phenaproxima’s picture

Added a little more rigorous testing and fixed bugs discovered by it. This is going to need really thorough test coverage.

DamienMcKenna’s picture

Status: Needs work » Needs review

Would it be ok to finish this after the 3.0 release?

phenaproxima’s picture

Here's a new version of the patch that fixes some stuff. I have added thorough kernel-level test coverage of the panelizer_block_content shadow entity type, proving that it behaves identically to a block_content entity as far as Drupal is concerned, and transparently so.

Status: Needs review » Needs work

The last submitted patch, 6: 2844054-6.patch, failed testing.

samuel.mortenson’s picture

I noticed that the current test coverage doesn't assert that rendering blocks will result in the same HTML - is that still going to be covered?

phenaproxima’s picture

Yes. Absolutely. And I want to test Quick Edit compatibility too. But all of that, in my mind, is contingent upon the shadow blocks being transparently identical to normal ones. So the storage stuff is the foundation.

phenaproxima’s picture

Another iteration. This patch:

  • Plugs a bunch of holes in the null storage class that were making Panelizer uninstallable. As in, not possible to install. Which is why #6 failed.
  • Added a test assertion proving that one can transparently load a block_content as a panelizer_block_content using a UUID. (This is the mechanism that the block_content plugin uses to load blocks for display.)
  • Added a functional test proving that displaying a panelizer_block_content using the block_content plugin produces identical markup as displaying a normal block_content.
DamienMcKenna’s picture

Thinking through it, this might be better in Panels so that it could be used by *all* Panels-based display systems.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests, +quickedit
FileSize
24.54 KB
6.39 KB

IT WORKS!

This has no tests yet, and there are known bugs (particularly a bizarre double-save that might be IPE's fault, but I don't know yet) but it proves the concept. Behold, the prototype of inline revisionable content blocks!

Props to @EclipseGc for his invaluable help in making this happen, and for writing the original versions of the InlineBlock plugin and its deriver.

Still need Quick Edit integration and tests.

Status: Needs review » Needs work

The last submitted patch, 12: 2844054-12.patch, failed testing.

EclipseGc’s picture

FileSize
24.67 KB

Ok, fixed an issue we had with custom blocks actually getting created in the previous patch.

Eclipse

PS If you want to try this against imagefield blocks, you need to hack the shared tempstore class and use the dependency serialization trait. Still trying to figure out how to move something that will suffice for that shortcoming into the patch.

phenaproxima’s picture

DamienMcKenna’s picture

Project: Panelizer » Chaos tool suite (ctools)

Per today's scotch meeting, I'm moving this over to CTools so it can be a more foundational system that all Panels-ish modules can use.

EclipseGc’s picture

Ok, so I moved this all over to ctools and added some small checks to make it work inside of regular block layout as well. Tested with imagefield and it worked (which seems to be the high bar here). I moved the tests over but didn't get them working yet. Removing quickedit tag for the moment. We'll do that in a follow up.

Eclipse

phenaproxima’s picture

Won't pass the tests yet, but this formalizes the ctools_inline_block module and introduces a mechanism for loading inline blocks like normal entities.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
36.73 KB

OK. I've updated all the tests, so this should have parity (and then some) with the original patch for Panelizer. I'm leaving the "needs tests" tag applied because we should probably have coverage of how inline blocks interact with image fields -- that was a point of possible pain, so let's head off those problems at the pass.

Status: Needs review » Needs work

The last submitted patch, 19: 2844054-19.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
41.22 KB
6.46 KB

Added a functional JS test of using image fields with inline blocks! And it passes. On my localhost, anyway...

Status: Needs review » Needs work

The last submitted patch, 21: 2844054-21.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing

The failures are not due to anything wrong with the patch; I suspect wonkiness in Drupal CI, and the fact that it cannot load in dependencies. Therefore, I'm marking this as needing manual testing because you need to run the test suite on a local machine.

The last submitted patch, 17: 2844054-17.patch, failed testing.

The last submitted patch, 18: 2844054-18.patch, failed testing.

DamienMcKenna’s picture

Note: the tests will (most likely) fail because it needs #2858103.

Status: Needs review » Needs work

The last submitted patch, 26: ctools-n2844054-26.patch, failed testing.

DamienMcKenna’s picture

I'm thinking of listing this as a blocker for Panelizer 3.0/4.0 final.

  • EclipseGc committed 8204ef7 on 8.x-3.x
    Issue #2858103 by phenaproxima, DamienMcKenna, EclipseGc: Add...
phenaproxima’s picture

Status: Needs work » Needs review

Queued #26 for re-testing now that #2858103: Add serializable tempstore service (extracted from #2844054) is in. I don't think it'll pass the tests unless Drupal CI knows how to download dependencies (seeing as how this depends on Inline Entity Form). But, let us wait and see...

Status: Needs review » Needs work

The last submitted patch, 26: ctools-n2844054-26.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
40.24 KB

Probably won't pass the tests. Don't care, because this really fixes problems with the storage layer.

As of this patch:

- Inline blocks will report their entity type ID as 'inline_block_content'. Themes can figure out on their own how to handle any CSS classes generated from that.
- Thanks to some good advice from @Berdir, this uses a few hooks to link block_content bundles' fields to inline blocks, without having to actually duplicate the configuration in the database.
- It works with entity reference fields of all kinds. I tested briefly with a user reference and an image field.
- The most arcane parts of the previous approach are gone. Everything just kinda...makes more sense. It's really nice.

Status: Needs review » Needs work

The last submitted patch, 33: 2844054-33.patch, failed testing.

phenaproxima’s picture

YAY! This passes all tests locally. Drupal CI will fail it because it can't load up Inline Entity Form, but consider this as "needs review".

EclipseGc’s picture

FileSize
50.33 KB
11.56 KB

Added dispatching for block add/update/delete events on the BlockVariantTrait and panelizer support during those events.

Eclipse

DamienMcKenna’s picture

Per discussion in Slack, this needs to accommodate for displays which are exported and then reimported via CI, so that the appropriate {inline_block} records are created.

EclipseGc’s picture

FileSize
54.51 KB

Current patch. Sorry for the lack of interdiff, kind of drive-by-patching today since #drupalcon.

I'll try to get an interdiff later.

Eclipse