[Needs design] Previews of pages containing (dynamically) empty blocks are malformed

Created on 7 January 2025, 12 days ago

Overview

🐛 Empty global regions add unnecessary spacing to preview Active fixed the incorrect rendering of pages rendered not by the BlockPageVariant but by XB's PageTemplateDisplayVariant, when it is rendering blocks.

But, as demonstrated at #3497747-18: Empty global regions in PageTemplate add incorrect spacing, causing incorrect previews , a critical bug remains:

👆 Inside XB's preview while editing, the preview is very different from the end result, due to the need to visualize upon hovering component instances in the panel where on the page those component instances are placed.

😭 Unfortunately, the designs do not provide answers for the highly dynamic Block-sourced Components: it assumes all component instances in the panel are always visible. That's not true for many Block plugins:

  • messages block
  • local tasks block
  • local actions block

Proposed resolution

TBD. ⚠️ But this likely requires the use of \Drupal\Core\Plugin\PreviewAwarePluginInterface — because some blocks only render at all in specific contexts — for example: the messages block is only visible at all when there's actual messages.

See \Drupal\layout_builder\EventSubscriber\BlockComponentRenderArray::onBuildRender() (see also how that uses \Drupal\Core\Render\PreviewFallbackInterface), introduced in #3027653: Allow block and layout plugins to determine if they are being previewed and CR at https://www.drupal.org/node/3272267 . Note that this will also require core changes, because not all crucial block plugins implement that interface, which is why 🐛 Can't manage Status Messages block in Layout Builder Needs work still exists.

P.S.: This tangentially relates to 📌 [later phase] ApiPreviewController must use the previewed route's controller, and override canonical content entity routes' received entity object Postponed , because without that, the preview will not be accurate. Hopefully we can do that at a later time, though.

User interface changes

TBD

🐛 Bug report
Status

Active

Version

0.0

Component

Page builder

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.
  • Usability

    Makes Drupal easier to use. Preferred over UX, D7UX, etc.

Sign in to follow issues

Comments & Activities

  • Issue created by @wim leers
  • 🇺🇸United States effulgentsia

    I think we should make two quick hard-coded fixes for now, and then figure out how to abstract and provide UI affordance in a follow-up:

    • Don't render the Highlighted region within the canvas. It can still be in the layers panel.
    • Don't render the Help block within the canvas. It can still be in the layers panel.

    My premise here is that what editors generally want to see in the canvas is what their site visitors will see under typical circumstances, not what only other admin users will see or what site visitors will see only under rare circumstances. There's plenty of gray area around this, but the above two items seem pretty clear cut to me, and I wonder how much closer the canvas will get to the preview with just those two fixed.

  • 🇫🇮Finland lauriii Finland

    I agree with the premise set by @effulgentsia in #2. There was already some related discussion in 📌 Add support for global regions Active on this.

    Content creators not seeing the site as the site visitor is a pain point that comes up consistently in interviews with users. There are several actions that are already on the way to make the rendered preview represent more closely what an actual visitor of the site would see.

    We are already moving local tasks from the page to the navigation in Drupal Core in Create the Top Bar Needs review . The goal would be to do the same for local actions.

    Messages may need a special case along with main content and help because they are usually not changed after the initial configuration. The initial configuration could be done by a developer.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Per #4.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    The way both of you are talking about this, is about hiding irrelevant regions from the XB user. That's a different problem than the one described in the issue summary.

    We can do that, but then not here: I don't want to repeat the problem space I outlined in the issue summary that will still need to be resolved anyway.

    Hardcoded temporary work-around for hiding irrelevant regions in Olivero only, imperfectly

    I disagree with #2 (and hence #3) being a solution to the problem described in the issue summary.

    It would only solve the "problem" for a very small set of users:

    • Those using Olivero
    • Those who aren't putting similarly dynamically empty blocks in other regions: primary_menu, secondary_menu, hero

    All #2 would achieve is the out-of-the-box experience for Olivero users, without placing more blocks.

    I'm fine with doing what's proposed in #2, if it's something that we believe improves 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active for the Drupal CMS demoing purposes. But then it must be explicitly acknowledged that this MUST be removed prior to 1.0-stable.

    A possible long-term solution

    I agree that hiding certain regions is likely worthwhile, actually! (Related: Focus mode for global regions Active would become more usable/useful as a bonus!)

    Proposal:

    • Build upon this single checkbox:
    • … upon checking it, show a list of checkboxes: one per region, to allow opting specific regions out from editing through XB.
    • This information could trivially be stored in the PageTemplate config entity.
    • … we could then automatically respect that in ApiLayoutController + ApiPreviewController
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Issue created for the proposed long-term solution described in #6: 📌 PageTemplate: allow configuring which regions are exposed Active .

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #3498248 works. See #3498248-21: PageTemplate: allow configuring which regions are exposed .

    Restoring original title. I explained in #6 why #2 is about a different problem.

    That also means this is no longer a , because the proposal in #2 is achieved in 📌 PageTemplate: allow configuring which regions are exposed Active . 😊

Production build 0.71.5 2024