Changes to code components in global regions is not loaded until published

Created on 12 March 2025, 2 months ago

Overview

Changes made to code components in global regions are not reflected in the preview until changes to the code component are published, and page is refreshed.

Steps to reproduce

  1. Add a code component to a global region (e.g., header or footer).
  2. Make a change to the code component (e.g., update text or styles).
  3. Save the changes.
  4. Observe that the changes are not visible until the changes are explicitly published, and page is refreshed.

Proposed resolution

User interface changes

๐Ÿ› Bug report
Status

Active

Version

0.0

Component

Theme builder

Created by

๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @lauriii
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    So this does not happen when the auto-saved code component is in a content entity's XB field, and only if it is in a PageRegion?

    If so, this would be a missed edge case in ๐Ÿ“Œ Code Components should render with their auto-saved state(if any) when rendered in the XB UI Active .

    Or are you referring to the PageRegion itself being auto-saved?

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    So this does not happen when the auto-saved code component is in a content entity's XB field, and only if it is in a PageRegion?

    Yes, exactly. This is what I tried to describe in the steps to reproduce.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    25-min deep dive suggests this is because ๐Ÿ“Œ Code Components should render with their auto-saved state(if any) when rendered in the XB UI Active only modified \Drupal\experience_builder\Controller\ApiLayoutController::buildPreviewRenderable() to do:

    $renderable = $item->toRenderable(TRUE);
    

    which results in a JsComponent::renderComponent(isPreview: TRUE) call, rendering the auto-save (draft).

    But the page regions are not rendered in ::buildPreviewRenderable(), but in \Drupal\experience_builder\Plugin\DisplayVariant\XbPageVariant::build(), where there's still this:

          $fiber = new \Fiber(fn() => $component_tree->toRenderable());
    

    (note the absence of isPreview: TRUE)

    It'll be up to \Drupal\experience_builder\EventSubscriber\PreviewEnvelopeViewSubscriber::onViewPreviewEnvelope() to pass that information to XbPageVariant somehow.

    AFAICT the only feasible approach is to rely on this bit in HtmlRenderer:

          // Instantiate the page display, and give it the main content.
          $page_display = $this->displayVariantManager->createInstance($variant_id, $variant_configuration);
          if (!$page_display instanceof PageVariantInterface) {
            throw new \LogicException('Cannot render the main content for this page because the provided display variant does not implement PageVariantInterface.');
          }
          $page_display
            ->setMainContent($main_content)
            ->setTitle($title)
            ->addCacheableDependency($event);
          // Some display variants need to be passed an array of contexts with
          // values because they can't get all their contexts globally. For example,
          // in Page Manager, you can create a Page which has a specific static
          // context (e.g. a context that refers to the Node with nid 6), if any
          // such contexts were added to the $event, pass them to the $page_display.
          if ($page_display instanceof ContextAwareVariantInterface) {
            $page_display->setContexts($event->getContexts());
          }
    

    IOW: update XbPageVariant to implement ContextAwareVariantInterface. Then do something like \Drupal\display_variant_test\EventSubscriber\TestPageDisplayVariantSubscriber::onSelectPageDisplayVariant() to provide a yet-to-be-created "XB preview" context, which can then be respected by XbPageVariant, and which would result in $page_display->setContrexts(['xb_preview' => TRUE]);.

    The only alternative I see: add a #xb_preview => TRUE key-value pair to the "main content", which then is detected by XbPageVariant.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    โš ๏ธ AFAICT this also affects the component preview upon hovering the list of available components:

    diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    index 4fbac989c..adb573f8d 100644
    --- a/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    +++ b/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    @@ -302,7 +302,7 @@ final class BlockComponent extends ComponentSourceBase implements ContainerFacto
           return ['build' => []];
         }
     
    -    return ['build' => $this->renderComponent([], $component->uuid())];
    +    return ['build' => $this->renderComponent([], $component->uuid(), TRUE)];
       }
     
       /**
    diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/GeneratedFieldExplicitInputUxComponentSourceBase.php b/src/Plugin/ExperienceBuilder/ComponentSource/GeneratedFieldExplicitInputUxComponentSourceBase.php
    index c55528b5b..39653166a 100644
    --- a/src/Plugin/ExperienceBuilder/ComponentSource/GeneratedFieldExplicitInputUxComponentSourceBase.php
    +++ b/src/Plugin/ExperienceBuilder/ComponentSource/GeneratedFieldExplicitInputUxComponentSourceBase.php
    @@ -603,7 +603,7 @@ abstract class GeneratedFieldExplicitInputUxComponentSourceBase extends Componen
     
         return [
           'source' => (string) $this->getSourceLabel(),
    -      'build' => $this->renderComponent([self::EXPLICIT_INPUT_NAME => $default_props_for_default_markup], $component->uuid()),
    +      'build' => $this->renderComponent([self::EXPLICIT_INPUT_NAME => $default_props_for_default_markup], $component->uuid(), TRUE),
           // Additional data only needed for SDCs.
           // @todo UI does not use any other metadata - should `slots` move to top level?
           'metadata' => ['slots' => $this->getSlotDefinitions()],
    
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Extracting #5 into a separate issue, because as @f.mazeikis and @longwave pointed out: there's cache invalidation challenges there. So extracted A) preview-on-hover into ๐Ÿ› Changes to code components are not visible in preview-on-hover-component-list until published Active .

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    While reviewing another MR, I stumbled upon \Drupal\experience_builder\EventSubscriber\RenderEventsSubscriber::onSelectPageDisplayVariant(), which made me doubt my proposal for a second!

    But โ€ฆ that solves a different problem: that's for loading the auto-saved PageRegions when appropriate, not for loading the auto-saved JavaScriptComponents when appropriate.

    So: all good AFAICT ๐Ÿ‘

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    working on this

  • Merge request !846Resolve #3512385 Regions preview" โ†’ (Merged) created by tedbow
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    Gave this a start, I put a couple todo's in where I was user how to get the preview context.

    Manually testing it, it does work to pick up the autosaved Code component changes

    I can work on it tomorrow but if someone wants to take it over before then that works too

  • Pipeline finished with Failed
    about 1 month ago
    Total: 1577s
    #464973
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Manually testing it, it does work to pick up the autosaved Code component changes

    ๐Ÿฅณ

    Nice progress here! ๐Ÿ˜„

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 618s
    #467619
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    Assigning to myself to add tests. I looked at this last week but confused about the number of places/ways I could test this.

    Hopefully a couple days break will have given me clarity ๐Ÿคž๐Ÿป

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    To write a test I really wanted to understand how this all works

    Just noting that I just now realize how the html previewing works. I wasn't involved in these issue but wondering if we should document better how these classes relate to each other

    1. PreviewEnvelopeViewSubscriber
    2. PreviewEnvelope
    3. XBPreviewRenderer
    4. XbPageVariant
    5. RenderEventsSubscriber
    6. ApiLayoutController

    I will re-read the class docs again and see if they are clear but I think they might be clear only because I have actually done some research to see how all these classes fit together.

    I think each one this classes have good `@see` links to 1 or 2 of the other classes but I haven't yet found a place where it is clear how on the parts fit together.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    #15++ for improving docs here. I propose a class-level docblock on \Drupal\experience_builder\Controller\ApiLayoutController.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Very nice work here โ€” only trivial feedback, I think that once the docs exist, this is ready! ๐Ÿ˜„

  • Pipeline finished with Canceled
    about 1 month ago
    Total: 771s
    #468388
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 124s
    #468402
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 373s
    #468404
  • Pipeline finished with Failed
    about 1 month ago
    Total: 1417s
    #468413
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    @wim leers re #15 and #16

    Could we actually do that in another issue? I think doing it in another issue would allow us to rename a class or 2 to make the relationship more clear, if needed. Whereas if we just do it here I think we might just do quick job and keep the whole flow a bit hard to understand

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA

    re #18, I chatted with @wim leers and he is fine with this in a follow-up. I created ๐Ÿ“Œ Doc and otherwise make clearer how the 'html' preview element is generated Active

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States tedbow Ithaca, NY, USA
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • Pipeline finished with Success
    about 1 month ago
    #468457
  • There are two scenarios, one of which is working as expected, and the other is not:

    Working Scenario:
    When we click "Add to components" from the right-hand side section, changes made to code components in global regions are reflected in the preview immediately without needing to publish or refresh the page.

    Non-Working Scenario:
    When creating a code component and clicking "Add to components" from the left-hand side section, changes do not reflect in the preview. Below are the steps to reproduce the issue:

    1. Click "Add new"
    2. Enter the component name (e.g., myCode7).
    3. Do not make any changes to the code.
    4. Navigate to the left-hand side under the "Code" block.
    5. Click on the three dots of myCode7 component and select "Add to components".

    Hover functionality doesnโ€™t work, drag-and-drop of myCode7 does not show anything, and the preview remains blank.

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    I don't think that's a regression because we already have an issue for that: ๐Ÿ› Adding component to component library results in component code and configuration being lost Active .

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ
  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    I don't think this has been fixed yet at least for components that had been previously added to the component library, and then changed when they are placed in a global region:

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland

    I'm actually now realizing that my component wasn't in the header region so this isn't related to global regions ๐Ÿค” I'll report this as a new bug.

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

Production build 0.71.5 2024