🇫🇮Finland @lauriii

For a brief moment, what's in #10 indeed was the plan. However, given that we are aiming to move towards Workspaces based approach, it seemed that it would be better to simplify the workflow and to remove the step where changes would be autosaved and not visible within the workspace. The concept of requiring a manual step for changes to take effect within XB doesn't exist within XB outside of the code editor. Ideally we'd keep the experience there consistent with everything else, and not have this there as well. Therefore the current designs are basically taking the same approach to saving components and don't require a manual step for changes to take effect.

Let's hold off from renaming the config entities. The foundations are designed to be used for code base stored JavaScript components too, which would make using the in-browser code component term somewhat inaccurate in these use cases. This would be implemented in ✨ CLI tool to manage code components Active .

I feel like we can go back and forth whether the global CSS is meant to be theme agnostic. If we want to architect the perfect system, the answer will be yes and no. 🙂 That's why I'm proposing to work out a more generic storage mechanism, then we can do a simple layer on top with a simple use case as a start, i.e. the 1:1 mapping that was already mentioned.

Had a brief conversation with @balintbrews to make sure we're aligned on the solution. We're aligned that having a single CSS file per theme for shared CSS within that theme is enough to capture most of the value and enable the Preact + Tailwind v4 solution that the team has been architecting. 👍

there's not a single word in the 64 XB product requirements about variants for page templates. Only for content type templates, the concept of "variants" is stated. Route-specific choices should be achieved using requirement 41. Conditional display of components per the requirements. If your thinking has evolved, can you please add a new requirement there?

Clarified this in the requirements 👍 I'm glad this doesn't something that would be particularly complicated for us to introduce later. I'm also glad that we discussed about the requirement now that we were about to introduce something new that potentially may have impacted implementing this in future. 😊

#16 And we start off with one Style Guide entity per theme, […] — but @lauriii and @longwave in #15 concluded the global CSS will be applied whether or not a page template is currently in use., which implies theme-agnosticness?

The CSS should be tied to a specific theme. This would be used essentially for the same purposes as what the libraries key in themes .*.info.yml is used (as mentioned by @balintbrews in #18). If someone wants to reuse the same CSS in another theme, depending on their requirements, they could either set up a base theme with the common CSS or they could just copy the CSS to the other theme.

How does a JavascriptComponent specify which SharedCss it relies upon, and how do we expose that in the UI?

There's no explicit dependency to the global CSS in Drupal today. This assumption introduces many problems but I think we should keep it that way going forward because otherwise we'd be essentially introducing a 1:1 coupling of the theme and its components.

How do we guarantee that it works correctly across all themes? (AFAICT we don't/we can't/it's up to the JS component author?)

This needs to be taken into account in the way that components are build. E.g. they need to avoid hard dependency on the global CSS (e.g. providing default values for CSS variables when they do not exist). This is a hard problem that for example Tailwind tries to solve.

Not everyone cares about this. Many frontend developers are fine with introducing explicit dependencies from their components to their global CSS via CSS variables and rules. We should make it clear in documentation that doing this will make it harder for them to adopt external components. Unless you aim to repurpose your components or import external components, there's nothing wrong in that.

I don't get why we are saying this here, but previously we stated that we would only ever have one page template per theme.

I don't think this assumption is true – at least it's not an assumption we should rely on. What might change this in future is having to implement support for variants. I don't know how we'd implement these technically, but one possible option may be to allow multiple page templates in a theme with some negotiation on which page template should be resolved to a given route. I don't know how fundamental this assumption is currently but if it is, maybe we need to try to design the system for the variants soon so that we can plan for what needs to change.

To me a global stylesheet ties in with a global page template. Back then I could see that I might want multiple page templates - some "special" pages might want to opt in to an entirely different template via some mechanism - but we ruled that out, so why aren't we doing the same for global CSS?

Global CSS in it's most basic form is different from this though. There needs to be some place where it is possible to store design tokens that are true across the whole system. On top of that, we may need a separate stylesheet in future that is tied into a page template as an additional form of customization but it doesn't seem like a mutually exclusive concept.

So, does that mean we are switching /ui/src/ to Astro (still using React component, but different framework) ?

We are not shifting XB UI itself to using Astro. Astro is used for loading JavaScript code components in the preview and in the frontend. If you are not using JavaScript code components, Astro is not being used. Astro allows JavaScript code components to built with React, Preact, Svelte, Vue, and SolidJS. Experience Builder will provide an in-browser experience for authoring these components using Preact, and CLI tooling that allows adding components using other supported frameworks.

What's in the MR isn't still fully working:

1. Font family is incorrect; for some reason Media Library is loaded with font-family: Arial, Helvetica, sans-serif; in XB whereas outside it is loaded with font-family: "system-ui", -apple-system, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;
2. The media library items are not rendered entirely correctly.

   In XB:
   

   Outside of XB:
   

3. Styles for focusing the select element is broken:
4. Styles for the insert for media library button are broken for the non-default state:
   
5. On the table mode, links are black instead of blue:
   

Removing the security tag because it makes this issue listed on the project page as as a publicly disclosed security issue which this isn't since the security implication has to do with what's being done in this issue.

Here's the script that was used for the research: https://docs.google.com/document/d/1Usb6nOkNC9JeLelGl1zis-P_IbEzKrzxx5-1.... Specifically the task related to this was attaching images to the content type. We did not specifically ask users to attach a non-reusable image field because that is a significantly less important use case. There are valid use cases for it though like uploading a profile picture. I did a quick pass and couldn't find the summary of the results.

I'm not saying that the proposal you've made isn't good. Media was hardest to come to something sensible so I'm not surprised it's coming up as something that we may want to change. It also has the added description text which I fully support getting rid of if we can. However, I would feel more confident with the direction if we were able to validate that our hypothesis on these improvements are true.

It would be great if we could build a prototype with the proposed changes and then evaluate the next steps based on that.

For some reason, I'm also unable to type to the alt field after uploading an image. The field shows up and I'm able to focus it but if I type, nothing shows up on the form.

For some reason Media Library looks like this with the latest changes in the MR:

I wonder if I'm doing something wrong? 🤔

FWIW, this was tested with users back in 2023 and it tested quite well. We specifically tested adding images to a content type and most users went ahead and used the media option for that without hesitation. I can see how the current categorization makes the image field harder to find but it's also used much less frequently. If you're using media, wouldn't you only use that to create an image media type which would be added by default to fit majority of the use cases.

In general I'm not sure we should be making individual changes like this without first validating that the plan works out holistically. It would be great if we could build a prototype in 📌 Refine field descriptions Active for the proposed new descriptions and categories. Then those could be validated to make sure that at least the key journeys remain easy.

Singleton would be fine for the 0.3.0 release but I agree that we may want to put some consideration to the long term need. We may want multiple global stylesheets in future but I'm not sure how much we need to account for that now. If we start with a singleton (i.e. one stylesheet per theme), wouldn't it be pretty straightforward to migrate to whatever format we decide in future?

I'm not sure that the page template is the right config to tie this with. In future, I'd imagine we would tie the global CSS to the style guide once that becomes available. Given that the style guide is not fully defined yet, it might be hard to take everything into account now.

lauriii → made their first commit to this issue’s fork.

balintbrews → credited lauriii → .

This is a duplicate of ✨ UI for moving components between global regions Active .

Could we write an automated test for this? The MR also needs a rebase.

Agreed #27. I somehow thought that the latest revision tab was default but that's most definitely not the case. Based on that, the two states are fine and we can build the content moderation integration to this in a follow-up.

Thank you @wim leers! 🙏

I agree that we need to figure out a way to integrate content moderation with this. However, it would be great to punt as much of that to future as we can so that we can continue to make rapid progress. The key challenge I'm seeing that's preventing us from committing to the UX as it exists today is that the information displayed in the top bar in the case of content moderation could be outright inaccurate. What I propose to address that is to provide three states instead of the current two states; Published, Unpublished, Changed. Changed is a state we would use when the page has been published but there's a forward revision for the entity. This way it becomes clear that what the user is seeing, is not actually what is visible in the published site.

Like I said, none of this means we shouldn't integrate content moderation with the top bar, it just means that we can ship something that works even for the content moderation use case, and allows us to properly design what that API for overriding this should look like.

Thank you @larowlan and @wim leers for fixing this so quickly 🙏

Discussed with @plopesc and @roshni upadhyay to align on the approach. We agreed that we will continue with the approach proposed in #11. What we will do is:

1. Create a new submodule specifically for Drupal CMS (and Experience Builder)
2. For now, the submodule will not be using the Device config entity but we will hard code those values in the Drupal\navigation\TopBarItemBase implementation which will allow us to provide a good experience consistently
3. We will avoid making changes to the main module for now so that we can ship the submodule in a separate project if necessary
4. In future, we might integrate the device settings with Experience Builder to automatically define them based on what has been configured there.

I'm getting following error after this issue:

AssertionError: assert(!array_key_exists('default_markup', $this->values)) in assert() (line 29 of /var/www/html/drupal/modules/custom/experience_builder/src/ClientSideRepresentation.php).

It doesn't seem that \Drupal\experience_builder\Entity\Component::normalizeForClientSide is providing the required values 🤔

For some reason, I'm getting occasional PHP warnings after this change:

Warning: include_once(/var/www/html/drupal/core/profiles/standard/standard.profile): Failed to open stream: No such file or directory in include_once() (line 160 of /var/www/html/drupal/core/lib/Drupal/Core/Extension/Extension.php)

ckrina → credited lauriii → .

ckrina → credited lauriii → .

griffynh → credited lauriii → .

We should aim to support all of the core built-in entity types at 1.0 so that it's possible to build a Drupal CMS site without having to learn the existing Field UI tooling for the built-in functionality.

The Figma isn't immediately clear. Is it correct that the categorization is as follows

Correct 👍

Should be a simple fix. I think the code currently falls back to the component icon for types it doesn't know and does not have a case for 'node'.

Ah you're right that this is actually a node and it should use the CMS icon. I think we should also come up with a better fallback icon.

I believe this will need a change to the data the endpoint returns - perhaps better suited to a follow up issue.

+1 for a follow-up.

This poses many questions. Where does the colour come from? Can users choose a colour? Is the colour stored somewhere or should it be randomly generated on page load? Should the colour come back as a property of the user and be determined on back end? What are the available colour options? Again, maybe a follow up for this.

Sounds like something we should probably figure out in a follow-up.

I don't think we want to support non-revisionable entities. They would be really confusing in the XB. We might want to consider adding support for templating non-revisionable entity types in future but not something we need to worry about now.

We might need some follow-ups for this if these don't already exist:

1. The change icons are not using the correct colors. Based on https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n..., components are supposed to be purple and global regions are supposed to be green.
2. For some reason, a page shows up categorized as a component. It should be categorized as a page.
3. Instead of showing "Olivero global template" has been changed, the review panel should show which regions have been changed: https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n...
4. The current color behavior doesn't seem to result in a consistent color for a user. A single user should have the same color displayed for them to make it easy to identify changes made by one user.

That looks really nice 🤩

Does that also work with Gin after 🐛 Dialog styles are not loading correctly in Experience Builder Active has been fixed? 😊

penyaskito → credited lauriii → .

I don't see much value from product perspective in splitting the node integration to a separate module. If that's desirable from technical perspective, we could consider it.

It looks like what's inside the media library starts to look pretty close but the modal dialog itself looks like the default styling from jQuery UI. Is #46 still missing the modal dialog styles from Claro?

Turning on this setting would actually make the end-user visible site use XB for placing blocks which is not what we're trying to do The goal of the demo is to show case XB in the context of page building since that can be isolated to a single page that doesn't affect the live site. I think the issue and proposed solution make sense to me from that perspective.

I spent few minutes looking at the iframe MR earlier today just to see why it wasn't working since it was working earlier. Seems like the problem was with the vite integration module which is essentially writing over the library that is adding the JavaScript to integrate with the modal. With that change, the submit button inside the iframe is triggering outside of it.

We'd still have to figure out how to solve the problem of the buttons not being loaded in the jQuery UI modals bottom panel. Wouldn't this require moving those buttons outside of the iframe which would then mean custom styling or loading admin theme styling again? We'd also have to change how the events work I guess because now the event would be triggered outside of the iframe.

Not saying we should proceed with this approach, but just wanted to see how the MR worked to better understand what is happening.

Change ComponentPluginManager or something it delegates to, to only create Component config entities for SDCs provided by the active theme (and not its subthemes or modules).

I'm wondering if we should do this within the discovery or if we should filter the list in run time. If we did this, we should also group the components in the UI by theme instead of a single list of all components.

Let Drupal CMS just mark all Component config entities as disabled (using the disable config action), except the ones the Drupal CMS demo wants to be enabled?

+1 for using that for now as a workaround.

balintbrews → credited lauriii → .

I've tested the MR before around when I commented #33 and uploading images and inserting images to the page was working back then. I can confirm that in the latest state it isn't working though.

I don't know if this is helpful but here's a mention of the Site Studio change in their release notes: https://github.com/acquia/cohesion/blob/8.0.x/RELEASE_NOTES.md#media-lib....

Here are the key files that are relevant from there before they reverted to a different workaround:

1. https://github.com/acquia/cohesion/blob/7.3.x/js/media-iframe.js
2. https://github.com/acquia/cohesion/blob/7.3.x/src/Render/MainContent/Ifr...
3. https://github.com/acquia/cohesion/blob/7.3.x/src/Ajax/OpenIframeCommand...

What we want the solution to provide at minimum:

1. Consistent experience for using Media Library between XB and the rest of the admin UI (at minimum with Claro and Gin)
2. No duplication of styles so that Claro and Gin can keep updating their designs without our involvement
3. No explicit styling for XB in Claro and Gin

I think we should choose the solution that provides us the easiest way to accomplish this. If we don't have a solution that allows us to achieve all of these, these are in prioritized order so we should sacrifice from the end of the list first.

#10 Sorry, I wasn't clear enough in #9 that this is definitely a positive change in the short term because it helps bridge the gap between what exists today and having an easy to use XB. I was primarily just concerned that this might be something that we would be stuck with even once we have transitioned into the XB way of building themes and sites in future. It sounds like that isn't a concern which is great. 😊👍

I don't think #38 looks quite like the Media Library is supposed to look like in Gin. The goal is to provide a consistent experience for the media library regardless of whether it's being used in XB or outside of XB. I'm not keen to a specific solution, but it's hard to see how we could load the base styles in XB without them leaking, at least without modifying the CSS either in JS or PHP. Do you have thoughts on how we can solve that without hard coding the styles in XB?

Do we think this is something we can remove easily later once we have cleaned things up? I'm not sure that this is the right solution for long term. If we can remove this easily later on, this seems ok as a stop gap but. In the long term, we need to make managing the whole site in XB not a confusing experience.

wim leers → credited lauriii → .

I agree with the premise set by @effulgentsia in #2. There was already some related discussion in 📌 Add support for global regions Active on this.

Content creators not seeing the site as the site visitor is a pain point that comes up consistently in interviews with users. There are several actions that are already on the way to make the rendered preview represent more closely what an actual visitor of the site would see.

We are already moving local tasks from the page to the navigation in Drupal Core in ✨ Create the Top Bar Needs review . The goal would be to do the same for local actions.

Messages may need a special case along with main content and help because they are usually not changed after the initial configuration. The initial configuration could be done by a developer.

I was told "demo mode does not imply upgrade path, people will need to reinstall". Is that not true? 🤔

It's fine to require a reinstall of the Experience Builder module, but it would not be a whole site reinstall. It would be fine to require a step that removes everything related to XB and starts fresh before it could be used outside the demo mode once we release 1.0. However, we should make this really simple for user, i.e. users should not have to run CLI commands or go modify their database manually. 😅

ckrina → credited lauriii → .

pameeela → credited lauriii → .

lostcarpark → credited lauriii → .

Merged 📌 Support additional top-level data in returns from ApiPreviewController Active .

@anjali rathod Could you reimplement the API to utilize the new API?

Merging to unblock ✨ Show page information in top bar Active .

lauriii → made their first commit to this issue’s fork.

Ensuring the patterns that PHP support are also supported on the client-side.

Would it be a bad idea to process the date formatting in PHP and send the client both the formatted date and timestamp?

What if the chosen DateFormat config entity is deleted? ← mitigated by using a locked DateFormat, which \Drupal\system\DateFormatAccessControlHandler disallows being deleted. Not a perfect guarantee, but probably good enough. ✅

I don't think locked date formats provide the right capabilities. Besides preventing deletion, locking prevents changing the date format and translating the date format. Especially latter is something that is pretty unfortunate, because there isn't a standard international time format. Because of this, maybe the config dependency approach would work better for us?

Bikeshedding which DateFormat config entity the XB UI would use 😜

The designs are currently using "M j" as the date format. We should at least start with that.

I like the proposal in #5 🤩 Only problem I see with that is that I'm sure there would be a lot of people who'd want this behavior on regular media reference fields using images 😅

@wim leers Is there a specific change that you think is needed to that ADR? I read it again and to me, it doesn't seem to be in conflict with what's being stated here. We want to keep the experience where SDCs compatible with core could be consumed by XB without requiring changes to the components. This doesn't mean that XB couldn't allow SDC authors to use XB specific settings to enhance how components are displayed in XB, so long as the way that XB provides these settings, allows the SDCs to be still consumed by Drupal Core.

We have the "Cancel own user account" and "Select method for cancelling account" permissions which make sense for the use case of cancelling your own user. As a requirement, that makes sense because on some sites, you may want to allow users to remove their own user.

If a site is legally mandated to keep the information about author even after canceling the user, the site administrator, would have to disable the "Select method for cancelling account" permission from users, and configure the user cancellation to "Disable the account and keep its content".

I'm not sure I see what's the use case where you'd explicitly have to be able to block your own user outside of the canceling scenario.

I clarified the original product requirements with two principles that explain the original motivation in ✨ Allow developers to customize how components behave in XB Active .

1. Components built for Experience Builder should work with Drupal Core without Experience Builder installed.
2. Components built for Drupal Core should work with Experience Builder without additional changes.

This doesn't mean that we couldn't add additional capabilities within XB, so long as it doesn't violate these principles.

Image is already a capability that's specific to Experience Builder. Are we really blocked on core to make this change?

even if a new field being added to a bundle is not exposed in the UI, it would still get exposed via validation errors (for invalid field value, or potentially even missing required value) and via APIs: REST, JSON:API, GraphQL, as well as the PHP Entity/Field API. And more complex still: what if the same field gets added twice?

We would have to bypass validation in this case because the field technically wouldn't exist in the live workspace, i.e. even if you're adding a required field, it should not trigger a validation error. The field should also not be exposed in REST, JSON:API, GraphQL until it has been deployed to the live site.

Adding the same field twice seems edge case enough that it could generate a validation error.

So, when trying to edit the live version of a content entity that already has a modified version in any workspace, you'd get a message informing you editing it is not possible at this time? 🤔

I think this would be a pretty harsh limitation. There are use cases where you may want to prepare multiple versions of a change. Also having a change to a page in one workspace even if it's stale, would mean no one else would be able to edit it.

I can reproduce #9 including on a completely fresh installation. Seems fundamental enough to require additional tests.

It seems that #18 is something we could consider if we run into challenges with the current comments approach. The data looks concerning because only one of the non-UI Suite themes has 100% coverage. It also seems that the concerns around using HTML comments are currently hypothetical.

It will show up outside the workspace for other admins and potentially other workspaces could also add config that depends on it - maybe this is a feature rather than a limitation?

Isn't this primarily an UI concern if we are able to identify that a field was added in a workspace? I would imagine we'd have to be able to identify fields that don't exist in the live workspace anyway to make sure that they don't get exported in a config export.

Discarding the entire workspace might be hard to do, but I guess it is not necessarily that much different to deleting a field for any other reason.

Given that we are isolating the use of the field to the workspace where it is being added, couldn't we delete the field safely when the workspace gets discarded?

To clarify my comment from #16, I'm +1 to adding the in_preview (or similar variable). At the same time, I think that we should continue improving our tools to better facilitate an seamless integration of components for common use cases, even if they come with some interaction.

I agree with the premise from #2 that the proposal to add in_preview variable is in conflict with the idea that the components shouldn't have to be aware that they are being rendered in XB. That said, think we will eventually have to add in_preview as a tool to help working around the limitations of the editor in the complex use cases.

There's an alternative approach to this, which is to add a new property that allows toggling the accordion. This would essentially allow setting the default value for the open/closed state, but would also allow opening the accordion item in XB.

pameeela → credited lauriii → .

If you change a config on live before it is staged it will become automatically enabled on stage. Once a config change is made in the workspace it will apply over the top of any config changes made on live when the workspace is published.

In the Experience Builder workflow, it should be never possible to make changes directly in the live workspace. All changes are made in non-live workspaces and then merged into staging which would be then published to live.

it works OK with config entities that have no effect on database schema, like views, and it doesn't work at all with e.g. adding or removing fields from entities for pretty obvious reasons.

Could we allow additions to database schema to take place immediately even if the change is done in a workspace? Theoretically this could work because even if we make the database changes, it shouldn't impact the live site unless they get exposed somewhere. In the case of adding a new field to a bundle, it would only get exposed if the form display and entity display uses that field.

Deletions are trickier and should probably be handled differently so that the deletion only happens on publish. In this case, we would hide the fields from the user just like it was already deleted, even though the actual deletion would only happen on publish.

I think not allowing concurrent editing of the same region might be acceptable, although I defer to @lauriii on that.

I think we can work with the assumption that we don't have to merge changes within a single region. There may be use cases that would require creating alternative versions of a region, e.g., multiple proposals or an event that requires preparing to multiple outcomes. Based on that, the behavior might be just that the region gets overridden with the new contents regardless if other changes took place in between.

Committed 23ad40d and pushed to 11.x. Thanks!

Leaving this open in case that we want to backport this to specific release branches.

pameeela → credited lauriii → .

tim.plunkett → credited lauriii → .

lauriii → created an issue.

lauriii → created an issue.

lauriii → created an issue.

xjm → credited lauriii → .

which are not supposed to have anything processable in them

Is there a concrete problem that you're anticipating?

We're using HTML comments because we don't want to rely on a specific attribute for this. The goal for Experience Builder is to provide an easy way for developers to expose their existing SDC. Requiring a specific attribute would make the integration harder because components would need to always have this attribute which isn't the case today based on what I've seen in design systems.

Attributes also have another challenge. We need to be able to identify all of the slots as well. This means that not only would we need to have the attributes in the wrapper, but also on wrappers for slots. This would add additional requirements for components (i.e. consistent display of wrappers + attributes for slots) which is not acceptable from DX perspective.

lauriii → created an issue.

This is definitely solving a pain point that I've heard mentioned in many conversations with Drupal users. However, many of these sites are re-using media and it would be great if we could solve this in a way that doesn't get rid of the re-use media part. I think there are ways in which this could be solve so that we would have both.

This is not the only pain point that I've heard mentioned related to inserting media. Maybe we should do a more holistic design effort around how media gets referenced from content types to try to design something that could solve more of these problems at once? We don't have to implement all of these at once but having a north star design would be more likely to get us an overall better experience. It looks like some of these considerations are already being documented in 🌱 [Meta] Consider adopting Media Widgets as an experimental feature Active .

Here's a patch that contains all of the changes I made to be able to do the demo. It includes:

1. Fix for 🐛 PHP warning on image props without examples Active
2. 📌 Create back link in XB Active
3. Disabling contextual links from the frontend
4. Adding "Edit with XB" local task

lauriii → created an issue. See original summary → .

lauriii → created an issue. See original summary → .

Was supposed to mark as needs work.

wim leers → credited lauriii → .

UX wise this seems like a nice improvement! I asked review from @jessebaker on the solution.

Got a +1 from @larowlan on Slack that this is ready to merge.