🇫🇮Finland @lauriii

I don't think this is either a beta or a stable blocker. 👍 We could potentially remove later phase in case someone wants to contribute to this since it would be a nice UX/DX improvement.

wim leers → credited lauriii → .

This won't be needed anymore after: ✨ Move zoom controls, show only one viewport Active .

The fallback rendering actually was rendering the subtrees on both the live site and in the preview. That's what the Fallback source was literally doing.

Is this still the behavior? If it is, we need at least a follow-up to revisit that.

This is not a fair comparison IMHO. If an SDC/code component adds a new required prop, the component will refuse/fail to render if it is not populated. Content entities in Drupal that don't have a required field just won't render that field, and subsequent edits will require a value to be specified.

The point was that core is not protecting the data from becoming invalid; instead it deals with it on the runtime. This is probably something we will have to do as well when it comes to changing props.

FWIW, you will run into that exact same problem if you use SDCs for rendering the content. Because of this, using required isn't common for components. For example, UI Suite themes are not using required fields at all.

So, you're proposing a new issue for identifying and surfacing dangling/dead component subtrees?

Yep, +1 for a follow-up on that. And +1 for postponing that until we have made decisions around what the data model is going to be.

I think this looks great. Few comments that would be great to address to make sure that it's clear how this guidance should be applied:

1. Can we add a reference for geographical browser usage? If we don't have one, I don't think we should include it in the heuristics.
2. Can we define specific thresholds for when we would we consider a browser is disproportionally used by screenreaders per webaim screen reader survey?
3. Should we add guidance on how browser versions are handled? If they follow the existing policy, we could drop support for previous major of Safari for example.

#60 🐛 Cannot delete JS components due to component depending on them Active is a great workaround to this problem 👏 Definitely remove some urgency from 📌 [PP-1] Improve the UX of the fallback component input form Postponed .

Could we update the text to: "Component has been deleted. Copy values to new component." to be a bit more concise?

Just seeing this now. I believe this requires some consideration regarding how users would move exposed slots from one place to another within a template. The fact that slots were getting purged later enabled users to achieve this without a specific UI affordance.

In the meanwhile, we could think through how we'd implement this with publish workflow. If you delete a slot, that change should only take place during publish, not when the slot gets deleted from the template.

Without the Component Source Plugin, we lose the metadata about the props/block settings. We would need to rely on data stored in XB field on a node and do best our attempt go generate input fields matching value, expression and source type.
This is certainly possible, but it feels like somewhat a separate deep dive that warrants it's own issue.

I'm totally fine if we want to move this to a follow-up. Tagging just so that it doesn't get forgotten!

i.e. subtrees in slots will be visible, nothing else will be, nor could be

Per #36 🐛 Cannot delete JS components due to component depending on them Active , subtrees should not be visible in the preview or in the live site when the parent component is missing. Maybe you're describing that they are visible in the layers?

I don't think we can continue to show the same component inputs form (because how we handle SDC vs Blocks isn't compatible) - we could probably just store the values in hidden inputs so at least the values are retained.

Would it be possible to show the values in disabled fields? This way it would be possible to copy and paste the content elsewhere. I understand this may not be rendered exactly like the real component form would be but that seems fine since this is a fallback which is not really intended for authoring, but just to prevent you from losing data.

But, is it safe to keep the values around in case the scenario that caused the component to be remove is reversed? What happens if the deleted JS component is added back but with different props?

Wouldn't this fall under the scenario where props have been changed (i.e. 🌱 Plan to allow editing props and slots for exposed code components Active )? I think in the absence of that, it would be fine if we cannot render the component until the shape is in a valid state, i.e. no missing required props. When content with that component is being edited, it would have to be brought to validity.

This is not all that different from how things work today in Drupal. You can run into this without even doing anything too crazy because if you add a required field to an existing content type with content, it makes all of the content invalid. We have no safe guards against that today.

We may need a sibling issue to 📌 [SPIKE] Prove that it's possible to apply block settings update paths to stored XB component trees Active to document how one could delete all instances of a component when you really want to get rid of all of the content.

balintbrews → credited lauriii → .

I created another issue which is likely a duplicate of this.

Likely a duplicate of 🐛 Two Column component width dropdown gets blank after changing width Active .

lauriii → created an issue.

Test failure is caused by: 🐛 Cypress tests include invalid menu config on 11.x HEAD Active .

lauriii → created an issue.

lauriii → created an issue.

Adding the validation errors.

lauriii → created an issue. See original summary → .

lauriii → created an issue.

lauriii → created an issue.

This is not a beta or stable blocker but would be a nice improvement to build. Moving to active in case someone from the community wants to pick this up ✌️

This is a performance regression which belongs under bugs: https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... → . If it was clear that there was a performance regression and it was agreed upon as a trade-off, we would have opened a follow-up to fix this, which probably would have been a task. Usually this wouldn't be as critical but since this impacts one of the most commonly used tasks in the builder, it suddenly matters a lot.

Added ✨ [PP-1] Use CKEditor 5 for example value in code editor formatted text prop Postponed to the list.

✨ Use CKEditor 5 for text area prop's example form element in code editor, adjust prop shape sent to backend Active is done 🎉

FYI: this issue caused a (perceived?) performance regression: 🐛 Regression: loading state when navigating between components Active .

lauriii → created an issue.

+1 for the follow-ups mentioned in #12 📌 Leap ahead of #3493070 in core: SDC `enum` props should have human-readable labels: use `meta:enum` Active .

📌 Improve server-side error handling Active has landed

The blocking issues have landed now.

In other words, this is a problem that only occurs when we publish content that has an empty required field, which IMO shouldn't be possible in the first place. For simple-shaped fields, client side validation already prevents empty values from being written to autosave, so the publish process doesn't have to enforce it in those instances (but presumably can as it uses validation constraints). For fields such as image with more complex shapes, it looks like there might be more needed to enforce them being required.

It must be possible. The whole premise of XB is that all required fields must have an example value. What that example value enables us to do is to render previews but to guarantee that when you add a component to a page, it's in a state where it could be saved.

The difference between optional and required image fields in XB is that from an optional image field, it must be possible to remove that image and not render an image at all. For required image fields, it should never be possible to remove the image completely, but it should be possible to replace it with another image.

But: how did you make that component crash during rendering? It seems like you deleted an SDC, code component or a block plugin, instead of triggering one of the 6 failure modes described at the top of this issue?

I did not delete a component but I added invalid Twig syntax to a template. The response to /xb/api/v0/config/component is 500 with message: Unknown \u0022nonExistingFilter\u0022 filter.. I would not expect that having a component in an invalid state would lead into the XB UI being unusable which is why I'd like to prioritize it before the steps 3 and 4 here.

I already showed the current error message to Callum and he's going to do some designs around this that might help make the process more efficient. 😄

I tried to delete a SDC that was placed on a page (the experience_builder:two_column component). So I placed the component on a page, deleted the component from the code base, cleared caches, and reloaded XB.

I have no idea how to read this issue because #37 🐛 Cannot delete JS components due to component depending on them Active to me seems to be in a direct conflict with the issue title and #31 🐛 Cannot delete JS components due to component depending on them Active . I'm probably missing some nuance which is why I've completely misinterpreted this issue. 😅

It would be great if we could update the issue summary but besides that, I'm always open to jumping on a quick call to get us on the same page again.

How could this be a duplicate of 🐛 Changes to code components are not visible in preview-on-hover-component-list until published Active if it was discovered as part of QA'ing that issue? I tested it as well on latest HEAD and confirmed that what was loaded wasn't the autosaved version but what exists in the config entity.

1. Yes 👍
2. Yes, I'll work with Callum on a design and we can then define based on that what would be needed 👍
3. They should not be visible in the preview or in the live version of the page. However, they should be visible in the layers. It is okay to implement this in a follow-up issue.
4. Yes 👍
5. Yes, I think we would want to somehow indicate that those components are missing / broken. I'll ask Callum to design this.
6. The primary use case that we'd want to enable is being able to drag components from the slots outside of them. I'd imagine them working largely the same as other layers with the exception that they are not visible in the preview?
7. Callum will design these and provide for you

I wasn't able to test this manually though. I tested this by deleting the experience_builder:two_column when I had that component placed on a page. I got following error:
The \u0022experience_builder:two_column\u0022 plugin does not exist. Valid plugin IDs for Drupal\\experience_builder\\Plugin\\ComponentPluginManager are: olivero:teaser, experience_builder:my-section, experience_builder:image, experience_builder:shoe_button, experience_builder:shoe_badge, experience_builder:shoe_icon, experience_builder:shoe_details, experience_builder:video, experience_builder:heading, experience_builder:shoe_tab_panel, experience_builder:shoe_tab, experience_builder:shoe_tab_group, experience_builder:one_column, experience_builder:druplicon, experience_builder:my-hero, navigation:title, navigation:badge, navigation:toolbar-button

@mayur-sose I can reproduce both issues. Could you file new issues for these? 🙏

Yay 🎉

It looks like it's actually an assert that's triggering this. Changing to major because of that.

gábor hojtsy → credited lauriii → .

lauriii → created an issue.

Closing as a duplicate of 🐛 Hovered preview for JS components in library not working Active .

lauriii → created an issue.

I was able to reproduce this! This is critical because of data loss.

Ah, this is specific to required image fields! Updating the title + issue summary.

lauriii → created an issue.

This seems like a duplicate of 🐛 JS component slots don't appear in the preview canvas until published Active . I somehow thought that was only about slots.

Did a quick git bisect on this and this seems to have been caused by 📌 Add a route for PATCHing both a config entity and its auto-saved version together Active .

lauriii → created an issue.

Tested this with the built in components and couldn't reproduce #18 🐛 Default image is not loaded after removing media Active with them. I attached config for the component that allows you to reproduce this.

When remove is clicked on an optional image, it should not fallback to the example image.

Keeping this needs review in case that the approach needs review.

Tested this now that 📌 Improve server-side error handling Active landed. It seems that the preview rendering is somewhat working now. It would be great if the component was wrapped with the HTML comment so that it would be possible to see what is the name of the component in XB.

It looks like the library and the props forms fail to render → because of the component is erroring. That is at least equally important with not crashing the whole preview. Is there an issue for that already?

I think we could de-prioritize ✨ [PP-4] Present all component instances that fail to render using toasts Active and possibly even 📌 [PP-3] Server-rendered component instances failing to render should expose the meaningful error to the XB UI in a structured way Postponed until we have designs to not have to spend time on implement a temporary solution.

lauriii → created an issue.

Tried to test this and it doesn't seem to be doing what's expected. The image persists but I'm also unable to make changes to other props after removing the image 🤔

tedbow → credited lauriii → .

This definitely seems like an improvement over what was there before 👍

Would there be a way for us to only add the wrapper if a component doesn't render anything or renders a text node?

Linking designs from ✨ Create a navigation modal for changing pages in the editor Active .

#22: ✨ Add automated image optimization to image component Active picture is like another layer on top of the image component. See https://nextjs.org/docs/app/api-reference/components/image#getimageprops for how Next.js manages this and some other use cases with their API.

lauriii → created an issue.

This seems at least somewhat related to 🐛 Default image is not loaded after removing media Active .

Unfortunately, neither of the options considered in #17 🐛 SDCs with optional images without examples cannot be placed Active is the correct behavior. 😔 It must be possible to have optional images without a default value. That is a completely valid state and configuration to have since the image is optional. Based on these requirements, image-required-without-example was already added as a valid configuration (and has been documented as such in image-optional-without-example.component.yml). The src should be only required when an image is provided.

Added some more context to the issue summary.

It sounds like we could benefit from an ADR for this because we are trying to weigh tradeoffs between two options. Should we do that here or should we open a separate issue for that?

Here's the component I used for this:

uuid: 3dda3b0f-e2a1-4f44-999b-5f31c2b83c3a
langcode: en
status: true
dependencies:
  enforced: {  }
machineName: link
name: Link
block_override: null
required: {  }
props:
  link:
    title: Link
    type: string
    format: uri-reference
  title:
    title: Title
    type: string
    examples:
      - Link
slots: {  }
js:
  original: |-
    import { cn } from "@/lib/utils";

    const Link = ({
      title = "Link",
      link = '#',
    }) => {
      return (
        <a href={link}>{title}</a>
      );
    };

    export default Link;
  compiled: |
    import { jsx as _jsx } from "react/jsx-runtime";
    import { cn } from "@/lib/utils";
    const Link = ({ title = "Link", link = '#' })=>{
        return /*#__PURE__*/ _jsx("a", {
            href: link,
            children: title
        });
    };
    export default Link;
css:
  original: ''
  compiled: ''

Will do, that's easy for me to do! I just didn't realize it would be helpful 😊

lauriii → created an issue.

Tested the latest changes here. I'm still getting flickering. It does look like it is primarily the JavaScript components that are flickering. Wondering if we are waiting long enough before swapping the iframes to have the JavaScript components render?

lauriii → created an issue.

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

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

All of these questions were discussed as part of the design process but in fairness haven't been documented in the issue queue so I'm glad you're asking these questions. I don't know how close we're having all of these capabilities available so someone else will have to respond to this from that perspective.

1. How will this handle re-ordering of slots?

The slots should have an unique id. User should be able to change their placement in the template without it impacting the contents of that slot or other slots.

→

2. If a content template removes a slot, what should happen?

User should receive a warning that content inside the slot will be deleted. However, the content would not be deleted immediately; it would actually get deleted when the content entities with the slot contents get re-saved.

→

3. If I want to delete all field data associated with a slot programmatically is there a way to do this?

We discussed implementing garbage collection process running on cron. That would require having APIs for this.

4. If a content template re-adds a slot with the same name after previously deleting it, what should happen?

In this scenario, in case that the garbage collection had not happened yet, it would be fine to bring back the old slot content. But this would require the slot having the same unique id.

lauriii → created an issue.

This seems to improve the flickering situation but doesn't quite remove it, at least when using in-browser code components. Here's a short gif:

We cannot remove the allow list for now. We need to make sure that the absolute must have blocks continue working one way or another. That would be Menu, Breadcrumbs, Title, Branding, and Views. It doesn't seem that there's any option that would keep Views Blocks working in Experience Builder. Does that mean that regardless of what we do in XB, it will not be possible to place Views Blocks using it?

balintbrews → credited lauriii → .

lauriii → created an issue.

I do not have steps to reproduce for this but this is something I've been running into quite frequently today. It might warrant some investigation.

lauriii → created an issue.

lauriii → created an issue.

I ran into this by making a change and then quickly trying to publish it after making that change. I think the error I got was "This item is unexpected." and it was really confusing because I had no idea what is "this" in the context of the message and why it was unexpected.

I did. I think we should do the narrower scope from 📌 Handle components without HTML wrappers Active for stable and leave this as a separate issue to deal with later.

I think we need to revisit the UI a bit. I don't think "Component data" is the right place to display this in. Is the import components tab simply an interface to give the imports that can be copy pasted? Or does selecting a component in that interface have side effects?

I don't think the dependency between Settings Tray and Toolbar is fundamental so we could most likely get rid of that if we want to. At the same time, Settings Tray is at ~4-5% usage per 🌱 By default deprecate non-experimental modules that are used by less 5% of sites before the next major version Active . We should probably consider moving Settings Tray to contrib with Toolbar. People using it alongside Toolbar could continue using their current setup from contrib.