Overview

When a in-browser component is placed inside a slot, auto-saved changes made to that components definition are not properly reflected in the preview canvas. The changes are successfully saved in the backend because they are loaded correctly when not placed inside a slot.

  1. Create a page with a component that includes a slot
  2. Add a in-browser code component inside a slot
  3. Add a in-browser code component outside the slot
  4. Make changes to the component (e.g., edit text)
  5. Observe that the component inside the slot hasn't been updated but the one outside has been updated.

Proposed resolution

User interface changes

CommentFileSizeAuthor
CleanShot 2025-04-19 at 11.56.16.gif1.21 MBlauriii
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:

Comments

lauriii created an issue. See original summary.

lauriii’s picture

Issue summary: View changes

lauriii’s picture

Status: Active » Needs review

wim leers’s picture

balintbrews made their first commit to this issue’s fork.

balintbrews’s picture

I went ahead and merged the MR — after thorough manual testing and reviewing the code. Keeping the issue open to add the necessary tests.

wim leers’s picture

Issue tags: +sprint

Now let's add the missing tests.

isholgueras made their first commit to this issue’s fork.

isholgueras’s picture

Assigned: Unassigned » wim leers
Status: Needs work » Needs review
wim leers’s picture

Assigned: wim leers » isholgueras
Status: Needs review » Needs work

Tests are failing, so no, it's not that simple 😄

When previewing, there are none of those wrapping HTML comments. So all the existing test expectations are no longer met.

Did you see @larowlan's test suggestion of 2 days ago on the already-merged MR?

isholgueras’s picture

Oh, I see. I've ran only the tests for ComponentTreeHydratedTest locally and it passed, but fails for ComponentTreeHydratedWithBlockOverride

If there are no comments, but the ComponentTreeHydratedTest tests pass... maybe there is something wrong here. I'll check.

Ok, I'll add more options to test.

Thanks!

isholgueras’s picture

Assigned: isholgueras » wim leers
Status: Needs work » Needs review

I've added the isPreview:true option to every single test to validate each.

When previewing, there are none of those wrapping HTML comments. So all the existing test expectations are no longer met.

I couldn't see any output difference (in terms of wrapping html comments). Everything is handled by each renderComponent

  • For BlockComponent, is only being used for$build['#xb_preview'] = $isPreview;.
  • For SingleDirectoryComponent, $isPreviewis not being used.
  • For JsComponent, $isPreview; is being used to generate the component URL, the CssLibrary, scopedDependencies, libraries and for the autosave.

With this in mind, the component types that are being tested in the root are SDC and Block (with the OverrideTest), but no JsComponent. JsComponent are added in the slots.

Should we add test for them too in the uuid-in-root ?

wim leers’s picture

I couldn't see any output difference (in terms of wrapping html comments).

Out of scope here — that's being handled in #3499352: SDCs should only have get HTML comments injected when `renderComponent(isPreview: TRUE)`.

[the different ways that isPreview is being respected

Out of scope here, being handled in #3492358: Provide ComponentSource-appropriate and -specific APIs to allow component instances to behave differently while being previewed.

What's missing here, and which will cause an output difference: a draft JavaScriptComponent config entity existing. Specifically, given the existing tests use it: a draft of the my-cta code component. Add something like this to \Drupal\Tests\experience_builder\Kernel\DataType\ComponentTreeHydratedTest::test():

    // But store an overridden version in auto-save (draft).
    /** @var \Drupal\experience_builder\AutoSave\AutoSaveManager $autoSave */
    $autoSave = $this->container->get(AutoSaveManager::class);
    $autoSave->save($code_component, $saved_component_values);

(source: \Drupal\Tests\experience_builder\Kernel\ApiLayoutControllerPostTest::testWithDraftCodeComponent())

That will cause "draft CSS+JS code component URLs" to appear in the resulting HTML.

isholgueras’s picture

That will cause "draft CSS+JS code component URLs" to appear in the resulting HTML

Perfect, I'll work on that. Thanks!

wim leers’s picture

#17 is not yet implemented, but this MR is sure looking great, plus I learned something new about PHP from @isholgueras! 😄

wim leers’s picture

isholgueras’s picture

Assigned: isholgueras » wim leers
Status: Needs work » Needs review

I think it's ready for review.

The only difference that I've found in JavaScriptComponents is when is a draft in preview, but I've tested, in a separate tests, the 4 different options, JavascriptComponent regular, in draft, in preview and draft in preview.

I've also adapted the tests for the BlockOverride, that otherwise it fails.

isholgueras’s picture

Assigned: wim leers » isholgueras
Status: Needs review » Needs work

There is a conflict with 0.x that I need to work on.

isholgueras’s picture

Assigned: isholgueras » wim leers
Status: Needs work » Needs review

Merge with 0.x is now complete and now the expected #is_preview works well. If I set it as preview, it returns the '#is_preview' => TRUE, otherwise is FALSE.

After rebasing 0.x all tests successfully failed 🤣.

Now it's ready for review.

wim leers’s picture

Assigned: wim leers » isholgueras
Status: Needs review » Needs work
isholgueras’s picture

Assigned: isholgueras » wim leers
Status: Needs work » Needs review
penyaskito’s picture

Assigned: wim leers » penyaskito
wim leers’s picture

Assigned: penyaskito » wim leers
wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests, -sprint

wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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