🇫🇮Finland @lauriii

+1 for removing these messages including the "check available translations for your languages".

xjm → credited lauriii → .

Any SDC that has enum without the necessary meta:enum will no longer be available to XB users, and will now appear in the list of disabled XB components at /admin/appearance/component/status, with an explanation of why.

I'm not entirely convinced that we should not allow SDCs without meta:enum to be used in XB because it is possible to achieve an acceptable UX without providing meta:enum, albeit not in all scenarios. For example, for margin-top, you could choose to provide options '4px', '6px', and '8px'. In this case the same value could apply both as the human readable and machine readable value.

We could still allow translating the enum options but keep the machine readable names consistent across translations or we could simply prevent translating these enums.

I don't think this needs to block this issue from being committed. It seems that we could loosen this requirement in future if this would require a non-trivial amount of work to change.

wim leers → credited lauriii → .

wim leers → credited lauriii → .

I don't think the current UX is acceptable; invalid form value should not result in the component rendering an error.

I'm wondering why are we throwing an error instead of rendering the component with the empty string? JSON Schema required allows empty string so the component itself should be able to render with that.

Even though JSON Schema allows that, we could enforce a stronger validation and show it as a validation error on the form / review changes panel, but that could be separate from the rendering part.

Merging this despite not having all the code owner approvals here. This has seen plenty of scrutiny for an initial commit. 👍 Thank you everyone for continuing to push on this! 🙏

+1 for stable blocker since this is related to accessibility.

I don't think this needs to block the beta. It would be very nice to have though for the beta since this would make testing XB outside of Drupal CMS / other pre-defined setup much easier.

I agree that #8 🐛 If user with less/different permissions edits a page after a user with more/different permissions data lost could occur Active would be an acceptable way to get us to stable. This could be then improved post-1.0.0.

hooroomoo → credited lauriii → .

griffynh → credited lauriii → .

hooroomoo → credited lauriii → .

Another regression from this issue is that the caret is spinning after making a selection 😅

This was a pretty major regression for the styling of the textfields:

Before:

After:

@Wim Leers Contributing in a single MR makes it difficult for multiple people to contribute. We want to get away from that as soon as possible because it's already hurting the velocity. The module should be hidden and no one except people developing it should be installing it and therefore the harm from those issues should be minimal.

On a second thought, I'd assume we'd have to implement something around the field storage for the design systems regardless because design tokens wouldn't be limited to colors. We would not want to implement this to each field type individually. But it might be that there's still enough unknowns to warrant not working on this. Providing a freeform color selection isn't that useful for many use cases; you'd want to limit the options. But you probably do not want to define those colors on component by component basis so we'd have to design how we'd want those color options to be defined.

Stepper we may want to actually design the widget more properly. It would be great to split that to its own issue so that we have dedicated issues for both.

This is something that would be utilized for overrides; not for full on styling the site. The one example where I could have used this was applying a colored filter to an image; it just isn't a great experience to manually enter a color. The lack of having this feature to provide these types of overrides has come up in several conversations with users. There's a lot of capabilities that will need to be built on top of this to fully leverage this but we'd always have to start somewhere. This would become a lot more useful once we also have 🐛 DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active .

That said, the lack of thought for design tokens is something that concerns me a little bit. I was thinking that we may want to implement this with https://www.drupal.org/project/color_field → but it might be that we need something more bespoke in order to be able to utilize design tokens in there.

At first I was thinking that there wouldn't be harm in providing this as a stepping stone but it seems that adding support for design tokens could become more painful if we do this first. Removing this as a beta blocker for now.

So while XB is trying to reach beta and stability, if a critical change in XB breaks tests in this module, is that going to then prevent the change being made in XB? Seems like it would be better for that overhead to be isolated in a separate module.

Yep that seems like an acceptable trade-off to me for now to be able to move much faster towards a good user experience. We might want to reconsider that decision in future, but since this is not being built as a proper extension, it would be really challenging to get to this experience if this was kept in a separate module.

It looks like the final step of the issue summary has been solved most likely by 📌 Publish checks validation constraints, but not form validation Active . 👏 However, this issue is actually about the fact that the message shouldn't be visible in the preview. The error should be visible in the review changes panel and there should be a highlight inline the field. I think this issue should focus on just making sure that the message doesn't render and we need a follow-up to make sure that there's some indication of the error on the form.

This will add a composer dependency to ai_agents/ai_agents to every site that uses experience builder, even if they never install this module. As a result will also mean that XB is unable to be fully compatible with major versions of Drupal core until https://www.drupal.org/project/ai_agents → is.

Making it a separate project that depends on both xb and ai_agents would avoid both of these issues.

I don't think we should move this to a separate project. There will be a deep integration to XB from this module so moving this to a separate project will cause overhead given that you'd often likely have to modify both XB and this module. It will make managing versions / dependencies between the modules complicated too.

Could we remove the module from the main composer.json file? We will have to install these in the CI but is that something we could do without introducing a hard dependency? This would solve the former problem, but not latter. I think the latter we could accept as a risk.

It seems like @mayur-sose raised similar concerns internally on this.

xjm → credited lauriii → .

Makes sense to remove the button while it's broken. 👍

Yep, I think this would only happen if you had manually modified config files. It seems worth adding a warning because the experience is definitely less than ideal when this happens. Agree that this is minor 👍

Maybe there's something more specific that's wrong but this doesn't seem to be working for me? Attached video to show what I'm seeing.

griffynh → credited lauriii → .

This is likely needed by 🌱 [META] Experience Builder Personalization Active but otherwise I don't think there's a reason we'd block a release on this.

It looks like in the case of a required image for a SDC, after removing the image, the component fails to render due to an error. Is this expected?

I don't think this is a beta or stable blocker but would be a good to prioritize this with other non-blocking bugs.

restrict scope of this issue to only those prop shapes where #1 (a client-side transforms) exists, and which meets #2 (transforms to a valid resolved value per the JSON Schema for that prop)

Where can I find a list of prop types that meet this criteria?

I'm running this into a situation where I don't even have the override enabled. Debugged this for couple minutes and I can confirm this isn't caused by template_preprocess_breadcrumb__as_js_component(); the problem is lower level than that. It seems likely that this is caused by the fact that we are not using the regular routes for rendering the preview, but \Drupal\Core\Breadcrumb\BreadcrumbManager::build uses the route match as the basis of rendering the breadcrumb.

Realized this is a duplicate to 🐛 Breadcrumb is not rendering with the right context Active .

lauriii → created an issue.

I can't reproduce this anymore. Back in October we had a myriad of media related bugs so this could have been fixed by one of those.

Decreasing the priority now that 📌 Always create a new revision on publish Active has been committed.

This was fixed by ✨ Refactor Layers UI to use DnDKit Active .

lauriii → created an issue.

One example use case we should think about is lists. If you need a list component that can include links, text, and images, how would we build that?

I like the idea of dynamic slots but AFAIK neither Twig or React really supports this. #5 seems like it could potentially work for XB but wouldn't really work outside of XB. Or can we think of a syntax that would allow us for other Twig template to pass multiple slot values?

I don't think this issue at least justifies parsing Twig markup. It seems if anything, we should make the client a bit more resilient.

It would be nice to have client side validation for this still for UX purposes. Reducing priority.

Yep, it looks like this has changed shaped since this was first filed. This is not super critical though so moving to minor.

I think we should express the uploaded videos as follows in schema.json:

{
  "video": {
    "title": "video",
    "type": "object",
    "required": ["src"],
    "properties": {
      "src": {
        "title": "Video URL",
        "type": "string",
        "format": "uri"
      },
      "poster": {
        "title": "Poster image URL",
        "$ref": "json-schema-definitions://experience_builder.module/image-uri"
      },
      "width": {
        "title": "Video width",
        "type": "integer"
      },
      "height": {
        "title": "Video height",
        "type": "integer"
      },
      "autoplay": {
        "title": "Autoplay",
        "type": "boolean"
      },
      "loop": {
        "title": "Loop",
        "type": "boolean"
      },
      "muted": {
        "title": "Muted",
        "type": "boolean"
      },
      "controls": {
        "title": "Show controls",
        "type": "boolean"
      }
    }
  }
}

For oEmbed, I'm not sure we need the max width / max height here. It seems something that the component itself should handle? We may need to expose the height / width of the resource, as well as title which would make this shape as follows in schema.json:

{
  "oembed_video": {
    "title": "oembed_video",
    "type": "object",
    "required": ["url"],
    "properties": {
      "url": {
        "title": "oEmbed URL",
        "type": "string",
        "format": "uri",
        "contentMediaType": "application/oembed+json",
        "x-oembed-type": "video"
      },
      "width": {
        "title": "Video width",
        "type": "integer"
      },
      "height": {
        "title": "Video height",
        "type": "integer"
      },
      "title": {
        "title": "Video title",
        "type": "string"
      }
    }
  }
}

For documents, to display a link, we'd need a bit more than just the URI in the schema.json:

{
  "document": {
    "title": "document",
    "type": "object",
    "required": ["src"],
    "properties": {
      "src": {
        "title": "Document URL",
        "$ref": "json-schema-definitions://experience_builder.module/document-uri"
      },
      "title": {
        "title": "Document title",
        "type": "string"
      },
      "description": {
        "title": "Document description",
        "type": "string"
      },
      "filename": {
        "title": "Filename",
        "type": "string"
      },
      "filesize": {
        "title": "File size",
        "type": "integer",
        "description": "File size in bytes"
      },
      "mimetype": {
        "title": "MIME type",
        "type": "string"
      }
    },
    "document-uri": {
      "title": "Document URL",
      "type": "string",
      "format": "uri-reference",
      "pattern": "^(/|https?://)?.*\\.(txt|rtf|doc|docx|ppt|pptx|xls|xlsx|pdf|odf|odg|odp|ods|odt|fodt|fods|fodp|fodg|key|numbers|pages)(\\?.*)?(#.*)?$"
    }
  },
}

Should there be a reverse relationship too? An example of this would be to limit people to put accordion_items only into accordion_group components.

I was thinking about this but I'm not sure that we can do that because components can include multiple slots and we don't have the concept of a default slot. We don't know which slots these restrictions would apply to in the case that a component includes multiple slots. I would recommend opening a follow-up issue to consider this after we've added slot restrictions in this issue. This seems something that could be easily added as a new API there.

penyaskito → credited lauriii → .

I think the main drawback of one field per exposed slot is with respect to future Views integration. Suppose in the future we want to be able to support Views (or even just EntityQuer(ies)) for finding all instance of a Heading component whose element prop is equal to h2 (or likewise get lists of component instances matching whatever other condition). If we want such a View to be able to query across all exposed slots, I think that gets more complicated (maybe not impossible) when the data for each exposed slot is in a different field.

There might be other use cases that prevent us from going with the field per slot approach but this tradeoff seems acceptable to me. I can't think of use cases where we'd need this.

The slot itself is supposed to convey something and we shouldn't have to be able to take components outside of that context. This is only relevant in the case of a single exposed slot, e.g.,

content

where the slot is less relevant. But when you have two slots, there's already meaning attached to it, e.g., content_top and content_bottom.

To continue with the heading example, a module helping user to create a proper heading tree, may want to list all of the components inside all of the slots, and filter them based on whether that component is adding a heading. Even in this case, I'd imagine this would happen always in the context of the slot. It wouldn't be all that useful to show that there's a h2 somewhere, you'd want to show that there's a h2 in the content_top slot.

Also, wouldn't Paragraphs have a similar limitation to this, when you use multiple entity reference fields (i.e. multiple slots)?

Debugged this a little bit and this seems related to 🐛 Autosave publish process does not acknowledge pathauto deactivation Postponed .

Looks like this is potentially also impacting 🐛 Create New Revision checkbox is always unchecked and doesn't create revision Active .

There could be more than one problem here. I was able to get the boolean accounting to work but regardless of that this is not working. The value is missing when $items->getValue() gets called on that field because \Drupal\Core\TypedData\Plugin\DataType\Map::getValue ignores the pathauto property because per \Drupal\pathauto\PathautoItem it's a computed field.

✨ Refactor Layers UI to use DnDKit Active has landed.

This was solved by ✨ Refactor Layers UI to use DnDKit Active .

Indicating that this could be worked on by anyone.

I don't think we should block the release on this but we should try to get this fixed regardless.

I've been thinking how we should name this. I'm thinking supports and conflicts would be nice because it would make it explicit that we're documenting what components work nicely in the slot. This is relevant meta information even outside of page builders because it would allow developers to document what set of components should be tested with the component.

I think we need to support minimum and maximum children too. Often times there's a limit to how many components a slot is able to accommodate.

Allowing the use of categories and tags allows building design systems that are flexible without introducing a high number of direct dependencies between components. Adding tags besides categories allows managing situations where there's tension between categories which would be used for categorizing components in the UI and the restrictions.

Some example uses of tags could be:

1. full_width
2. editorial_content
3. dark and light

This way one could configure that XB root only supports full_width components. If you have components that specify a dark or light background, you could specify that the component only supports dark or light components.

I'm thinking that the following schema would be able to satisfy all of the use cases I have in mind:

slots:
  slot_name:
    title: string
    minimum: integer
    maximum: integer
    supports:
      components: array<string>
      categories: array<string>
      tags: array<string>
    conflicts:
      components: array<string>
      categories: array<string>
      tags: array<string>
    examples: array<object>

Here are some example configuration which would support components tagged as "feature" to be added. There could be anywhere between 1 and 6 items. Example values allow specifying content that a page builder could add to the slot by default:

name: Feature Section
slots:
  features:
    title: Feature Items
    minimum: 1
    maximum: 6
    supports:
      tags: [feature]
    examples:
      - component: "my_theme:feature_card"
        props:
          title: "Fast Performance"
          description: "Lightning fast load times"
          icon: "speed"
      - component: "my_theme:feature_card" 
        props:
          title: "Secure & Reliable"
          description: "Enterprise-grade security"
          icon: "shield"
      - component: "my_theme:feature_highlight"
        props:
          title: "24/7 Support"
          badge: "Premium"
          link: "/support"

Adding steps to reproduce

People administering XB would be the ones deciding what behavior they want for what gets done with the examples. For example, someone using XB to create a demo site for a DrupalCon presentation might want examples to be the initial value, and stored with the content, as per the current behavior. But someone making a corporate site for production might want SDCs' example values to help give some guidance to the content creators, but not get accidentally leaked to the published site.

It's unclear to me still how we'd implement this at this point. I think we have to design the no-code component editor before we make decisions around implementing this. There's tension between the single directory nature of SDCs and making decisions like this in XB UI. However, if we can allow users to achieve all of this by easily decorating a component via the UI, it wouldn't actually be conceptually modifying the component but creating effectively a new component.

I think we'll want to implement something more explicit like wrapping/extending the widget to give the content editor a choice to either fill in the decorated/base widget or choose a "Use the default" option, or something like that.

💯 👏

The components load really fast but I definitely still see the loading animation briefly even if I wait for bunch of time before:

The components load really fast but I definitely still see the loading animation briefly even if I wait for bunch of time before:

Yep, that's what I'm referring. Same pattern would apply in future for Sections and no-code components.

+1 that it's pretty annoying that clicking the component adds them to a page.

Could we use the clicking of a component to open the component in the code editor? It's pretty annoying to have to right click components to edit them. I realize the + button to indicate opening the library doesn't necessarily make sense with this so we may want to get some opinions from the design team.

I see you added loading states to the different component categories. I'm thinking it would probably make sense to load at least components on the initial load to avoid users seeing the loading animation.

We should definitely target this for 1.0 but we can reconsider in a later triage more closely.

I don't think this needs to be postponed. This should work regardless of the States API not working.

Spent few minutes debugging this to make sure this isn't a duplicate and I agree.

It looks like the problem here is that \Drupal\experience_builder\ClientDataToEntityConverter::setEntityFields has special handling for boolean fields. However, that handling does not account to the fact that fields can have multiple properties, and therefore other field types could include a boolean field. This is why the path field is not being detected for having a boolean, which is added by \Drupal\pathauto\PathautoWidget::formElement.

This problem seems pretty fundamental because you could run into this with other field types too. The simplest example would be Field Union fields. Updating issue summary based on this.

lauriii → created an issue.

In one of the early conversations with @lauriii he was very concerned that the data model should reflect how things appear on the page (e.g. ordering not getting out of sync). I get the principle that only ordering within a slot would actually affect the rendering though.

This is probably one of the conversations at dev days but I have no recollection whatsoever on what the specific concern was. What's important is that components within a slot are ordered correctly. I don't have strong opinions on whether that's done with field delta or not.

What's also important is that there's a simple way to construct the tree of components because we may have decoupled clients like Lupus building alternate rendering engines for the data model.

Amazing work on this @jessebaker 👏

Let's make sure to test that the UI handles this gracefully to not introduce more error handling debt.

+1 that we should not stick to the current page title block if it turns out to be more work than working around it would be.

it's restricted to the Page content entity type. But e.g. article nodes' path alias should have this same UX?

This is currently restricted to Pages (since that's all that XB supports). We could expand this to Nodes in future once we work on those.

If the answer to the above is that we shouldn't make it work for component instances, then … why don't we just create a new widget and at-runtime override the entity form display to not use \Drupal\path\Plugin\Field\FieldWidget\PathWidget?

I don't think we would make this work with component instances since this is specific to the path field type which is tightly coupled with aliases. Not creating a new widget makes adding Pathauto support simpler most likely because Pathauto is extending the default widget. If that's not the case, it seems this would be really easy to refactor to a widget if we want to do that as a follow-up to 🐛 XB pages doesn't respect Pathauto widget Active .

Addressed feedback on the MR

This seems potentially duplicate of 🐛 XB pages doesn't respect Pathauto widget Active or maybe there are two separate problems?

lauriii → created an issue.

@daffie Thank you for the detailed analysis! I totally agree that performance is critical for Experience Builder. Improving performance over the existing solutions was one of the goals since the beginning.

Are you aware of the discussion in 📌 [PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob Postponed ? The issue includes most of the intended changes. In #3468272-33: Store the ComponentTreeStructure field property one row per component instance → there was an assessment from @catch where he stated that the expected performance impact would be largely neutral. It would be great if you could look at that issue and provide any further insights there!

What comes to relying on PostgreSQL, the challenge is that if we introduce changes to the hosting requirements, it will slow down adoption. Because of that, it seems unlikely that Experience Builder would introduce a dependency on PostgreSQL.

Could we place these after code? Then these are in the order of how frequently they are going to be accessed. It's also more logical because Components + Code are related, and Dynamic Components + Overrides are related.

There's some more work needed to get pathauto to fully work with XB: 🐛 XB pages doesn't respect Pathauto widget Active . We need to do that issue regardless of what we do here.

I think we should do the following for the pathauto integration in the other issue:

1. Make pathauto not break the path field when there's no pattern configured for the entity type
2. Make sure that the solutions do not conflict, i.e. this should be only loaded when pathauto hasn't been enabled

I'd be more than happy to tackle 2. but I think we need to fix 1. more urgently first.

+1 for type: string, format: uri-reference not being relevant here.

penyaskito → credited lauriii → .

penyaskito → credited lauriii → .

penyaskito → credited lauriii → .

+1 for this being a beta blocker because of data loss.

Yep, that's correct! 😎

I don't think we would do 2. 1 seems something we might do but it also seems like a new API to specify attributes outside of the schema. Same as how https://github.com/rjsf-team/react-jsonschema-form separates UI Schema from the Schema.

balintbrews → credited lauriii → .

Unpostponing since in case that folks want to contribute to this.

Would you be able to workaround this by adding a prop that controls whether the details element is collapsed or expanded?

#19 🐛 DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active : Exactly. Providing an API for this would help improve the UX/DX since it would allow users to see the default value on the form – but I don't think this is critical. The reason this gets surfaced as a critical feature from Site Studio users is because there's literally no way to achieve this there.

I would imagine if we did this, we would still keep the examples[0] behavior just as it's today. Then we'd add support for default which would have a different behavior.

Sorry about that. I think I got confused because it looks like #12 🐛 Setting default values does work for boolean props Active was tested with code components but the original issue was for SDC 😅

So let's remove that restriction from both patterns and field component tree 👍

#17 🐛 DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active : I understand clearly that there's a limitation where the current example values are stored in the database. If you don't want that behavior, you can define fallback values in the SDC/React component. I understand we may want to provide an API for this in future for an improved DX+UX but I don't see how that should be critical and block a release.

Best experience would be to support adding the title block for pages too. How difficult would this be? If this is something that would be difficult to build and/or hard to maintain, we may want to reconsider that.