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?
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.
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 → .
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.
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.
Do we have a list somewhere on what browser would be supported under the proposed heuristics?
I'm +1 to this because this aligns with the strategy to not compete with contrib. Webform provides clearly a stronger platform to build on top of which most people are choosing as a result. Form building is an important feature, so we may want to consider in future if core should be providing this feature, but it's clear that Contact isn't the right approach to start implementing that.
I'm +1 for moving History to contrib. It is not a key platform feature core needs to be providing and it seems like it could have a better future in contrib.
Did the honors to merge this given that we had code owner approvals.
lauriii → created an issue.
lauriii → created an issue.
There's a regression with this change:
We certainly should not delete the whole content template as a result of deleting a field. 😅 I think we should just remove the use of the dynamic prop from the content template. In case the deleted field happens to be a required, we replace the dynamic prop expression with example value.
balintbrews → credited lauriii → .
This seems like something for a site builder to configure. Even if not using responsive image styles as such, there needs to be settings for things like whether to crop to specific aspect ratios etc. as discussed above, which should never be exposed to content creators on a per-image basis either. So there has to be some kind of configuration step for the component before it's available to site builders, not just within the component at the content creator level, and then it can happen at that step.
The proposal is for the "(responsive) image style" to be able to live inside component code.
For example, the card component may do the following:
{% include 'image' with image|merge({ width: 250 })}) %}
This would automatically generate the image with the width 250px.
The hero component might do the following:
{% include 'image' with image|merge({ width: 2500 })}) %}
This would automatically generate a responsive image style where the highest width would be 2500px.
In future, we could add support to configure these using a config entity so that they could be used in the UI.
Yes it has nothing to do with file entities, this is why it's good to verify requirements before thousands of lines of code are written, even if an LLM is writing the code.
Looking at this again, it does look like the garbage collection only works for file entities because the only place where \image_path_flush is called is \Drupal\image\Hook\ImageHooks::filePredelete and \Drupal\image\Hook\ImageHooks::fileMove. So if we use image styles for non-file entities, we'd have to add another way of handling garbage collection.
Also if for some reason the format (or anything else) is taken from global configuration, then translated to a string to pass to the component, how is this translation layer different to what would be necessary to use a responsive image style to generate the relevant attributes and URL from to pass to the component?
There's nothing wrong in having global settings in configuration so long as the set of configurations is predictable, so that we are not introducing a dependency from components to a specific config entity which may or may not exist on a site. We can also provide sensible defaults for the configured image types. The problem with image styles and responsive images is dependency on specific config entities that may or may not exist, and the fact that they are extremely difficult to configure. Even if they exist, they may or may not have the configuration expected by a specific component because how would a component know if e.g., "small" image style has the configuration that the component expects?
this is not very encouraging.
This is one of the problems solved in the current MR. I think it would be better to wait for an actual proposal until doing further evaluation on this level.
This is still relevant.
The SDC principle is that it should receive the props, but the responsive image style config itself doesn't need to be a prop - instead the SDC component could receive the image URLs + dimensions + alt text, which can be built in advance based on the data stored in Experience Builder itself and passed in - just enough information to generate the img + srcset (or picture element) from.
But where would you choose the responsive image style in this scenario? This should not be up to the content creator to decide. The component should always render with the same (responsive) image style.
Since the URL or URLs contain all the necessary information for the image style callback to render the image, the SDC doesn't need to know anything about anything else - it can render the markup/attributes based on what's passed in.
I looked at \Drupal\image\Controller\ImageStyleDownloadController again and it looks like we may be able to re-use the download parts of image styles. I think this is something that @justafish indicated she would be looking into. There's already a PoC in https://git.drupalcode.org/project/image_style_dynamic/-/tree/8.x-1.x?re... for this.
I somehow had the impression that the garbage collection of images was relying on file entities but after reviewing this again, it doesn't seem to be the case. Based on this, we should be able to use some of the lower level APIs from image styles. I removed this challenge from the issue summary.
At the moment, any component library would have to re-implement all the various configuration options in the SDC added here in order to support responsive images, whereas really they should just be able to take the minimum set of information to build the markup.
Based on above, we may be able to provide additional components for loading image styles and responsive images from components as a fallback.
The current implementation is shoe-horning half a dozen configuration options into the component, including whether AVIF or webp should be used. In general, avif or webp feels much more like a site-building task than a content editor one - why add the load of choosing this every time? There could eventually be even more formats available, as well as composite formats, like avif with webp fallback, potentially even with a file size check if we do #3516434: Consider falling back to webp from AVIF based on filesize. Is every combination of those going to have to be provided in the SDC as they're added? This really doesn't seem to be something to configure for every XB component.
The component is not necessarily designed to be exposed in XB – it's a developer tool for whoever is building the components. Also, there are settings that should not be in the component itself. For example, the image types that are generated should be a global configuration. See https://nextjs.org/docs/pages/api-reference/components/image#advanced for how Next.js does this.
On top of this, the existing image style system provides a lot more options than are mentioned here: cropping which is necessary when uploads can be different aspect ratios, focal point crop etc. I imagine Experience Builder would also want to support these - are they going to need to be baked into the SDC component too?
We should support modules like focal point with the component. I'm not sure if that's the case for the MR as it currently stands but it is mentioned in the issue summary.
This is a reason to improve the usability of responsive images in core, not to build an entirely parallel system that may or may not be able to do the same things and can only be used by experience builder.
It would be very hard to iteratively address this in the responsive image module. I think it would be better to build an alternative module in contrib, and battle test it before putting it to core.
We should merge this ASAP so I'd recommend we move adding tests to a follow-up unless something that can be done very easily. There are quite a few people building on top of XB even though it's alpha and this is a pretty disruptive regression so we should try to roll a new release with this as soon as we can.
Do we need a generic "Use Experience Builder" permission which we'd use for the XB route and would move the components behind it? Otherwise it is pretty complicated to figure out when users should have access to the components.
Thank you @isholgueras for pushing this over the finish line! 👏 🎉
This seems like a duplicate of 🐛 Define what should happen if you delete a page while you are editing it Active .
Layout Builder has to use \Drupal\Core\Render\PreviewFallbackInterface::getPreviewFallbackString because it doesn't have an alternative way to interact with the blocks that have been placed. In Experience Builder, we do have the layers as an escape hatch.
Relying on the layers isn't necessarily ideal so there's a trade-off between preview accuracy and discoverability of these components. I'll make sure to discuss this with @callumharrod and @reneelund in future but I don't think this is something we need to solve urgently.
I'm leaning towards removing this from stable blockers, and handling 📌 Handle components without HTML wrappers Active separately as a stable blocker.
@gabor is this true when using 'content type templates' or the equivalent of layout builder default layouts, as described in #3455629: [Needs design] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model, or only when using the XB field/landing page mode?
It has been designed to work with both. In fact, it looks very similar to Same Page Preview but in Experience Builder!
I'll let @pameeela weigh in on whether this should be in Drupal CMS or not, but it doesn't seem ideal to target this for core at least.
Glad to hear you're interested in testing Experience Builder! We will provide support materials including documentation closer to a stable release. The current releases are only intended for developer testing. If you are curious about the current state, you can watch my recent presentation from DrupalCon for an update: https://www.youtube.com/watch?v=9klcnRxoMho.
balintbrews → credited lauriii → .
Holy smoke, this is so cool! 🤩
I don't think that's a regression because we already have an issue for that: 🐛 Adding component to component library results in component code and configuration being lost Active .
+1 for this! These are common and important fields in the context of a page builder.
And, to pile on even further, when you opt a bundle (I'm just gonna say node type from here on out, for expediency) into XB, what happens to existing view displays? Does a template get created for the full view mode only? Or do templates get created for all view modes? If it's the former case, then what happens when you view an entity in teaser but no XB template exists for it? If it's the latter case, how do you set up the teaser template initially? We almost have to fall back to core rendering, so as not to break sites that enable XB with content and view displays already set up.
If you are enabling XB for a content type, the expectation is that you would rebuild those view modes in XB... Because why would you be enabling XB otherwise in the first place? Thinking about this more, whether XB should overtake all of the view modes or not is a fair question. I believe I have said that it should. Thinking about this more deeply now, I'm realizing that in many cases, you would get most of the benefits of XB from using XB for the full page view mode, even if you are not migrating the rest of the view modes.
The reason I was trying to avoid this earlier, is that if we do not move all of the view modes to XB, we will then have to allow people to also edit their old view modes which will complicate the user experience. Ideally there will be an easy way to get a view mode converted from Manage Display / Layout Builder to XB. However, even this could be tricky because there's myriad of modules extending manage display that people are using, so building a migration that works in 100% of the use cases isn't possible.
Based on all of this, it seems that there needs to be some way for these two to co-exist to make the experience of migrating existing content types to XB reasonable. I'd like to avoid complicating the user experience for content types that do not have any non-XB view modes.
Seems fine from product perspective. I've wondered several times myself why we don't have these out of the box 🥲
Marking as fixed since there's a commit 🎉 Great work @jessebaker and everyone!
That makes sense 👍
Storing the default image is consistent with other default values. We may need a separate issue to consider how to use image as a fallback value. It's already possible in an SDC but this gets trickier in a JavaScript component because they cannot include images inside the component; they would have to host the image somewhere. I guess as a workaround, they could upload it to media library and reference from there.
FYI, ✨ Non-empty global regions need to be highlighted when hovering with the green color Active is where we would allow users to double click a global region inside the preview to edit it.
balintbrews → credited lauriii → .
Committed, so I expect us to now use format instead of $ref 🤓
Committed 914ecc3 and pushed to 11.x. Cherry-picked all the way back to 10.4.x. Thanks!
We should be able to remove pattern from the list because patterns are not leaving any references to the config entity; they are always stored as a copy.
Same problem exists for SDCs; there's nothing preventing them from being deleted from the code base 📌 Gracefulness when developing SDCs: SDCs may appear/disappear from one request to the next Active .
Epic work here getting to a 13 hour turnaround time. 🔥
#14: That seems fine for now. At worst case, I could see us wanting to show the empty regions somehow in the layers panel. I don't think we have to think about this too much now but it doesn't seem to be fundamentally limited by the HTML comments, even if they may be connected today.
larowlan → credited lauriii → .
We're not providing upgrade paths for XB currently, so it'd be fine to change this without an upgrade path.
It is number 3, the XB canvas' preview. This is different from 🐛 Changes to code components in global regions is not loaded until published Active because with overrides, the block doesn't have to be placed in a page region for this bug to occur.
That's a trivial move: instead of being listed at /admin/structure, add 2 new tabs at /admin/appearance. It'd also allow us to clean up the long obsolete Page Builder Components UI label.
Could we add a new tab called "Components" and then add the two tabs under it?
There is no parent context when using the XB UI and fetching the list of Component and Pattern config entities. The XB UI just fetches them regardless of which thing you're editing, no matter if it's a Page or Node content entity, or a ContentTypeTemplate config entity (once #3455629: [Needs design] [META] 7. Content type templates — aka "default layouts" — affects the tree+props data model is implemented).
Let's implement this based on the context-free proposal for now. We can add more complex permissions to the components later.
The permissions listed in #3452581: [META] XB Permissions do not include Administer Components, so I added that in #3452581: [META] XB Permissions
This seems too granular for now. Could we use administer themes permission for this? It might make sense to even move this under "Appearance" but that's off-topic for this issue 🤔
Use a tiny subclass of default \Drupal\Core\Entity\EntityAccessControlHandler for the Component and Pattern config entity types, aka all XB config entity types that provide the basic building blocks for the XB UI. These must respect the admin_permission, except that viewing must always be allowed.
For now, simply require the user to be authenticated (i.e. vary by the user.roles:authenticated) cache context).
Seems fine to allow viewing these to be tied to whatever parent context (e.g., access content).
wim leers → credited lauriii → .
Am I extrapolating correctly that you think permissions should be as simple as possible, and hence we should refactor away the administer xb_page permission in this issue?
Yes, we should keep the permissions as simple as we can at this point. We can keep adding more permissions as we add more functionality. We should avoid adding permissions which we don't have a clear use case for since they are pretty hard to remove later on.
I noticed that the administer xb_page permission isn't used for anything at the moment, and I also couldn't think of anything that we should move behind that permission at the moment. We will have to introduce this permission later, once we have page entity specific configurations in the UI.
Do we indeed want to add View Pages (view xb_page) instead of relying on access content? That permission used to be Node-specific, but has since been kind of generalized:
I agree that we should rely on the generic access content permission. I simply did not know that
#2892821: Core modules check node module's "access content" permission for displaying things that have nothing to do with nodes →
had happened because the permission is still listed under Node. 🤯
Looks good! It would be nice to make this customizable in future but this is a great improvement so I don't think we need to hold this back on that.
balintbrews → credited lauriii → .
griffynh → credited lauriii → .
wim leers → credited lauriii → .
FWIW, this is called "Default value" in the Field UI module today and it behaves the same way as XB.
My experience is that most page builders treat default values exactly the way Experience Builder does now: if there’s text in a field, that’s what will be saved. This matches how forms typically behave: when you see prefilled text, you expect it to be submitted unless you overwrite it.
That said, we need to decide whether to rely on default or examples in JSON Schema for this. Personally, I don’t have a strong preference either way.
Additionally, we may want to consider supporting both a placeholder value which is purely a form display concern and a true fallback value which is a rendering concern (i.e. fallback value determined at render time). I don’t think these should block the stable release.
kristen pol → credited lauriii → .
Nice to see this land! I tested this and it looks like the widget isn't showing when there's an image in the auto-saved state:
Proposed solution makes sense 👍
lauriii → created an issue.
disallow changing the machine name once a corresponding Component exists (which requires modifying the new constraint @larowlan added), and continue to use the machine name to refer to it.
I don't like the impact this limitation introduces to the UX. Similar to this, I already raised concerns around not being able to edit props / slots for components ( 🌱 Plan to allow editing props and slots for exposed code components Active ). I think we should try to find a solution without this limitation.
I don't think we need to allow <br> for inline content to build #10. If the design is supposed to have text on two lines regardless of the width, it should not be managed with a RTE.
Usually the use of <br> results in more problems that it solves so I'd say we should probably avoid getting to those. If we don't allow <br>, explaining inline text and regular rich text remains simple since one allows multiple lines and one doesn't.
I can't reproduce this anymore. Not sure what was causing this 🤷♂️
kristen pol → credited lauriii → .
1-3: Yes, it should work in these contexts. We want the canvas to have a cohesive behavior so these features should be available across the board.
4. I'm not sure I understand the question. I can't think if a way in which the labels could be translated asymmetrically.
5. 👍