PageTemplate: allow configuring which regions are exposed

wim leers → credited effulgentsia → .

wim leers → credited lauriii → .

wim leers → changed the visibility of the branch 3498248-pagetemplate-exposed-regions to hidden.

wim leers → changed the visibility of the branch 3498248-pagetemplate-exposed-regions to active.

Configuration UI implemented:

Drupal CMS can preconfigure to omit the regions we think are causing confusion 😊

Do we think this is something we can remove easily later once we have cleaned things up? I'm not sure that this is the right solution for long term. If we can remove this easily later on, this seems ok as a stop gap but. In the long term, we need to make managing the whole site in XB not a confusing experience.

Sure we can. All this does is freeze the component trees in those regions at the time of PageTemplate creation, because they'd simply not be exposed as editable in the XB UI.

Based on what you wrote at #3497990-3: The XB canvas incorrectly renders blocks (and their corresponding margins as specified by the theme's CSS) that usually have empty content for most/all site visitors → , it sounded like something like this would be desirable long-term. Especially because … a theme might very well ship with a region that you never put anything into anyway, so it's then permanent noise 👻

For me, there's a LOT of regions in Olivero I never will use on https://wimleers.com — and this UI would literally allow me to ignore those regions 😄

#10 reminded me of the discussion in 📌 [later phase] [PP-1] Gracefully handle deleted regions in PageTemplate config entities" Postponed . I definitely agree that a lot of theme regions, especially from generic themes like Olivero, stay permanently empty on a lot of sites, so it seems generally useful to be able to say 'for the purposes of this site, this theme region doesn't exist so please ignore it'. It might also help for custom themes that are transitioning to XB from the block UI and gradually finding that they have regions that are no longer used which was the focus of that issue.

#11++

Configuration UI now works, and stores its information in the PageTemplate config entity:

And it works end-to-end. (Not yet passing tests though. Bugs probable. But it works.)

region removed, as per @effulgentsia's request:

… and now you can even restrict it down to say 3 useful regions:

#10 Sorry, I wasn't clear enough in #9 that this is definitely a positive change in the short term because it helps bridge the gap between what exists today and having an easy to use XB. I was primarily just concerned that this might be something that we would be stuck with even once we have transitioned into the XB way of building themes and sites in future. It sounds like that isn't a concern which is great. 😊👍

I think this looks great. I approved the MR for the subsystems I'm a code owner of, but it still needs additional approvals.

I can imagine a UI at some point that supersedes the need for this config (for example, eye icons within the Layers panel that toggle visibility), but I think this is a great step until that point, and even if we end up needing to deprecate this config after we provide upgrade paths, this should be easy config to deprecate following existing core patterns of deprecating config.

Does this fix 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active (or does that already not surface to begin with) in the case where someone doesn't check the "Use Experience Builder for this theme" checkbox? In that case, the regions aren't shown in the Layers panel, but they're still shown in the canvas, so my question is whether dynamically empty ones are shown there with extra margins?

I tested manually and the answer to #17 is that when the "Use Experience Builder for page templates in this theme" checkbox is off, then the canvas renders correctly (no whitespace coming from margins on empty regions). However, when I click the checkbox, then whether or not I select/deselect the per-region checkboxes, it has no effect on the canvas: the layers panel correctly hides the uneditable regions, but the canvas still seems to show whitespace for those uneditable empty regions. Is that expected? Perhaps this MR only improves the whitespace situation when used in conjunction with 🐛 Empty global regions add unnecessary spacing to preview Active ?

🥳 Glad @lauriii and @effulgentsia +1'd! Agreed with all prescriptions about the future by @effulgentsia in #15.

Thanks @longwave for getting this to green 🙏

#17 + #18: I expect this to fix that problem once rebased on 🐛 Empty global regions add unnecessary spacing to preview Active (which landed overnight), which I just did. It did not fix it. But AFAICT that's fixable.

Fixed that, but in doing so, found another bug: the more granular variant of 🐛 If an autosave entry exists before enabling global regions for a theme, theme regions cannot be seen Active , which @larowlan fixed yesterday. Rather than an autosave entry existing before enabling a PageTemplate, starting in this issue, it can be caused by changes to which regions are marked editable after an autosave entry was created.

Fixed. Still needs tests. But I need to move on to other things.

@larowlan already expanded ApiLayoutControllerTest in #3497744; this should build upon that work.

I think Lee is the ideal person to both review this and write that remaining bit of test coverage 😇 If somebody else is interested, don't hesitate to self-assign though!

Two screencasts attached that demonstrate it all in action.

Actually, let's first get @f.mazeikis to approve the config entity changes — leave the controller changes review to @larowlan, Felix 🙏

Per #3497990-8: [later phase] [Needs design] Previews of pages containing (dynamically) empty blocks are malformed → , this supersedes #3497990 in implementing @effulgentsia's proposal at #3497990-2: [later phase] [Needs design] Previews of pages containing (dynamically) empty blocks are malformed → . Will update the parent meta issue too.

Wrote unit tests. Only tests that are still missing: additional coverage in ApiLayoutControllerTest.

Left a review on the MR, nice one

Addressed all of @larowlan's feedback, and one of those remarks turned out to reveal pre-existing problems. I did a detailed walkthrough of the analysis + solution at https://git.drupalcode.org/project/experience_builder/-/merge_requests/5....

In doing so, I ended up writing the missing/needed test coverage! 😄🥳

Reviewed and tested - great work!

I addressed all of @larowlan's review points, and I'm pretty confident he'll like what he sees. Any remaining concerns I'd be happy to address in a follow-up MR on this same issue. Plus, he's working on lots of other important XB things!

Let's keep the train moving 😊