Introduce an example set of representative SDC components; transition from "component list" to "component tree"

Created on 12 May 2024, 8 months ago
Updated 2 September 2024, 4 months ago

Having a set of example components to build against and test in various experiences will help to make sure experience builder supports as many different component building approaches as possible.

The MR adds example components that we can use to experiment/test/validate component config/data approaches while building out experience builder. See #9 for more nuance.

To start I've added a handful of components that build out the proposed supported approaches from 📌 Document supported component modeling approaches Active . There's a handful of straightforward components that are used to build up the complex components. The complex components included in the MR are:

How do I test these components?


This MR was not merged until the necessary infrastructure existed in the XB module to create and store a component tree. This issue/MR includes only a Component config entity in its default config (i.e. to make an SDC available in XB) for the components/containers/two_column SDC. 📌 Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria Fixed will make it possible to place more of these components.

XB's default component tree for the article content type in the Standard install profile has been updated to use those 2 columns, which means that now for the very first time, XB's default layout is an actual tree, with components in slots of another component:

⚠️ Note: not every of these components will necessarily work in XB immediately after this MR is merged! Having the components landed simplifies discoverability of unknown unknowns, and helps make abstract problems concrete 👍

Unexpected consequences

Note: The side by side appears to have already found a bug in SDC which I've opened 🐛 SDC incorrectly throws an exception about embedded slots Needs work to resolve. For now when turning on the submodule your frontpage will throw an SDC error. To get past this you can either comment out

core/lib/Drupal/Core/Template/ComponentNodeVisitor.php:165

      $error_messages[] = sprintf(
        'We found an unexpected slot that is not declared: [%s]. Declare them in "%s.component.yml".',
        implode(', ', $undocumented_ids),
        $component->machineName
      );

or disable the "SDC: Side by Side" block on the block layout page.

Why are there shoelace components?

I wanted to keep the actual html/js/css for components as simple as possible. By leveraging an already existing web component components the supporting code required for components stays small while allowing the examples to function like expected so issues can be spotted easily.

What's with all the todo's?

I added many todos to cover some of the intentions of the slots and props that are not yet supported by our APIs, like which slots should be dropzones, an example of how we could layout complex components in the SDC file, and $refs in the schema to embed definitions of other components in the complex one.

📌 Task
Status

Fixed

Component

Code

Created by

🇺🇸United States ctrladel North Carolina, USA

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

Merge Requests

Comments & Activities

  • Issue created by @ctrladel
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    🤩

  • 🇺🇸United States ctrladel North Carolina, USA
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    I tried to get this to work but ran into some challenges. I get that there are a bunch of @todos such as

    #      $ref: "json-schema-definitions://experience_builder_example_components.module/two_column"
          # @todo refs don't seem to work yet
    

    (Yep, that only works in the https://git.drupalcode.org/project/experience_builder/-/tree/research__d... branch right now!)

    But I did expect that I'd be able to kinda get this branch to work, since you didn't share any caveats/instructions on how to get it to work 😅

    This seems to be triggering a lot of SDC schema validation errors. I spent a good while stepping through the SDC schema validation process and fixed a few — the first one:

    Twig\Error\SyntaxError: An exception has been thrown during the compilation of a template ("We found an unexpected slot that is not declared: [column_one, column_two]. Declare them in "side_by_side.component.yml"."). in Twig\Environment->compileSource() (line 2 of modules/contrib/experience_builder/modules/experience_builder_example_components/components/compound/side_by_side/side_by_side.twig).
    

    required a work-around that I think goes against the intent of what you were trying to convey.

    The second change (a required label prop that AFAICT should've been optional) seems just a minor oversight.

    But then the third:

    Drupal\Core\Render\Component\Exception\InvalidComponentException: [width] String value found, but an object is required/n[width] Does not have a value in the enumeration [25,33,50,66,75] in Drupal\Core\Theme\Component\ComponentValidator->validateProps() (line 203 of core/lib/Drupal/Core/Theme/Component/ComponentValidator.php).
    

    had me to step through a significant part of the schema validation process to make sense of what was going on.

    So … rather than me spending multiple hours trying to figure this out while being uncertain of the intended direction … could you perhaps provide some additional context, or just instructions on how to see the whole thing in action? 😇🙏

    EDIT: cross-posted with #4 — yeah, I definitely don't get to see that point 😅

  • 🇺🇸United States ctrladel North Carolina, USA

    Can you give it another try? I pushed a fix for the width issue, looks like something I broke while doing clean up before opening the MR.

  • 🇫🇷France pdureau Paris

    Hi,

    As I already did for a few themes:

    Here is some feedbacks about the SDC components which may be helpful.

    my-banner

    3 (ctaText, ctaHref & ctaTarget) of the 5 props are not related to the banner but the CTA which is hardcoded in the template:

    {% include 'experience_builder_default_design_system:my-cta' with { text: ctaText, href: ctaHref, target: ctaTarget } only %}

    Also, ctaText prop definition is less strict than my-cta's href prop definition.

    Injecting the CTA component through a slot would prevent this kind of issue and make the implementation cleaner.

    About this:

    {% if image is not empty %}

    It is better to use {% if image %} instead to catch when the image is missing or resolved to false for other reasons.

    image prop looks like an URL (iri-reference) instead of a plain string.

    my-button

    text prop may be a slot instead

    my-cta

    href prop : uri JSON schema format is too restrictive (relative-reference & internationalization are not allowed). It is better to use iri-reference.

  • Pipeline finished with Failed
    7 months ago
    Total: 196s
    #190131
  • 🇺🇸United States ctrladel North Carolina, USA

    ctrlADel changed the visibility of the branch experience_builder-3446722-2 to hidden.

  • Pipeline finished with Failed
    7 months ago
    Total: 199s
    #190133
  • 🇺🇸United States ctrladel North Carolina, USA

    @pdureau this branch/MR was created before there was a 0.x and there were some previous components pulled from SDC at the top level which it looks like you reviewed. I just rebased the MR onto 0.x so now the only example components are the ones in the experience_builder_example_components submodule, would be great to get your thoughts on those.

    Knowing that XB will likely require some changes to SDC schemas and that there are many features that need to be built on top of the schemas the overall goal here is to provide a diverse set of example implementation approaches to ensure that XB doesn't unintentionally limit how components can be built/used. A smaller set of example components also has the benefit of being easier to update and adjust as extra settings are added/removed from the schema. The full design system implementations you mentioned will be great to use as a more thorough test once XB is a bit better defined.

    Would be interested to know If there are any common component structures/approaches you use that are missing from the examples and what would be a good example we could use to add them in.

  • 🇫🇷France nod_ Lille

    if it's for testing it should probably be in a testing module no? If it's in an example module it makes it seem like that's what we recommend doing vs. what's possible to do.

  • 🇫🇷France pdureau Paris

    now the only example components are the ones in the experience_builder_example_components submodule, would be great to get your thoughts on those.

    Thanks. I would be pleased to have a look.

    Would be interested to know If there are any common component structures/approaches you use that are missing from the examples and what would be a good example we could use to add them in.

    We (UI Suite people) are working on a component template validator which will catch many of those feedbacks, but not all of them. It will be ready this summer.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    I never responded here since DrupalCon, but I never forgot about @ctrlADel's awesome work here! 😅

    I've been wanting to get to this issue, and this issue's MR, which is now the oldest open MR for XB 😬

    To be able to merge this MR, the field storage pieces that https://git.drupalcode.org/project/experience_builder/-/merge_requests/15 (no d.o issues yet back then) added and the subsequently added terribly hacky editing experience to demonstrate the data model ( https://git.drupalcode.org/project/experience_builder/-/merge_requests/20 aka #3452497) need to first grow to allow an actual component tree to be defined.

    There's no support for nesting components currently.

    I hope to continue pushing this forward next week, but chances are it'll be longer still before I get to this. Because in the very short term, even being able to edit a single component's props completely through the UI has many other moving parts that need to be addressed.

    But rest assured, I will get to this 👍😊

  • Assigned to wim leers
  • Status changed to Postponed 7 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Note: when working on this, I'll also port relevant pieces from 🌱 [PP-1] Create components for a default design system Postponed , if any.

  • Issue was unassigned.
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Blocked on 📌 FieldType: Support storing component *trees* instead of *lists* Fixed — otherwise the scope of this MR would be far too big.

  • 🇺🇸United States Kristen Pol Santa Cruz, CA, USA

    @pdureau

    We (UI Suite people) are working on a component template validator which will catch many of those feedbacks, but not all of them. It will be ready this summer.

    Sounds great! Is this based on this work?

    https://git.drupalcode.org/project/ui_patterns/-/tree/2.0.x/modules/ui_p...

  • 🇫🇷France pdureau Paris

    Hi Kristen,

    This sub-module: https://git.drupalcode.org/project/ui_patterns/-/tree/2.0.x/modules/ui_p...

    • You can clone the 2.0.x branch
    • Once the module installed, you can go /admin/reports/ui-components
    • We are still tweaking the rules and messages, but the "framework" is already there and working
    • We will add a drush command later
  • 🇺🇸United States michaellander

    Is it worth including some examples to pressure test on the extreme end of low-code customization? For example a heading where you can adjust font-style, font-weight, heading level, margins(top, bottom, left, right), padding(top, bottom, left, right), display, position(top, bottom, left, right), text properties(color, align, decoration, etc), height, width, line-height, etc etc...

    This would be both useful from performance/storage perspective, but also in some UI decisions.

  • 🇺🇸United States Kristen Pol Santa Cruz, CA, USA

    Thanks @pdureau :)

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    📌 FieldType: Support storing component *trees* instead of *lists* Fixed is in.

    @tedbow is working on 📌 Create validation constraint for ComponentTreeStructure Needs work . That will ensure that the tree structure for components with slots is strictly validated 👍

    Once that's in, rebase Kyle’s aka @ctrlADel’s MR here, which adds components with slots. Once rebased:

    1. update the default XB field value so that an article uses Kyle's side-by-side component by default, and has two of the currently existing default components side-by-side (one in each slot)
    2. that will require updates to \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::resolveComponentProps() and \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated
    3. … but not to \Drupal\experience_builder\Plugin\DataType\ComponentPropsValues nor \Drupal\experience_builder\Plugin\DataType\ComponentTreeStructure

    Strictly speaking, that would be out of scope, but this issue is the one with an existing MR that has slots for its SDCs, so it's the perfect place to do those last pieces of work after #3460856 is done :)

  • Assigned to tedbow
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @tedbow will be working on #20 once he finishes the issue blocking this one: 📌 Create validation constraint for ComponentTreeStructure Needs work . I just reviewed it, and it's close 👍

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    While Ted starts working on this, 📌 Refactor SdcController::preview() to use ComponentTreeHydrated, and update it to support nested components Fixed can happen in parallel. Then this issue updating \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated will automatically result in SdcController::preview() supporting actual component trees too 👍

  • First commit to issue fork.
  • Status changed to Needs work 5 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @tedbow This should save you a ton of time: 📌 Refactor SdcController::preview() to use ComponentTreeHydrated, and update it to support nested components Fixed — I mentioned this in #22.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    … I just realized something: #3463986-5: Refactor SdcController::preview() to use ComponentTreeHydrated, and update it to support nested components 🙈 — but hey, it'll make the remaining work here smaller 😊

  • 🇮🇹Italy kopeboy Milan

    I would add at least a default component that has a form input, like an add to cart block or a donate/deposit/withdraw one.

  • 🇫🇷France pdureau Paris

    @pdureau this branch/MR was created before there was a 0.x and there were some previous components pulled from SDC at the top level which it looks like you reviewed.

    Thanks @ctrlADel for the information. I got a look and here are some feedbacks.

    experience_builder:image

    $refs don't seem to work yet, so the component was not fully tested, but the component template doesn't make sense: <img src="{{ image.src }}" alt="{{ image.alt }}" width="{{ image.width }}" height="{{ image.height }}"></img>

    What is the purpose of this component?

    • it has a single prop, also called "image", so tje component looks like a meaningless wrapper around this single prop
    • it doesn't use the expected attributes object
    • no HTML classes (and the component has no dedicated CSS), no specific UI purpose
    • it is nothing more than a rigid HTML element, like an image renderable but less powerful because with hardcoded attributes (so not compatible with lazy loading or any API working with images)

    Am I missing something?

    my-experience_builder:hero

    If there is already a class attribute in attributes, two class attributes will be printed:

    <div {{ attributes }} class="my-hero__container">
    

    It would be better to do:

    <div {{ attributes.addClass("my-hero__container") }}>
    

    Adding attributes in the prop definition is not harmful, but not useful because always automatically added:

        attributes:
          type: Drupal\Core\Template\Attribute

    name: Attributes

    No slots, is it normal? heading, subheading, cta1 & cta2 look like slots.

    experience_builder:my-section

    The template doesn't use the expected attributes object.

    The heading prop machine name and label doesn't match. Is it "heading" or "text"?

        text:
          type: string
          title: Heading
    

    No slots, is it normal? text looks like a slot.

    Other components

    From the example module, with a lot of "todo" annotations, so is it too early to have a look?

  • 🇺🇸United States tedbow Ithaca, NY, USA

    I am trying to update `config/optional/field.field.node.article.field_xb_demo.yml` use the new Side By Side component

    I am able to fill the slots correctly I think but not sure what to do with the properties, specifically what to put for Heading property
    for example

    modules/experience_builder_example_components/components/compound/side_by_side/side_by_side.component.yml has

        heading:
          title: Heading
    #      $ref: "json-schema-definitions://experience_builder_example_components.module/heading"
          # @todo refs don't seem to work yet
          type: object
    

    is " # @todo refs don't seem to work yet" stil right
    If I uncomment the "$ref:" line I get

    TypeError: Drupal\experience_builder\SdcPropJsonSchemaType::from(): Argument #1 ($value) must be of type string, null given in Drupal\experience_builder\SdcPropJsonSchemaType::from() (line 205 of modules/contrib/experience_builder/src/FieldForComponentSuggester.php).
    Drupal\experience_builder\FieldForComponentSuggester->getRawMatches('experience_builder_example_components:side_by_side') (Line: 59)
    Drupal\experience_builder\FieldForComponentSuggester->suggest('experience_builder_example_components:side_by_side', Object) (Line: 284)
    Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem->getAvailablePropSourceChoices() (Line: 74)
    Drupal\experience_builder\Plugin\Field\FieldWidget\TwoTerribleTextAreasWidget->formElement(Object, 0, Array, Array, Object) (Line: 459)
    Drupal\Core\Field\WidgetBase->formSingleElement(Object, 0, Array, Array, Object) (Line: 219)
    Drupal\Core\Field\WidgetBase->formMultipleElements(Object, Array, Object) (Line: 120)
    Drupal\Core\Field\WidgetBase->form(Object, Array, Object) (Line: 190)
    Drupal\Core\Entity\Entity\EntityFormDisplay->buildForm(Object, Array, Object) (Line: 121)
    Drupal\Core\Entity\ContentEntityForm->form(Array, Object) (Line: 134)
    Drupal\node\NodeForm->form(Array, Object) (Line: 107)
    Drupal\Core\Entity\EntityForm->buildForm(Array, Object)
    call_user_func_array(Array, Array) (Line: 536)
    Drupal\Core\Form\FormBuilder->retrieveForm('node_article_form', Object) (Line: 284)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult(Object, Object)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
    ....

    Leaving it commented out for now

    Not sure how to fill that out in the props array. In the editor when I adding an article I get

    ⚠️ This component prop has a shape that has no equivalent in Drupal fields — yet.

    if I try to added an article with default values, plus uploading 2 images, for the fields I get

    TypeError: Twig\TemplateWrapper::render(): Argument #1 ($context) must be of type array, null given, called in /Users/ted.bowman/sites/exp-d-core/vendor/twig/twig/src/Extension/CoreExtension.php on line 1332 in Twig\TemplateWrapper->render() (line 36 of vendor/twig/twig/src/TemplateWrapper.php).
    Twig\Extension\CoreExtension::include(Object, Array, 'experience_builder_example_components:heading', NULL, ) (Line: 56)
    __TwigTemplate_b267b61305cf51ea791258bf2a6a18e5->{closure}() (Line: 1775)
    Twig\Extension\CoreExtension::captureOutput(Object) (Line: 53)
    __TwigTemplate_b267b61305cf51ea791258bf2a6a18e5->doDisplay(Array, Array) (Line: 360)
    Twig\Template->yield(Array, Array) (Line: 125)
    __TwigTemplate_a43b7fe6eafe0515efeef249f410eb00___1303851157->doDisplay(Array, Array) (Line: 360)
    Twig\Template->yield(Array) (Line: 40)
    __TwigTemplate_a43b7fe6eafe0515efeef249f410eb00->doDisplay(Array, Array) (Line: 360)
    Twig\Template->yield(Array) (Line: 335)
    Twig\Template->render(Array) (Line: 38)
    Twig\TemplateWrapper->render(Array) (Line: 234)
    Drupal\Core\Template\TwigEnvironment->renderInline('{# inline_template_start #}{# This template was dynamically generated by single-directory components #}
    {% embed 'experience_builder_example_components:side_by_side' %}
    {% block column_one %}
    {{ column_one }}
    {% endblock %}
    {% block column_two %}
    {{ column_two }}
    {% endblock %}
    {% endembed %}
    ', Array) (Line: 54)
    Drupal\Core\Render\Element\InlineTemplate::preRenderInlineTemplate(Array)

    So seems like the expected problem of modules/experience_builder_example_components/components/compound/side_by_side/side_by_side.component.yml not sending any value on to modules/experience_builder_example_components/components/simple/heading/heading.component.yml

    When I look at the page output of EndToEndDemoIntegrationTest when fails this is the same.

    Basicalyy

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Talked to @Wim Leers briefly about this and we agreed it will better to change the Side By Side to simplify or remove the props and focus on getting the slots working.

    will work on that

  • 🇺🇸United States Kristen Pol Santa Cruz, CA, USA

    I'm unfamilar with "shoe" design things so I asked chatgpt about it... would love more explanation :)

    In a design system, the term "shoe button" isn't a widely recognized standard term. It could be a specific term used within a particular organization or design system to refer to a unique button style or function. In the context of user interface design, buttons can have various styles and functionalities, often named descriptively based on their appearance or use case.

    This is in the summary but I don't know what that means... where are these from? Sorry if this is obvious :)

    Why are there shoelace components?
    I wanted to keep the actual html/js/css for components as simple as possible. By leveraging an already existing web component components the supporting code required for components stays small while allowing the examples to function like expected so issues can be spotted easily.

  • 🇫🇷France pdureau Paris

    Hi Kristen,

    I understood "shoe" is about https://shoelace.style/ here.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    @Wim Leers re #31 and #20 I think I am going to switch from using modules/experience_builder_example_components/components/compound/side_by_side to using
    modules/experience_builder_example_components/components/containers/two_column this is simpler example. Side by Side passes object to sub components and two_column simply gives uses 2 slots to put components in .

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #34: WFM!

  • 🇺🇸United States tedbow Ithaca, NY, USA
    1. re #20 and work @Wim Leers laid out

      update the default XB field value so that an article uses Kyle's side-by-side component by default, and has two of the currently existing default components side-by-side (one in each slot)

      I changed this to use the Two Column see #34

      that will require updates to \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::resolveComponentProps() and \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated

      No changes were needed. I think because the changes needed were done in 📌 Refactor SdcController::preview() to use ComponentTreeHydrated, and update it to support nested components Fixed

    2. \Drupal\Tests\experience_builder\Functional\EndToEndDemoIntegrationTest::test passes for me locally but fails on gitlab because $tree->getComponentInstanceUuids() seems return the UUIDs in different order. I am giong to add a sort so this is predictable.
    3. I can't get EndToEndDemoIntegrationTest::test to pass. I tried to debug this but can't see a difference in the values
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #36.2: 🤔 How is that possible? \Drupal\experience_builder\Plugin\DataType\ComponentTreeStructure::getComponentInstanceUuids() does this:

    return array_column($this->getComponents(), 'uuid');
    

    … and it's coming from a JSON blob that was deserialized. How can the order possibly change?! If the order can change, that would also mean the component tree could randomly shuffle around?! 😱

    What am I missing?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Also, for some reason these very simple steps that manually reproduce EndToEndDemoIntegrationTest no longer work:

    vendor/bin/drush si standard --account-name=root --account-pass=root --yes
    vendor/bin/drush pm:install experience_builder --yes
    

    (They're documented at the top of /CONTRIBUTING.md.)

    The node.article.field_xb_demo field instance is simply not installed. I've tried it twice now, and I'm on the latest commit: 5b192757b1672a16a3fa227938cfe0e8549a3915. No relevant log messages after those two steps.

    Any idea what's going on?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #36.3: I’ve had to debug that DefaultConfigTest before too — with the right conditional breakpoint it’s totally doable. So far it’s never lied to me. But the failing test output can sure be confusing.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    This was because I needed add experience_builder_example_components as a dependency of the main module since it is needed for the default value of the field_xb_demo now. See my last commit

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    I did a high-level review of @tedbow's changes. I didn't like how the SDCs that @ctrlADel built had started deviating significantly.

    So:

    1. I merged in origin/0.x to get 📌 Introduce `hook_storable_prop_shape_alter()`, use it to prefer the Media Library widget for "image" PropShape if Media Library is installed Fixed , which introduced some crucial bugfixes to massageFormValuesTemporaryRemoveThisExclamationExclamationExclamation().
    2. That in turn allows reverting the change @tedbow made to the side-by-side component to not use a integer + enum.
    3. … but that revealed a limitation of the JSON encoding (which is also true of the YAML encoding): all keys are strings. But the keys were meant to be the values. Result: 50 as a choice would always be represented by '50': a string, not an integer!
    4. In debugging that, I discovered ::storageSettingsFromConfigData(), which was added ~10 years ago for addressing precisely this problem. Whew! 😅 Fix: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...
    5. In #3461499, I added fallback logic to ComponentPropsForm to pick the appropriate widget while we await follow-up issues to land (there's explicit @todos in place for that). But I did not update TwoTerribleTextAreasWidget accordingly. And that's what @tedbow was running into here. Fixed that: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...
    6. But then I found another bug, this time one deep in Drupal core: ListIntegerItem and its default widget actually store an string value … 🙃 It's missing the explicit cast it needs: calling getValue() on a ListIntegerItem actually returns '50', not the 50 its allowed values specified! 🤯 Fixed that in https://git.drupalcode.org/project/experience_builder/-/merge_requests/2....

    PHPUnit tests are passing again after I first broke them 😊

    So, now @tedbow can start his week with something closer to what @ctrlADel envisioned 👍

    I think we should split off https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... into its own issue — I'll ask Utkarsh to do that 😊

  • 🇺🇸United States Kristen Pol Santa Cruz, CA, USA

    🎉 Y’all going to find and fix all the core bugs 😂

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Putting notes here for myself tomorrow or @Wim Leers if you wanted to take a look. But I will look tomorrow if no one else does. Below is as far as I was able to debug before my end of day

    Just the cypress e2e test failing now. I think this because \Drupal\experience_builder\Controller\SdcController::preview is not sending back results with the slots.

    If I make a node and visit node/1 I get the components rendered in the 2 column layout. But if I visit xb/node/1 I see the default values for the slots of modules/experience_builder_example_components/components/containers/two_column/two_column.twig "The contents of the one column." and "The contents of the two column."

    Putting debug points in \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated::toRenderable
    when I go to node/1 in here

    $renderable_component_tree = $this->getValue();
    

    results in the slots being filled and $renderable_component_tree->getContent() is

    {"a548b48d-58a8-4077-aa04-da9405a6f418":{"two-column-uuid":{"component":"experience_builder_example_components:two_column","props":{"width":50},"slots":{"column_one":{"dynamic-image-udf7d":{"component":"experience_builder:image","props":{"image":{"src":"public:\/\/2024-08\/heading-no-field-for-shape_6.png","alt":"asdf","width":614,"height":206}}},"static-static-card1ab":{"component":"experience_builder:my-hero","props":{"heading":"hello, world!","cta1href":"https:\/\/drupal.org"}}},"column_two":{"dynamic-static-card2df":{"component":"experience_builder:my-hero","props":{"heading":"asdfasdf","cta1href":"https:\/\/drupal.org"}},"dynamic-dynamic-card3rr":{"component":"experience_builder:my-hero","props":{"heading":"asdfasdf","cta1href":"public:\/\/2024-08\/heading-no-field-for-shape_6.png"}},"dynamic-image-static-imageStyle-something7d":{"component":"experience_builder:image","props":{"image":{"src":"http:\/\/exp-d-core.test\/sites\/default\/files\/styles\/thumbnail\/public\/2024-08\/heading-no-field-for-shape_6.png.webp?itok=FuRwXRQg","alt":"asdf","width":100,"height":34}}}}}}}}
    

    when I go to xb/node/1

    Putting debug points in \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated::toRenderable
    when I go to node/1 in here

    $renderable_component_tree = $this->getValue();
    

    results in the NOT slots being filled and $renderable_component_tree->getContent() is just
    {"a548b48d-58a8-4077-aa04-da9405a6f418":{"two-column-uuid":{"component":"experience_builder_example_components:two_column","props":{"width":50}}}}
    So \Drupal\experience_builder\Plugin\DataType\ComponentTreeHydrated::renderify does not call itself recursively to fill the slots as "slots" is not present.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    If I make a node and visit node/1 I get the components rendered in the 2 column layout.

    I do too, but it looks awfully off, because the first column is 3456 pixels wide, whereas the second is only 140 pixels wide? 😅 (Fresh install, browser caches wiped.) → 🐛

    But if I visit xb/node/1 I see the default values for the slots of modules/experience_builder_example_components/components/containers/two_column/two_column.twig "The contents of the one column." and "The contents of the two column."

    Reproduced.

    That's because the UI/client provides this request body:

    {
      "layout": {
        "uuid": "root",
        "type": "root",
        "name": "root",
        "children": [
          {
            "uuid": "two-column-uuid",
            "nodeType": "component",
            "type": "experience_builder_example_components:two_column"
          }
        ]
      },
      "model": {
        "two-column-uuid": {
          "width": 50,
          "name": "Two column"
        }
      }
    }
    

    … because that is what the client received as a response from /api/layout/node/1.

    That's why you observe different behavior:

    • visiting /node/1 results in the stored component tree being loaded and hydrated and then rendering correctly
    • using the XB UI at /xb/node/1 results in the incorrect layout (top level only: a list, not a tree) being passed from server to client (you need to update SdcController::layout() to transform the server-side data structure to the client-side equivalent), and that same incorrect layout is then sent to the server for previewing, which is transformed back to the server-side data structure, but since it is incomplete, you get a weird hydrated tree: the tree the client sent because the server provided it

    IOW: this MR still needs to update \Drupal\experience_builder\Controller\SdcController::layout() 📌 Refactor SdcController::preview() to use ComponentTreeHydrated, and update it to support nested components Fixed only updated SdcController::preview(). You essentially now need to write the inverse of SdcController::clientLayoutToServerTree(), which is a helper I introduced in that issue.

    (Eventually the data models might become closer to each other or even the same, but for now, that's not the case. I started pushing 📌 Create an Open API spec for the current mock HTTP responses Fixed forward because it has not been finished yet, and will help clarify this.)

    The fact that you're at this point now means you're probably in the final stretch! 😄🥳

    P.S.: where you wrote , that should've been /xb/node/1. Fortunately it was clear from context 👍

  • 🇺🇸United States tedbow Ithaca, NY, USA

    @Wim Leers thanks for the context

    P.S.: where you wrote when I go to node/1 in here, that should've been /xb/node/1. Fortunately it was clear from context 👍

    fixed

  • 🇺🇸United States tedbow Ithaca, NY, USA
    1. Ok. now I think I have handled the problems I mentioned in #45 and @Wim Leers response in #46
    2. Currently I can now go to xb/node/1 and see the components being rendered. Also SdcController::wrapComponentsForPreview is now working on the nested components in the slots. When I hover over the components I can see the highlight outline which I think depends on the div added by wrapComponentsForPreview
    3. This does get more of the Cypress tests passing. You can see before my latest push of commits there were 4 screenshots for failed tests in a11y.cy.js and the now there is just 1 🎉
    4. The last failed test for a11y.cy.js is for the form loading. I have confirmed locally the forms do not load when you click a component. Debugging at \Drupal\experience_builder\Form\ComponentPropsForm::buildForm and trying this on 0.x and the MR I think this because on the MR for some reason tree is being sent back to the server as an empty array when trying create the form.

      Maybe something needs to change on the client, I am not sure

    5. There are a few more Cypress test failing but I have not had a chance to investigate them
  • 🇺🇸United States tedbow Ithaca, NY, USA

    With some help from @hooroomoo I was able to figure where the JS that caused #48.3

    I pushed up some hacky JS to get the forms to work. I don't have time to see if this causes eslint problems, but probably. Can fix next. Will see if it passes the Cypress test. Locally the forms open now🎉

  • 🇺🇸United States tedbow Ithaca, NY, USA
    1. So my previous JS commits broke UI build gitlab job, so the Cypress tests didn't run
    2. Fixed that got the UI build gitlab job passing
    3. The Cypress tests ran again.
    4. All the cypress tests in a11y.cy.js are now passing 🎉
    5. The fix for a11y.cy.js also fixed a couple other tests. Went from 7 passing, 4 failing to 9 passing, 2 failing 👍
    6. I haven't had a chance to look at the remaining 2 failing Cypress tests. I am up for trying to fix these so will leave the issue assigned to me but if someone else with more express with Cypress want to get these passing that is fine too. but it might need some server side changes also
  • Assigned to wim leers
  • Status changed to Needs review 5 months ago
  • 🇺🇸United States tedbow Ithaca, NY, USA
    1. Fixed add-section.cy.js

      Just add to change to clicking the containing Two Column component.

      Someone should probably check the functionality manually to see if still works as intended.

    2. 1 more Cypress test failing, xb-general.cy.js. It is currently failing because it didn't expect the new "Two Column" component in the menu. I think I fixed that but there are a lot of other assertions below that. So it will probably fail somewhere else.
    3. Fixing the test would go much faster if I could run Cypress tests locally so I will look into that tomorrow.
    4. UPDATE: xb-general.cy.js just passed. Gitlab Pipeline is ✅. 🎉

    @Wim Leers I don't have time to self review of this now, so I am sure there is stuff I need to clean up/fix. Assigning to you if you want to take a look.
    but if you would rather look after I do a self review feel free to assign it back to me.

  • Assigned to tedbow
  • Status changed to Needs work 5 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Thanks SO MUCH for #48 + #49 + #50 + #51, that made this much easier to follow 😊🙏

    First of all: just testing this branch gets me this, which is AWESOME 🤩:

    Review

    1. 🤔 The changes made by you after @ctrlADel's last commit (de8cbb170e95ac6edf0c67dc1c92d008041bba21) should not remove anything from the original intent, and ideally resolve all @todos he added. Looking at git d de8cbb170e95ac6edf0c67dc1c92d008041bba21 -- modules/experience_builder_example_components. You did not deviate from the intent, but there's one thing left to be done, see next point.
    2. 🙏 👆 reveals $refs stuff is not yet updated. But we're lacking specifics for Kyle's intent there: he is using $refs but did not specify the corresponding schema 😅. He did not write the corresponding schema.json file, but he did write the corresponding Twig. Which means you can write the necessary schema, Ted. See modules/experience_builder_example_components/components/simple/heading/heading.twig for example: that reveals there's two inputs Kyle was imagining for heading: element and text. I assume text would be type: string and element would be type: string, enum: [h2, h3, h4, h5, h6]. You can derive his intent for the other $refs in a similar way.
    3. 🙏 Once that's done, I think it's time to remove the blocks he added — which I said in my previous review too: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...
    4. ✅ The changes to the Cypress E2E tests look 👍
    5. ✅ The changes to EndToEndDemoIntegrationTest look 👍

    That second bullet is a fair bit of work, but this is getting close! 😊🥳

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    The Cypress E2E test failures are legitimate — did you know you can see the screenshots it made of failures on GitLab CI? 😊

  • 🇺🇸United States tedbow Ithaca, NY, USA

    Not sure why EndToEndDemoIntegrationTest is failing. Seems that can't a widget "width" property of the two_column component

    Will debug more tomorrow

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Not sure why EndToEndDemoIntegrationTest is failing. Seems that can't a widget "width" property of the two_column component

    The reason for that: the logic that 📌 Support complex SDC prop shapes: introduce (Storable)PropShape to compute field type storage settings Fixed introduced does not resolve $ref.

    Fixed that for you: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    In https://git.drupalcode.org/project/experience_builder/-/merge_requests/2..., you deleted config/optional/field.field.node.article.field_xb_demo.yml, which also results in all Cypress E2E tests failing … because the necessary defaults are no longer set up, at least not without also installing the new "example components" module.

    Why not go with what I suggested yesterday, which would fix that entire problem? I wrote:

    Also, if it makes things simpler, then I say: move all these schema additions to experience_builder's own schema.json for now. I know that @lauriii has talked previously about having a standard set of object shapes/schema definitions that contrib SDCs should be able to adopt, because that'll allow more cooperation/mixing-and-matching.

    I think that's the more pragmatic choice? 😅 And it will ensure that @lauriii and the rest of us must have that crucial conversation. 👍

    P.S.: also asked @Utkarsh_33 to help make this MR much smaller by landing 49 of the 132 file changes in 📌 Move /submodules to /modules Fixed 👍 You'll need to merge that in.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    @Wim Leers re

    Also, if it makes things simpler, then I say: move all these schema additions to experience_builder's own schema.json for now. I know that @lauriii has talked previously about having a standard set of object shapes/schema definitions that contrib SDCs should be able to adopt, because that'll allow more cooperation/mixing-and-matching.

    Now the Cypress tests are passing.

    standard set of object shapes/schema definitions

    I think it is probably a good idea. But now that we have schema.json working in the sub-module it is seems like a more realistic test case to prove that another module can provide its own schema.json and have a prop exposed in the form.

    It seems like even if we have a standard set of object shapes/schema definitions modules will still need to have their own for unique cases.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
    1. +1 to your last question: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...
    2. #58:

      It seems like even if we have a standard set of object shapes/schema definitions modules will still need to have their own for unique cases.

      Yes, but the difference will be that those modules' schema.json files are not being depended upon by the XB module itself. That's what makes this one special.

    3. https://git.drupalcode.org/project/experience_builder/-/merge_requests/2... is still not yet addressed.
    4. #57 is not addressed, and hence the steps to a working environment documented in CONTRIBUTING.md no longer work.
  • 🇺🇸United States tedbow Ithaca, NY, USA

    re #59

    Yes, but the difference will be that those modules' schema.json files are not being depended upon by the XB module itself. That's what makes this one special.

    This is not actually the case anymore. You can install experience_builder without experience_builder_example_components since the example field is now in experience_builder_example_components

  • 🇺🇸United States tedbow Ithaca, NY, USA

    re #57

    In https://git.drupalcode.org/project/experience_builder/-/merge_requests/2..., you deleted config/optional/field.field.node.article.field_xb_demo.yml, which also results in all Cypress E2E tests failing

    Actually that moved the file to modules/experience_builder_example_components/config/install/field.field.node.article.field_xb_demo.yml

    I think it makes more sense to the demo field there since it relies on the demo component. I just moved the field storage in another commit to make it cleaner.

    This also means that we can easily put all new props in modules/experience_builder_example_components/schema.json

    #57 is not addressed, and hence the steps to a working environment documented in CONTRIBUTING.md no longer work.

    So the update docs in CONTRIBUTING.md work now with 4. `drush pm:install experience_builder experience_builder_example_components`

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Discussed in detail with @tedbow — he'll extract the schema-related challenges in the side-by-side component out of this issue into a follow-up. Because that is not relevant for the part of the issue title — and it's important that that lands very soon. It unblocks a lot of work for 🌱 Milestone 0.1.0: Experience Builder Demo Active .

  • 🇺🇸United States tedbow Ithaca, NY, USA

    So per #62 another thing @Wim Leers I discussed was getting rid of the new sub-module and put all the components in the main module. Because we can't make experience_builder dependent on the submodule because the submodule can't be installed without experience_builder

    The following problems happened when I got rid of the sub-modulea

    1. This seemed like an easy task but for some reason EndToEndDemoIntegrationTest is failing when it tries to install the module with the error below.

      The problem seems to be because the module config is being validated on install and JsonSchemaDefinitionsStreamwrapper is not being used because the module is not fully installed yet. I have set debugged and confirm \JsonSchema\Uri\UriRetriever is being used instead of JsonSchemaDefinitionsStreamwrapper. field.field.node.article.field_xb_demo.yml is being saved which depends on experience_builder.component.experience_builder+two_column which uses components/containers/two_column/two_column.component.yml. I am not clear why components/image/image.component.yml does not have the same problem in 0.x. maybe because width is integer and image is object property?

    2. Some kernel tests are also failing because they are dependent on the specific components we currently have in the main module and there not being different ones

    Unless there is obvious reason for 1) I think we should be practical and commit this issue with the new sub-module and follow-up to move all the components into the main module. I know this is not ideal for people trying out the sub-module but we even do a messages on install that says "Please install the sub-module to see an example."

    I know this issue is blocking front-end work and I don't think it will matter on the front-end if 1 or 2 modules need to installed. It just seems like every change uncovers a new problem and meanwhile I think solving these problem is really helping the front-end work.

    JsonSchema\Exception\ResourceNotFoundException : file_get_contents(json-schema-definitions://experience_builder.module/column-width): Failed to open stream: No such file or directory /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/Retrievers/FileGetContents.php:38
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/UriRetriever.php:208
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/UriRetriever.php:181
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:52
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:115
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:138
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:162
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:123
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/ObjectConstraint.php:145
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/ObjectConstraint.php:47
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:90
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/UndefinedConstraint.php:74
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/UndefinedConstraint.php:52
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:123
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/SchemaConstraint.php:92
    /Users/ted.bowman/sites/exp-d-core/vendor/justinrainbow/json-schema/src/JsonSchema/Validator.php:61
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php:170
    /Users/ted.bowman/sites/exp-d-core/modules/contrib/experience_builder/src/Plugin/Validation/Constraint/ValidComponentTreeConstraintValidator.php:81
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:202
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:154
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:164
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:164
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:106
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php:93
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/TypedData/TypedData.php:132
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php:109
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:89
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Config.php:230
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:278
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Entity/EntityStorageBase.php:486
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:257
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Entity/EntityBase.php:354
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:614
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/ConfigInstaller.php:389
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/ConfigInstaller.php:260
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Config/ConfigInstaller.php:164
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:75
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/Extension/ModuleInstaller.php:326
    /Users/ted.bowman/sites/exp-d-core/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
    /Users/ted.bowman/sites/exp-d-core/modules/contrib/experience_builder/tests/src/Functional/EndToEndDemoIntegrationTest.php:51
    /Users/ted.bowman/sites/exp-d-core/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

  • 🇺🇸United States tedbow Ithaca, NY, USA

    ok I figured out why 0.x does not have the problem that this MR has with EndToEndDemoIntegrationTest that I describe in #63.2. This is because in 0.x

    1. field.field.node.article.field_xb_demo.yml depends on 2 components but only components/image/image.component.yml uses a $ref.
    2. The components/image/image.component.yml does not throw an exception because it is being validated on install before JsonSchemaDefinitionsStreamwrapper being used but because the it uses a dynamic source type it will not throw an exception that we ignore because of the next point
    3. The exception in this MR is trigger at

      ValidComponentTreeConstraintValidator.php:81

      $props_values = $value->resolveComponentProps($component_instance_uuid);
      $component = $this->componentPluginManager->find($component_id);
      $this->componentValidator->validateProps($props_values, $component);
      

      so the last line will lead to the validate trying to use the $ref

      But for image with since it is using a dynamic prop source we can't evaluate the entity because it doesn't exist until we have a real node. So this gets ignored for default field config

      catch (\OutOfRangeException $e) {
              // DynamicPropSources cannot be validated in isolation, only in the
              // context of a host content entity.
              // @todo Create specific exceptions for this in
              //   https://drupal.org/i/3462160.
              if ($component_tree_type === 'config') {
                // Silence this exception until this config is used in a content
                // entity.
              }
      

    So basically I think we found a new problem: Default field Config that uses a static prop source can't use a component that uses $ref: json-schema-definitions://experience_builder.module/.... unless the XB module is already enabled.

    So basically if you had module the depends on XB then this is not a problem. Just for the XB module itself

    So basically I think we should have to sub-module to sidestep this for now unless someone knows a quick fix.

  • 🇺🇸United States tedbow Ithaca, NY, USA

    #64

    The other option would be to set protected $strictConfigSchema = FALSE; for now but this might hide other problems

    I didn't know that \Drupal\Core\Config\Development\ConfigSchemaChecker took a list of config to ignore. So maybe ignoring 'field.field.node.article.field_xb_demo' until a follow-up is ok so that we don't have to have the sub-module?

  • Assigned to wim leers
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @tedbow walked me through #64 earlier today. That was really helpful!

    The root of the problem is that config validation runs before stream wrappers are registered, resulting in this mind-boggling stack trace that really is just triggered by installing default config 😅

    
    1) Drupal\Tests\experience_builder\Functional\EndToEndDemoIntegrationTest::test
    JsonSchema\Exception\ResourceNotFoundException: file_get_contents(json-schema-definitions://experience_builder.module/column-width): Failed to open stream: No such file or directory
    
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/Retrievers/FileGetContents.php:38
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/UriRetriever.php:208
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Uri/UriRetriever.php:181
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:52
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:115
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:138
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/SchemaStorage.php:162
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:123
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/ObjectConstraint.php:145
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/ObjectConstraint.php:47
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:90
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/UndefinedConstraint.php:74
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/UndefinedConstraint.php:52
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Constraint.php:123
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/SchemaConstraint.php:92
    /Users/wim.leers/core/vendor/justinrainbow/json-schema/src/JsonSchema/Validator.php:61
    /Users/wim.leers/core/core/lib/Drupal/Core/Theme/Component/ComponentValidator.php:170
    /Users/wim.leers/core/modules/contrib/experience_builder/src/Plugin/Validation/Constraint/ValidComponentTreeConstraintValidator.php:81
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:202
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:154
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:164
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:164
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveContextualValidator.php:106
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/Validation/RecursiveValidator.php:93
    /Users/wim.leers/core/core/lib/Drupal/Core/TypedData/TypedData.php:132
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php:109
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php:89
    /Users/wim.leers/core/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Config.php:230
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:278
    /Users/wim.leers/core/core/lib/Drupal/Core/Entity/EntityStorageBase.php:486
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:257
    /Users/wim.leers/core/core/lib/Drupal/Core/Entity/EntityBase.php:354
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:613
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/ConfigInstaller.php:389
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/ConfigInstaller.php:260
    /Users/wim.leers/core/core/lib/Drupal/Core/Config/ConfigInstaller.php:164
    /Users/wim.leers/core/core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php:75
    /Users/wim.leers/core/core/lib/Drupal/Core/Extension/ModuleInstaller.php:326
    /Users/wim.leers/core/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
    /Users/wim.leers/core/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:500
    /Users/wim.leers/core/core/tests/Drupal/Tests/BrowserTestBase.php:575
    /Users/wim.leers/core/core/tests/Drupal/Tests/BrowserTestBase.php:370
    /Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    

    Ironically, \Drupal\Core\Extension\ModuleInstaller::install() does register the stream wrappers immediately after default config is installed! 😬

    Fortunately, we can fix this by doing that manually in hook_module_preinstall()! 🥳 Plus, we'll be able to delete that hook implementation once [PP-1] Allow schema references in Single Directory Component prop schemas Postponed is in.

    So, a solution to all of #64 + #65 with a trivial hook implementation: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    First: sorry, @tedbow, I didn't quite realize just how many SDCs were in this MR. In my recollection, it was far less. Given that we have various tests that verify that XB can provide a meaningful experience, getting this across the finish line was more laborious and more broadly scoped than I anticipated. 🙈😬

    Which brings me to…

    @tedbow removed the side_by_side component in https://git.drupalcode.org/project/experience_builder/-/merge_requests/2..., to allow the vast majority of this MR to land without being blocked on additional details.

    I have to agree with @finnsky: the purpose of the text component is very unclear to me. It's just wrapping an arbitrary string in a <div>. Perhaps it was intended to contain rich text/markup, and use CKEditor 5 as the input UX? I searched, and could not find an answer to that in neither the commits, nor the comments on this issue. So rather than bringing in the Accordion component, I omitted it: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2..., as well as the Text component: https://git.drupalcode.org/project/experience_builder/-/merge_requests/2...

    Created the follow-up to ensure we do not forget about them: 📌 Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed .

  • Issue was unassigned.
  • Status changed to RTBC 4 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Issue summary updated to reflect the reality of what is in the MR that is about to be merged.

    @everyone, I trust that we all prefer to see progress instead of stagnation. The spirit of this issue is the one @ctrlADel outlined in #9.

    @pdureau in #11: — looking forward to that — that'll be critical for the SDC ecosystem! 👏

    Follow-up: components left out of this MR

    See #68, issue: 📌 Introduce an example set of representative SDC components; transition from "component list" to "component tree" Fixed .

    Follow-up: name as the name for an SDC prop

    This issue/MR includes only a Component config entity in its default config (i.e. to make an SDC available in XB) for the components/containers/two_column SDC. 📌 Auto-create/update Component config entities for all discovered SDCs that meet XB's minimum criteria Fixed will make it possible to place more of these components.

    I tried using /admin/structure/component/add (which will soon be obsolete, see #3463999), and had to harden its logic a bit because we're exercising many more SDC prop shapes with these components added — great! 👍

    In doing so, I re-discovered a bug that we only created @todos for, because it was a theoretical future, but now it is a reality: 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active .

    Follow-up: SDC props of type: object with no direct match

    Obvious and common use case, but we didn't have an SDC for this yet → 📌 [later phase] Support `{type: object, …}` prop shapes that require *multiple* field types — also: nested components/component reuse Postponed

    Follow-up: page hierarchy display

    This was built very early on, and now that this is in place, that important UI piece can move forward: 📌 Improve the page hierarchy display Active .

    Far future follow-up: default design system

    Eventually, all of these components will likely disappear, or perhaps evolve into the default design system. As Kyle described in #9, they fulfill a purpose of poking at the edges, and going beyond what many of the (rather simple) SDCs in Drupal core do. That eventual issue already exists: 🌱 [PP-1] Create components for a default design system Postponed .

    … and I'm sure there's many more that will eventually be spawned because of this. But the key results are:

  • Pipeline finished with Skipped
    4 months ago
    #253330
  • Status changed to Fixed 4 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024