- Issue created by @jessebaker
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
I'm not sure I understand why/how yet, because
\Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::renderComponent()
uses'#theme' => 'block'
Related: 🐛 Placeholders/#lazy_builder is not supported for block component rendering Active + 🐛 Preview is not accurate for the site branding block Active .
Needs debugging to determine the root cause. Thanks for the detailed bug report!
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
D'oh — no, actually — I misinterpreted!
This is actually similar to 🐛 Enabling XB for managing page template breaks Olivero header Active , and there's nothing we can do about this.
The reason is essentially the same as #3505629-2: [upstream] Olivero's header works only correctly when using the "block" page variant → :
- Drupal core's
BlockPageVariant
renders the contents of block plugins with an extra wrapper: that's whatblock.html.twig
is about, and that's where<div class="block__content">
is coming from - XB is rendering all block plugins as-is, without adding more wrappers.
We could avoid this by doing something like core's
\Drupal\block\BlockViewBuilder::preRender()
, which is the "view builder" for theBlock
config entity type (which is IRRELEVANT/NON-EXISTENT when using XB!) that is responsible for this, see:// Place the $content returned by the block plugin into a 'content' child // element, as a way to allow the plugin to have complete control of its // properties and rendering (for instance, its own #theme) without // conflicting with the properties used above, or alternate ones used by // alternate block rendering approaches in contrib (for instance, Panels). // However, the use of a child element is an implementation detail of this // particular block rendering approach. Semantically, the content returned // by the plugin "is the" block, and in particular, #attributes and // #contextual_links is information about the *entire* block. Therefore, // we must move these properties from $content and merge them into the // top-level element.
- Drupal core's
- 🇫🇮Finland lauriii Finland
I do think we should mimic the block rendering from
\Drupal\block\BlockViewBuilder
so that same CSS that works outside of XB works when using blocks in XB. Ideally we should do this before stable but I don't think this needs to block the stable release. Worst case we'd document this in known issues. - 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
That'd mean using
'#theme' => 'block',
, but avoiding#contextual_links
(since it's not a block config entity) as well as#block
(same reason).@lauriii This has major implications, so please confirm once more you want the extra wrapper as I described in my previous comment: . I suspect the answer is "yes", with the rationale being:
In principle, this makes no sense at all for XB, those wrappers would be pointless cruft, but it is the choice that will maximize compatibility in the existing ecosystem, and hence it is the right choice.
Thanks! 🙏
- 🇫🇮Finland lauriii Finland
What are those major implications? Is it purely code / architectural or is there implications to the end-user?
The goal is to keep the rendering similar between XB and Blocks so that same CSS works with both. I don’t think we want to maximize the compatibility at any cost, which is why it would be good to understand what adverse effects this has.