sdc ComponentSource pluginWhen a component is placed it is assumed that it contains at least some HTML, which is wrapped in comments and has a data attribute attached so the XB UI can manipulate it.
However, it is not mandatory for components to output any HTML at all, e.g. if the component contains defensive code such as
{% if image and image.src %}
<img src="{{ image.src }}" alt="{{ image.alt }}" />
{% endif %}
It is also possible for a component to output text, but not HTML.
block ComponentSource plugin#3497747: Global regions containing only "dynamically missing blocks" (due to emptiness or access) cause malformed pages fixed the incorrect rendering of pages rendered not by the BlockPageVariant but by XB's XbPageVariant, when it is rendering blocks.
But, as demonstrated at #3497747-18: Global regions containing only "dynamically missing blocks" (due to emptiness or access) cause malformed pages, a critical bug remains:
This is caused by blocks possibly resulting in an empty render array to render:
$content = $block->build();
if (Element::isEmpty($content)) {
$content['#access'] = $access;
return $content;
}
👆 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 Layers
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 Layers
panel are always visible. That's not true for many Block plugins:
TBD.
sdc componentsWhen we are previewing a component that does not contain any HTML tags, detect this special situation and wrap them in a container, so they can still be previewed and manipulated correctly by the UI.
block components⚠️ 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 #3137995: Can't manage Status Messages block in Layout Builder still exists.
P.S.: This tangentially relates to #3491701: [later phase] ApiLayoutController must use the previewed route's controller, and override canonical content entity routes' received entity object, because without that, the preview will not be accurate. Hopefully we can do that at a later time, though.
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | Screenshot 2025-01-08 at 5.08.49 PM.png | 85.24 KB | wim leers |
Comments
Comment #2
effulgentsia commentedI 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:
Highlightedregion within the canvas. It can still be in the layers panel.Helpblock 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.
Comment #3
lauriiiI agree with the premise set by @effulgentsia in #2. There was already some related discussion in #3489899: Add support for global regions 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 #3402046: Create the Top Bar. 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.
Comment #4
effulgentsia commentedComment #5
wim leersPer #4.
Comment #6
wim leersThe 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:
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 #3455753: Milestone 0.2.0: Early preview 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: #3497648: Focus mode for global regions would become more usable/useful as a bonus!)
Proposal:
PageTemplateconfig entity.ApiLayoutController+ApiPreviewControllerComment #7
wim leersIssue created for the proposed long-term solution described in #6: #3498248: PageTemplate: allow configuring which regions are exposed.
Comment #8
wim leers#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 #3498248: PageTemplate: allow configuring which regions are exposed. 😊
Comment #9
wim leersI think #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container is a duplicate of this. Let's see if @longwave agrees :)
Comment #10
wim leers… but #3518250 is for SDCs, wheras this one is for Blocks. I think we need a general solution for all component sources!
Comment #11
longwaveAha, good find - I agree that #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container is basically a duplicate, will close that one.
However I think we can try to do something here even without a design? There are two slightly different cases here: a component that's completely blank, and a component that contains some text but no HTML wrapper.
I think we need to ensure that we are at least outputting an HTML tag that can have the XB UUID attached so the front end can find something to receive the click/be draggable/etc. We could also add some text similar to the "broken" block in core if there is none at all in the output? Leaving the "needs design" tag but removing the postponed flag because I think this could be worked on now and iterated over time.
Comment #12
effulgentsia commentedNow that we have #3512455: Implement new drag-and-drop solution that removes the need to drop items into the preview iFrame, do we really need this? I think what's receiving the click now is a transparent box over the iframe, and the bounds of the component are known by the HTML comments before and after it, so we shouldn't need to wrap the bare text that's inside the iframe, or at least I hope we don't have to. It would be great to keep our previews accurate regardless of what CSS people happen to have by not having to insert any wrappers.
Comment #13
larowlanI still think it makes sense to use
\Drupal\Core\Render\PreviewFallbackInterface::getPreviewFallbackStringas there are some blocks that might be expecting thisWe could have the component source interface extend that and defer to each source on how to handle an empty component
Comment #14
wim leersMerged @longwave's observations from #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container into this issue.
Comment #15
wim leers@larowlan in #13: That is indeed a possibility, but it violates the promise of “accurate previews”. So I think @lauriii won’t want to follow Layout Builder’s example?
Comment #16
lauriiiLayout Builder has to use
\Drupal\Core\Render\PreviewFallbackInterface::getPreviewFallbackStringbecause it doesn't have an alternative way to interact with the blocks that have been placed. In Experience Builder, we do have the layers as an escape hatch.Relying on the layers isn't necessarily ideal so there's a trade-off between preview accuracy and discoverability of these components. I'll make sure to discuss this with @callumharrod and @reneelund in future but I don't think this is something we need to solve urgently.
I'm leaning towards removing this from stable blockers, and handling #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container separately as a stable blocker.
Comment #17
wim leers@lauriii #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container has already been closed in favor of this — were you really intending to link to that? 🤔
Comment #18
lauriiiI did. I think we should do the narrower scope from #3518250: [PP-1] Handle components instances rendering nothing: wrap them in a container for stable and leave this as a separate issue to deal with later.
Comment #19
wim leersAlright. Retitling/tightening scope, and contrasting it with #3518250. Please confirm that I interpreted your comments correctly — both here and at #3518250-7: [PP-1] Handle components instances rendering nothing: wrap them in a container. 🙏
Comment #20
wim leersComment #21
lauriiiComment #23
wim leersClosed #3557933: The system_messages_block block plugin does not support previews as a duplicate.