Add setting to enable XB for page templates

Started this and immediately run into 🐛 third_party_settings in THEME.settings.yml cannot be dependency managed Needs work

Perhaps for now we just keep the setting at the top level of THEME.settings.yml.

We need a config option somewhere to override the default block layout and replace it with Experience Builder's page templates.

Why?

Back in 📌 Introduce an XB `PageTemplate` config entity Active , I already made it so that the existence of a PageTemplate config entity for the active (negotiated) theme would result in the BlockPageVariant page variant being ignored and PageTemplateDisplayVariant being used instead — see:

• logic: \Drupal\experience_builder\EventSubscriber\RenderEventsSubscriber::onSelectPageDisplayVariant()
• test coverage: \Drupal\Tests\experience_builder\Functional\PageTemplateVariantTest::test()

A new checkbox in theme settings allows site owners to enable XB at the page template level.

IMHO this makes things more complicated: either you have a PageTemplate config entity for a theme or you don't. If the concern is that you should be able to create an XB page template but then choose to not use it (for example because it isn't ready yet), then a clearer (with fewer moving config parts) would be to allow disabling a PageTemplate. That'd require supporting PageTemplate::$status, which exists already (thanks to ConfigEntityBase::$status) but is unused.

I want to stress that

[…] clearer (with fewer moving config parts) […]

is NOT a subjective statement. The issue summary proposes to add a new theme setting, i.e. a third party setting. This is not as easily deployable and packageable, because another environment may have other theme settings (third party settings) already set.

In other words: this objectively complicates product requirement 14. Configuration management..

Hm, OK. Felix and I discussed this previously and I thought we came to the conclusion that a checkbox setting was going to be needed somewhere, but perhaps this will work. Will switch tack to using the config entity status for now.

I still think in the future that a single page template per theme is going to be too limiting, but for now we can live with it.

t a checkbox setting was going to be needed somewhere

It's fine if the UI works the way this issue describes, but the value itself should be stored in this config entity for the reasons cited.

I still think in the future that a single page template per theme is going to be too limiting

That's what @lauriii spelled out in his 19. Modify the page template product requirement.

Context-dependent different "page templates" must be achieved using 41. Conditional display of components, per his requirements.

Discussed this with @f.mazeikis, @effulgentsia and @traviscarden.

We think there needs to be a checkbox to enable/disable this feature, at least for the foreseeable future, for the following reasons:

• When the user does not have a page template yet, what is the action that creates the page template? If this is an action purely in Experience Builder, that means we have to mark up at least the existing regions at all times even though the page template is not in use yet.
• When the user triggers the creation of the page template, what happens to the existing block layout?
• At present blocks can have conditions; until the XB UI has parity with the existing block UI in this regard site owners might not want to enable page templates, or if they do they need some way of switching them off again.
• We do not yet have separate permissions for content creators vs site owners, where the enabling/disabling and placing of components in non-content regions should be a site owner task.

For all these reasons it seems simplest to have a single checkbox in the theme settings. Until it is checked for the first time, the page template entity won't exist and rendering and preview will use block layout. Once it is checked then a page template entity will be created and that will be used for rendering and preview. If it is unchecked again then the page template entity will remain but the entity will not be used for rendering (haven't figured out what to do with preview here yet).

MR!471:

• Adds a checkbox to each theme settings page
• When first checked, a page template based on the existing block layout is automatically created

There is a test failure on Drupal 10 only, in the meantime assigning to Wim for review.

At present blocks can have conditions; until the XB UI has parity with the existing block UI in this regard site owners might not want to enable page templates, or if they do they need some way of switching them off again.

✅ Agreed. Full parity can only be achieved once product requirement 41 is implemented as mentioned in #6.

We do not yet have separate permissions for content creators vs site owners, where the enabling/disabling and placing of components in non-content regions should be a site owner task.

✅ Agreed. But that's true for literally everything in XB. Pre-existing issue for addressing that: 📌 SdcController cleanup tasks Active .

When the user does not have a page template yet, what is the action that creates the page template? If this is an action purely in Experience Builder, that means we have to mark up at least the existing regions at all times even though the page template is not in use yet.

✅ Agreed, great question! This is why I wrote in 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active for 19. Modify the page template.

When the user triggers the creation of the page template, what happens to the existing block layout?

"The existing block layout [for a theme]" is defined by a set of Block config entities (i.e. those with their theme key-value pair set to to that theme). Those aren't changed. They're simply ignored, because XB's PageTemplate "wins" in negotiating a page variant: see PageDisplayVariantSelectionEvent.

This is documented in 3.2 `PageTemplate config entity` in docs/config-management.md, and tested in \Drupal\Tests\experience_builder\Functional\PageTemplateVariantTest.

Until it is checked for the first time, the page template entity won't exist and rendering and preview will use block layout. Once it is checked then a page template entity will be created and that will be used for rendering and preview. If it is unchecked again then the page template entity will remain but the entity will not be used for rendering (haven't figured out what to do with preview here yet).

This is exactly what I proposed in #3?! 😅

For all these reasons it seems simplest to have a single checkbox in the theme settings.

… and I see that you implemented this in a way that it does NOT become part of olivero.settings (or generally: <theme name>.settings) — perfect! 😄👍

RTBC if it weren't for tests not passing. Approved the MR 😊

While out on a run, I realized that this bit is currently missing:

diff --git a/src/EventSubscriber/RenderEventsSubscriber.php b/src/EventSubscriber/RenderEventsSubscriber.php
index 02b12aa0..4860da04 100644
--- a/src/EventSubscriber/RenderEventsSubscriber.php
+++ b/src/EventSubscriber/RenderEventsSubscriber.php
@@ -36,7 +36,7 @@ final class RenderEventsSubscriber implements EventSubscriberInterface {
       ->load($active_theme_name);
 
     // For this theme, an Experience Builder PageTemplate config entity exists.
-    if ($page_template) {
+    if ($page_template && $page_template->status()) {
       $event->setPluginId('experience_builder_page_template_display');
       $event->setPluginConfiguration([
         PageTemplateDisplayVariant::PAGE_TEMPLATE_CONFIG_ENTITY_KEY => $page_template,

… which also made me realize that PageTemplateVariantTest must be updated slightly, because the

    // 5. Once an Experience Builder PageTemplate config entity is created for
    // the default theme, XB's PageTemplateDisplayVariant is used instead.

test coverage should test both with status: false and status: true.

I need this behaviour to unblock 📌 Implement auto-save of the page template config entity Active so going to get the tests passing, rebase and address #12

Tests are passing, MR is rebased and addressed #12

This is ready in my book but I can't merge it, needs another approval from Wim

I'll base 📌 Implement auto-save of the page template config entity Active off the top of it

In further testing, this yields invalid components -

AssertionError: assert($component instanceof Component) in assert() (line 51 of modules/experience_builder/src/Plugin/DataType/ComponentTreeHydrated.php).

This comes from the block.node_syndicate component.

Debugging further.

Added a failing test and fix for #15

This also results in duplicate blocks on page render, so we likely need more changes. Looking into them

The issue with #17 was because we skipped settings, added a test and fix for that - and removed the @todo

I'm getting close to figuring out the root cause of #17: the local tasks block appears once in Claro's header region, and once in its pre_content region. The first time with primary: true, secondary: false, the second time the other way around.

@larowlan's settings work does reflect those different settings per instance 👍, see:

Yet:

There must be some static caching or incorrect ID matching going on, should be a trivial fix. But first: dinner.

This should allow me to revert the AdminContext check.

Root cause found.

Was surprisingly tricky!

It's late now, I'll finish this tomorrow most likely.

MR is back to green

Yay for baby feedings! 😁🍼

HAH! This needs @mglaman's approval due to

/src/Entity/Page*

for the change to

src/Entity/PageTemplate.php

Follow-up issue+MR to fix that: 📌 Move config entity classes+interfaces from `\Drupal\experience_builder\Entity` to `\Drupal\experience_builder\Config\Entity` Active . Too much noise to do here.