Problem/Motivation

A display pinned to a real page previews as that page. Only Page Layout implements DisplayBuildableInterface::getPreviewPagePath(), so an entity view override and a Views page display fall back to rendering their sources alone, outside the page they actually live in.

Two things also block a draft from surviving that sub-request:

  • The preview check is hand-rolled inside PageLayoutPageVariant, so no other buildable can reuse it.
  • The render cache answers the sub-request from the saved display, and the editor never sees their own unsaved edits. Left alone, it would also store the draft and serve it to the next visitor.

Proposed resolution

  • Add DisplayBuildableInterface::getSourcesForRender(), implemented once in DisplayBuildablePluginBase. Returns ::getSources() except while previewing this very display, where it returns the unsaved builder state. Leaves before loading the instance on every request that is not such a preview.
  • Implement ::getPreviewPagePath() for EntityViewOverride, and for ViewDisplay when the display is a page with a path carrying no argument placeholder.
  • Add PreviewRenderCache, decorating render_cache, dropping reads and writes for the duration of that one sub-request only.
  • Move the preview attribute read into DisplayBuilderHelpers::previewedInstanceId(), used by the controller, both caches, and the buildable base.

Remaining tasks

Review the render cache decorator. It sits on every request once the module is installed; its kernel test asserts it is inert outside the sub-request as carefully as it asserts it works inside one.

Issue fork display_builder-3618618

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mogtofu33 created an issue. See original summary.

mogtofu33 opened merge request !356

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs work » Needs review
Issue tags: +display_builder-1.0.0-beta7, +AI-accelerated
pdureau’s picture

pdureau commented

With the split, the reviews must be done following this order:

  1. #3618072: Placeholders and real render, in Canvas and Preview
  2. #3542796: Preview of view sources, because the MR is rebasing the previous one
  3. #3618618: Preview an entity view override and a view page display on their own page, because the MR is rebasing the previous one

So, this one last.

It may be the opportunity to check if we can use PreviewFallbackInterface from Core.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Needs work

Hi Jean,

#3618072: Placeholders and real render, in Canvas and Preview is merged, #3542796: Preview of view sources has been reviewed, i can review this one next. Can you rebase?

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs work » Needs review

Sure, ready to go.

pdureau’s picture

pdureau commented

ViewDisplay::getPreviewPagePath()

Feature review: Views

I have a View with a Page display managed by Display Builder: /articles

I add a block in Block Layout:

id: ui_suite_daisyui_buttonuisuitedaisyui
theme: ui_suite_daisyui
region: content
plugin: 'ui_patterns:ui_suite_daisyui:button'
settings:
  id: 'ui_patterns:ui_suite_daisyui:button'
  provider: ui_patterns_blocks
  ui_patterns:
    component_id: 'ui_suite_daisyui:button'
visibility:
  request_path:
    id: request_path
    negate: false
    pages: /articles

✅ I see the button in /articles and I see it in Display Builder preview panel.

Feature review: Entity Override

node.article.default view display is managed by Display Builder and is overridable. I create an override for /node/1

I add another block in Block Layout, with:

visibility:
  request_path:
    id: request_path
    negate: false
    pages: /node/1

❌ I see the button in /node/1 but i don't see it in Display Builder preview panel.

DisplayBuildableInterface::getSourcesForRender()

So, ::getSourcesForRender() is used for "real" rendering:

  • EntityViewDisplayTrait::buildMultiple()
  • PageLayoutPageVariant::build()
  • PreprocessViewsView::preprocessViewsView()

And ::getSource() is used for building.

It make sense to split the use, but would it be the opportunity to use the newly introduced 'ui_patterns:in_preview' boolean context?

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Needs work
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs work » Needs review

Fixed ViewDisplay::getPreviewPagePath().

For DisplayBuildableInterface::getSourcesForRender():

ui_patterns:in_preview only exists inside Island plugins, set by IslandPluginManager::createInstance(). It answers "is this block rendering inside the builder chrome" for one block, so it can draw a placeholder.

getSourcesForRender() runs on the plain render pipeline — buildMultiple(), page variant build, views preprocess — no Island, no such context. It answers "draft or saved sources" for the whole display, on every request including live ones.

Different question, different layer, not available at the call site.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Reviewed & tested by the community

ViewDisplay::getPreviewPagePath()

Same tests.

Feature review: Views

✅ I see the button in /articles and I see it in Display Builder preview panel.

Feature review: Entity Override

✅ I see the button in /node/1 but i don't see it in Display Builder preview panel.

DisplayBuildableInterface::getSourcesForRender()

I may be tired, but I have struggled a bit to understand the path of the logic here and how everything fit together. That why it took me a lot of time to do the review.

I put the ticket to RTBC, because such change is optional, but here is a little something we can do to make the logic more explicit?

For example, would it be more understandable to replace DisplayBuilderHelpers::previewedInstanceId(?Request $request): ?string by DisplayBuildablePluginBase::isPreview(string $instance_id): bool?

Or anything like that...

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau

I merge but keep open on you for follow up.

  • mogtofu33 committed 973f6c1a on 1.0.x 
    task: #3618618 Preview an entity view override and a view page display...
pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.