Impossible to drop a component into an empty column of the two

#3468049: Cannot place or drag child components landed.

Paired with @jessebaker, he let me know what's needed:

1. slots without components placed in them MUST contain <div class="sortable-list" data-xb-uuid="%s" data-xb-type="%s"></div> for it to be presented as a slot in the UI
2. slots without components placed in them BUT with default content (i.e. some markup) must wrap that default content in that same markup, to allow overriding that default content

On it.

SDC default slot value woes

Turns out that SDC does not support:

1. setting [#slots][$slot_name] = [] → you'll get: Drupal\Core\Render\Component\Exception\InvalidComponentDataException: Unable to render component "experience_builder:two_column". A render array or a scalar is expected for the slot "column_one" when using the render element with the "#slots" property in Drupal\Core\Render\Element\ComponentElement->generateComponentTemplate() (line 118 of core/lib/Drupal/Core/Render/Element/ComponentElement.php).
2. setting [$slots[$slot_name] = [#prefix => …, #suffix => …] → you'll get only the prefix + suffix

But it does support not specifying any value at all for a slot, then it'll happily use the default.

IOW: I cannot wrap the default value of a slot (using #prefix + #suffix) — doing so causes the default value to disappear 😬

Investigation

So I investigated, to understand how to respect the SDC metadata for the two-column component that @ctrlADel added in #3446722: Introduce an example set of representative SDC components; transition from "component list" to "component tree":

  column_one:
    title: Column One
    description: The contents of the first column.
    # @todo can_be_dropzone: true
    examples:
      - <p>This is column 1 content</p>
    # @todo this should be a dropzone
  column_two:
    title: Column Two
    description: The contents of the second column.
    # @todo can_be_dropzone: true
    examples:
      - <p>This is column 2 content</p>
    # @todo this should be a dropzone

And discovered that two-column.twig du:

<div {{ attributes }}>
    {% if block('column_one') %}
      <div class="column-one width-{{ width }}">
        {% block column_one %}
          The contents of the one column.
        {% endblock %}
      </div>
    {% endif %}

    {% if block('column_two') %}
      <div class="column-two width-{{ 100-width }}">
        {% block column_two %}
          The contents of the two column.
        {% endblock %}
      </div>
    {% endif %}
</div>

… as you can see, the Twig template duplicates the sample content. That is where the default was coming from: from the Twig template not the SDC metadata! 😅

Now that I understand that, I can continue figuring out a solution.

TIL #slotsAlter, while discovering #5.

🐛 The test coverage in \Drupal\KernelTests\Components\ComponentRenderTest::checkRenderElementAlters() is incorrect. It suggests you can choose which slot to alter, but you cannot: it alters the *value* of one slot at a time. (Unlike #propsAlter, which alters all props at the same time, just as the name + test logic suggests.)

Hopefully that won't block this issue; because fixing that core bug would come with nasty BC break consequences.

Upstream core bug reported for #6: #3469933: `#propsAlter` alters all props simultaneously, `#slotsAlter` does not, despite its name and test coverage.

Ready to be tested for confirmation that this makes the UI work as expected 😊

Great observation!

If I hack that together without writing code:

diff --git a/components/containers/two_column/two_column.component.yml b/components/containers/two_column/two_column.component.yml
index 4ba49c11..149b9bb2 100644
--- a/components/containers/two_column/two_column.component.yml
+++ b/components/containers/two_column/two_column.component.yml
@@ -18,7 +18,7 @@ slots:
     title: Column One
     description: The contents of the first column.
     examples:
-      - <p>This is column 1 content</p>
+      - <p class="xb-internal-component-example">This is column 1 content</p>
   column_two:
     title: Column Two
     description: The contents of the second column.

… then it looks like this:

… which makes me wonder: wouldn't it be better if:

• the entire column becomes the drop target? (i.e. the existing markup is overlaid with the gray diagonal lines that indicate a drop target)
• … which removes the need for the before vs after distinction
• … and would remove the "jumping" of the component just after dropping it

Assuming you'd agree that's a good idea, I implemented something different instead: annotated the div.sortable-list wrapper with data-xb-slot-is-empty when it contains no components:

WDYT? 😊

+1!

But currently the display: none is causing a somewhat jarring experience IMHO: it causes the drop zone to disappear mid-drag-and-drop, which even makes it unreachable if you drop it back in the original location:

1. So: when needing to "fly over" some slots to get to the "intended destination slot", we trigger unintended things. Is that what you mean?

   If correct: I think that could be a fine follow-up issue. That's a weakness that this MR reveals, it doesn't introduce it?

   i.e. I expected to be able to drag onto any part of the left columnin #18, instead of only the comparatively tiny target at the top.

2. AFAICT it's really just the need to revert the display: none if the drag-and-drop action was canceled (i.e. if the dragged sortable is restored to its original position).

LGTM! I can't merge this though — it needs approval from both back-end and front-end folks 😊 Signaling that this is ready for final review by marking RTBC.

Let's first get @tedbow to review the changes to ComponentTreeHydrated(Test).

Back-end pieces are in place. Now needs front-end sign-off to land 🤓

🥳 Thank you!