The core block module renders useful IDs for its blocks as an ID attribute, however not all blocks that can be rendered by page_manager may implement the id() method, e.g. those which simply extend BlockBase. It is useful, however, to know what kind of block is being displayed when theming the output.
The attached patch adds a class to the wrapper div which either displays the id (if the method exists for the block) or the output of the getPluginId() method.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | page_manager-n2544406-11.patch | 743 bytes | damienmckenna |
| #11 | page_manager-n2544406-11.interdiff.txt | 615 bytes | damienmckenna |
Comments
Comment #2
berdirYou are mixing two things. Block plugins and block config entities are something different, only block config entities, which are only used by block.module have an id() method. And the plugin Id is already added in preprocess somewhere.
That said, yes, page_manager absolutely needs more CSS classes, likely in the form of manually set classes in the UI.
The block_class 8.x sandbox has a patch to add page_manager support but I haven't tried it yet, feedback would be very welcome: #2509142: Add support for page manager.
We could also build that directly into page_manager.
Comment #3
bradjones1@Berdir - thanks for taking a look. I'll also investigate why the test failed, though at first glance the error message in Jenkins isn't super helpful so I'll have to look a little deeper.
Regarding the id() method, yes I agree, and that's why I included the conditional logic that either implements id() or fails back to getPluginId(), which should exist in both cases. The goal here is to include some helpful output about the identity of the block, regardless if it's a plugin or config entity.
Comment #4
berdirYes, but id() never exists, unless you just add it to some blocks, but there is no interface or anything would tell blocks to do that. You also didn't call an id() method, you accessed an ->id property, missing ().
And it doesn't solve your problem in any way. It would still be the same class for blocks of the same plugin.
Comment #5
bradjones1You're correct... I was looking at the Block class, which is for block placements, apparently, not the blocks themselves which are implementations of BlockBase. Still learning my way through Drupal 8.
I still think there's value in providing at least some information about where the block came from, though, without depending on another module. block_class module will be cool but having the plugin ID is still more useful than nothing.
See attached, which makes it clear this is the block plugin id, with the class prefixed with "block-plugin-"
Comment #7
tim.plunkettYou'll need to update the corresponding test.
Comment #8
damienmckennaRerolled.
Comment #9
damienmckennaThe change isn't making any discernible difference to the output?
Comment #10
damienmckennaAH! This only affects the "Blocks page" display type, not a Panels page.
So yeah, once a Blocks Page is used you get extra classes, e.g. "block-plugin-search-form-block" for the core search block, "block-plugin-system-powered-by-block" for the "Powered by Drupal" block.
That said, I don't know if this is specifically useful when the blocks also have other classes, e.g. "block-search" and "block-system-powered-by-block" respectively for the two examples above.
At the very least if this is going to be included it'd be useful to also add some tests to confirm the output.
Comment #11
damienmckennaMissed a comma.
Comment #12
damienmckennaComment #13
damienmckennaI ported this change to Panels too: #2864483: Add block plugin class to block output
Comment #14
sadhanka commented