🇸🇪Sweden @johnwebdev

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

#15 hmm, I interpeted that as you can have an array of applicants => a field union field with muliple cardinality... but the applicant => the field union => cannot have any fields in it that are multi-valued.

But let larowlan confirm that!

Correct me if I'm wrong:

I think one problem with Field Union is that it doesn't support arrays in a union. If it does, please just ignore this comment.

Taking the applicant from the example of the module, https://www.drupal.org/project/field_union →

I'm unable to have a categories instead of category field on the union, e.g. an applicant having two categories. Two categories might not make sense, but perhaps I want multiple tags.

In an ideal world you would be able to mix object and array however you want. That would enable such a powerful modeling tool.

Adding a wrapper as in the MR, is a possible fix:

So the problem is "easy" to see when you remove the scaling:

Here is a component where the placement is right:

And here is a component where the placement is incorrect:

The problem is that Radix Tooltip doesn't take the scaling into consideration, so, since the component in incorrect example would collide with the viewport, radix adjust the position.

Setting the prop avoidCollisions={false} possibly fixes the problem, but might introduce another one, in case there is an actual preview that does collide (goes outside the viewport), it will not be adjusted. So I don't think that's the right solution here.

This MR just implements the most trivial fix, and that is changing the label in PageRegion. But, this also affects the label everywhere, and that might not be desirable? In that case, maybe we should add a dedicated getAutoSaveLabel() method on entities instead?

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

Keeping in mind that Site Studio stopped iframing due to issues that could not be fixed in a reasonable timeframe

And what are those issues? Are they present in the MR I wrote?

The PoC I wrote seemed to work fine. There are problems with the integration e.g removing media but that is not related to the iframe solution at all.

#26 Sorry, I'm not familiar with what Webforms does.

Generally when working with Drupal config you will do that locally as a developer and then export that configuration to file system and commit to Git.

How will this work work in XB? Should e.g. PageTemplate be ignored by the file system or how do we keep config in sync between Git and environments?

I would love to see an simple example EXTENSION_NAME.icons.yml in the change record

The extractor example file is quite overwhelming

#33 insertion already works with current code

#33

I'd argue it's convenient having them in the same list. Doesn't really matter it's unstructured or structured, it's still just Content. Just search or apply an filter if you need to be more granular in what you're looking for. This is great for small to medium sites IMO :)

I'd also argue being able to structure your content overview best fitting your IA is even better :) e.g. I can put Products or Events on a separate list because it might make sense for filtering or so.

Contributed modules integrate with nodes currently and could cause unexpected side effects with Experience Builder
Any business logic specified fields provided by Experience Builder may be reused on other content types or possibly modified, leading to unexpected side effects

I'd argue this is a great reason why to do it! It's early development and you get to find these issues early. Doesn't mean you need to adress them early.

While these are acceptable for a full release of Experience Builder, we are still in early development and need iterate rapidly. Experience Builder needs to make direct and opinionated architecture decisions early on. Integrating with an existing system could have unknown side effects. These concerns add additional variables that add complexity to the development of Experience Builder.

I don't really think this is a problem. You don't need to focus on solving those problems right away, but it's good to acknowledge them. If you expect people being able to go into production during early development it probably makes sense to have it separated early :)
And you don't need to act on all of these complexities right away either way.
Personally I think it's a great idea to try integrate it directly, because it would also allow time for existing integrations/systems to fix said issues from their end as well.

Side note: I'm not the one doing the full development on this, so I respect your decision either way, but I guess the beauty of open source is being able to express your opinions and thoughts and try help in the ways you can :)

This assumes incorrectly that the queue uses a database backend.

We shouldn’t assume that, and instead add a clear method to the backend interface and then call it.

I think this is a reasonable change.

Being able to create sorted lists of landing pages and structured pages

The /admin/content route would list structured content for all content types (nodes). There would be a new tab (like Media and Files) for "Landing pages" at /admin/content/landing-pages. These would list all unstructured content landing pages.

This is a stronger con from me when having a separate content entity. Having a separate list for landing pages seems hard to justifying? It’s only a technicality why we need a separate list in the admin UI.

I guess also integrations like:

Entities would need to use Dynamic Entity Reference to support referencing nodes and landing pages

is also a Stronger con imo

Have you considered vscode in web?

+1

I also think that accepting input should only be done when absolutely necessary otherwise go for good sensible defaults to not overwhelm the user 👍

Moved the check to isAutosaveEnabled as how it was solved in #4

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

Did you link to the wrong issue?

https://www.drupal.org/project/experience_builder/issues/3482393 ✨ Provide the frontend a way to distinguish module and theme components from one another Active

Is in XB, but your post suggests this is being solved upstream in core, but I couldn’t find any issue.

The only concern I have about Quick Node Clone is issues like: https://www.drupal.org/project/quick_node_clone/issues/3100117 🐛 Inline blocks from layout builder not cloned properly Needs review and https://www.drupal.org/project/quick_node_clone/issues/3405765 🐛 Non-reusable blocks must set an access dependency for access control Active

Nice! I think these benchmarks and exercises are really valuable!

What is the baseline? Are we setting the CMS up locally, or using some managed solution or trial experience? Or are we benchmarking all of these three as three separate benchmarks?

If setting up locally:

Will the exercise run on Windows, Mac - or both? Linux?
Will you start with having an existing development environment? Or is setting that up part of the benchmark?

#17

I tested changes with LB and they were also cloned, e.g. if you a block to the node via LB and clone the node, the block is there on the clone.

Awesome. But I am wondering if it pointed to the same block as the one cloned and not a duplicated one. Paragraphs and LB when overrides are composite so they should be duplicated too and not referenced as with other entity references.

A big + with using an existing contrib is that is hopefully already is battle tested in production too!

Does this work with paragraphs and Layout Builder?

Have this been documented?

Continuing on #22

1. Currently the dialog height is set to be 700px fixed. In Media Library UI its set to 75%. If the content of the dialog is less than configured height, it's changed to auto. I think that's a buggy behaviour since height is basically working as maxHeight, but makes it hard for use cases like this when we have iframes. I opened https://www.drupal.org/project/drupal/issues/3480282 🐛 The configured dialog height is overrided to be auto Active , but maybe someone have a better workaround in mind then just a fixed height.

2. Currently the modal is not closed when clicking Insert selected, we can solve that by just adding:

new CloseDialogCommand(),

to the $commands defined MediaLibraryXbPropOpener.php.

This is basically what MediaLibrarySelectForm and AddFormBase does:

    return $this->openerResolver->get($state)
      ->getSelectionResponse($state, $media_ids)
      ->addCommand(new CloseDialogCommand());

but we're only forwarding the commands from the opener and I think that's fine.

3. Instead of overriding media_library.ui, we can create a new "wrapper" route and do the overrides on them, this will allows us to only override in XB context.

So I basically suggest creating this route:

experience_builder.media_library.ui:
  path: '/xb/media-library'
  defaults:
    _controller: 'media_library.ui_builder:buildUi'
  requirements:
    _custom_access: 'media_library.ui_builder:checkAccess'
  options:
   _admin_route: 'TRUE'

... that simply uses the same controller. And then we just update experience_builder_preprocess_html as well to target that XB specific route.

4. experience_builder_preprocess_html()

Not sure yet, but either we remove all regions except 'content' and then remove any elements in said region except the Media library UI, or we create a new renderer that does above, which perhaps is a bit more efficient.

johnwebdev → created an issue.

#46 Maybe laurii can confirm? Personally I think starting in XB when creating a new page would be better UX than first having to create and save a node before getting started.

One question and possible problem with "Maybe we can just add the admin theme's CSS to the main doc?"

The question: Does that mean we add the "global-styling" or ALL libraries defined for the Admin theme?

Possible problem: I don't think just adding the CSS is enough, I can see for e.g. Claro there is a lot of preprocessing of e.g claro_preprocess_input, claro_form_media_library_add_form_alter, claro_preprocess_item_list__media_library_add_form_media_list that will affect things like Media library, e.g. if we didn't run claro_form_media_library_add_form_alter we would never even add the Media library CSS specifically for Claro theme! (Or add the classes it needs)
And if we're using a different Admin theme then Claro we have no idea what kind of preprocessing it does.

johnwebdev → created an issue.

I pushed some more changes and now it looks like this:

Spent some time looking into this tonight, and I think I have a solution on how this can be done. Here's how:

1. We need to form alter the MediaLibraryWidget, and override the ajax callback for the open_button

2. In said ajax callback, we need to build a new MediaLibraryState so that we can change the opener ID, which allows us to use our own Media opener. Here we will also construct the iframe URL, and then we'll return an AJAX command that needs to render said iframe.

3. We build a Media opener very similar to MediaLibraryFieldWidgetOpener, but we need to forward the commands from the iframe so that we can run them where the media field is actually rendered.

Leaving this unassigned, but I might pick it up again next weekend if someone else doesn't before me :)

I have a question that I’ve been thinking about around this, maybe a bit early to answer

Let’s say I have a header region, and I place a Hero component in that region. The Hero component an background image (Media entity)

How will that work? Let’s say I build the page template locally and then push the config entity to production, there is no guarantee that the media with that ID exists so the default will be broken image when you create a page.

Thinking loudly… Should page templates e.g be allowed to save “incomplete” components, or do we need some placeholder feature generic, or specifically for media?

What about Windows which is arguably the hardest to setup Docker in,

Content moderation has severe limitations - for example if media, taxonomy terms (via tags autocomplete) or other similar entities are created within experience builder while editing a node, it is not able to track their publishing state along with the node, so the media entity or taxonomy term will 'leak' into the live site.

I’ve rarely seen this as an actual problem on the websites I’ve worked on. Neither on different CMS that have same limitation, like Contentful.

jessebaker → credited johnwebdev → .

#16 UrlHelper::compressQueryParameter uses zlib (gzcompress) which is not supported in the browser, so the frontend would also need to install a dependency like https://github.com/imaya/zlib.js/

That approach should be able to solve 🐛 Make Media Library widget work when JS aggregation is enabled Active as well...

So I think I understand why this is happening, but I'm not yet sure how to solve it yet:

In experience_builder/ui/src/services/processResponseAssets.ts we have:

    try {
      await Drupal.AjaxCommands.prototype['add_js'](
        {
          instanceIndex: Drupal.ajax.instances.length + 1,
          selector: 'head',
        },
        {
          command: 'add_js',
          status: 'success',
          data: js.filter(
            (item: JsAttachItem) =>
              item.src && !document.querySelector(`script[src="${item.src}"]`),
          ),
        },
      );
    } catch (e) {
      console.error(e);
    }
  }

The following line:

item.src && !document.querySelector(`script[src="${item.src}"]`),

makes sure we don't add a JavaScript file that's already been added. The problem is that when we have aggregation enabled the file name is random and will contain multiple libraries/files.

So what's happening here is that multiple JS files already loaded will be loaded again, and e.g core/misc/ajax.js gets loaded twice, and I can see that it doesn't get the commands registered correctly (openDialog, closeDialog) from e.g dialog.ajax.

#4

We could then literally have a single DB table for all XB uses, by expanding the columns from

entity_type_id,entity_id,field_name,parent,slot,delta,uuid,component

I'm not sure that would with revisions and sync/async translations?

I think these kind of commands should have a confirm/are you sure step, running this by mistake and clearing the queue by mistake is rough. And those things happen.

I’m curious how Field union or similar would/could work when business wants a multi value for a field in the item. E.g add tags to the Picture image example

I suggest “advancedqueue:queue:clear [queue_name]”

This change record need an update to at least mention the name of the parameter

Fyi

We can only see following branches

8.x-1.x
3011563-computed
3011353

❤️

It is not clear to me if this module is being deprecated in 10 or 11. The change records suggests it will be removed in 11, but the merged code looks like it gets deprecated in 11. :)

++ amazing stuff

I think the implementation and UI looks great, and is exactly the use case I’ve had in all multilingual websites I’ve built so far.

++ to get this in

But I would be happy as well to just get the interface and manipulator in. In that case core implementation can be done in a follow up :)

#26 there are some differences

Komponent allows you have to multiple Layout Builder (similar to Paragraphs) since it provides you with a new field type that you add to your content type.

With Komponent you can enable regular Layout Builder and use it for layout, and then you can also have a Komponent field for the content creation.

Komponent have some nice widget configuration like instead of having a modal to select a block, it allows you to display the blocks as button directly, saving you a click.
It allows you to disable sections, for simple use cases where you only want a single section with blocks.

https://www.drupal.org/project/komponent →

This module allows you to add a Layout Builder field to your content types (or any other entity type) which allows you to build content directly on create and edit form. You can also use Layout Builder like normally on the content type. So you would use regular LB for actual page layout and Komponent for building content.

+1 on this aligns with the "ambitious site builder" experience.

quietone → credited johnwebdev → .

smustgrave → credited johnwebdev → .

Yes, it’s up to anon. Let me know if he doesn’t reply and I’ll ping him.

Sorry for not replying on email. Yes, I no longer work with Drupal actively, but we plan to make a new release for Linkit soon.

Adresses #106 and fixes cspell...

Note to self: Remember to Run code check :)

Adresses #101

4. That suggested change breaks the design. Leaving as is, given that we still need some design work to be done here.

8. Fixed.

10. Follow up?

11. I think follow up is fine.

12. Don't think so. Changed. Fixed double use of shouldBeCalled/willReturn.

13. Fixed.

Adresses #101 partially, hoping to get this going again!

1. That docblock is misleading (wrong), SectionComponentVisibility runs before BlockComponentRenderArray and that's why we stop propagation. I updated the comment.

2. Added true guard condition.

3. Not sure what we should do instead.

5. I changed 3 to 0. I added the weights in an attempt to see if it's more clear, not sure.

6. Removed.

7. Fixed.

9. Fixed.

Didn't have time to address everything now unfortunately...

Leaving at NW intentionally.

1. +++ b/core/modules/layout_builder/layout_builder.routing.yml
   @@ -145,3 +145,42 @@ layout_builder.move_block:
   +layout_builder.add_visibility:
   
   +++ b/core/modules/layout_builder/src/Form/BlockVisibilityForm.php
   @@ -0,0 +1,359 @@
   +            '#url' => Url::fromRoute('layout_builder.add_visibility', $this->getParameters($visibility_id), $options),
   

   Should this route be called "add_visibility" if it used for editing a existing one?

2. +++ b/core/modules/layout_builder/src/Form/BlockVisibilityForm.php
   @@ -0,0 +1,359 @@
   + * Provides a form for applying visibility conditions to a block.
   

   We're mixing block and section component. Should we be consistent and use one of them?

3. +++ b/core/modules/layout_builder/src/Form/BlockVisibilityForm.php
   @@ -0,0 +1,359 @@
   +              '#tag' => 'b',
   

   Should this be a heading element instead of bold?

4. +++ b/core/modules/layout_builder/src/Form/BlockVisibilityForm.php
   @@ -0,0 +1,359 @@
   +        '#empty' => $this->t('No required conditions have been configured.'),
   

   When is this ever displayed?

1. +++ b/core/modules/layout_builder/css/layout-builder.css
   @@ -220,3 +220,7 @@
   +#drupal-off-canvas #configured-conditions {
   

   No need to use a Id as selector. Use a class instead.

2. +++ b/core/modules/layout_builder/layout_builder.routing.yml
   @@ -145,3 +145,42 @@ layout_builder.move_block:
   +layout_builder.delete_visibility:
   

   For blocks we use "Remove block" - do we want to be consistent with the verb here?

3. +++ b/core/modules/layout_builder/layout_builder.services.yml
   @@ -44,6 +44,11 @@ services:
   +  component_visibility_subscriber:
   

   Should we prefix here with "layout_builder", similar to the other defined subscriber in this module?

4. +++ b/core/modules/layout_builder/src/EventSubscriber/SectionComponentVisibility.php
   @@ -0,0 +1,86 @@
   + * Determines component visibility.
   

   s/component/section component

Here is an example with pseudo code: https://gist.github.com/johndevman/6f095d41e6d49ed0306bd9683789c905

Alternatively, you could perhaps do something similar to cache (not entirely sure if the objects are shared or not here though)

  cache.entity:
    class: Drupal\Core\Cache\CacheBackendInterface
    tags:
      - { name: cache.bin }
    factory: cache_factory:get
    arguments: [entity]

  cache_factory:
    class: Drupal\Core\Cache\CacheFactory
    arguments: ['@settings', '%cache_default_bin_backends%']
    calls:
      - [setContainer, ['@service_container']]

Well, services are shared and ConfigImporter stores internal state (errors, validated status) etc which suggests to me that it shouldn't be a service.

What about a factory class?

#189

But aren't we already kinda doing that in the console script?

+++ b/core/lib/Drupal/Core/Console/Bootstrap.php
@@ -0,0 +1,70 @@
+      $kernel = DrupalKernel::createFromRequest($request, $this->classLoader, $env);
+      $kernel->boot();

+++ b/core/scripts/console
@@ -0,0 +1,26 @@
+$kernel = DrupalKernel::createFromRequest(Request::createFromGlobals(), $class_loader, 'console');
+$kernel->boot();

+++ b/core/lib/Drupal/Core/Console/Bootstrap.php
@@ -0,0 +1,70 @@
+      // Create a meaningful request object. We shouldn't really need this but
+      // Drupal complains if it's not present.
+      $request = Request::createFromGlobals();
...
+      $kernel = DrupalKernel::createFromRequest($request, $this->classLoader, $env);
+      $kernel->boot();

This part is confusing. It looks to me like the kernel is too tightly coupled to the Http layer. Perhaps we should try fix that, but at the very least we should elaborate on why we need to do this.

+++ b/core/lib/Drupal/Core/Console/Command/CommandBootstrapBase.php
@@ -0,0 +1,45 @@
+abstract class CommandBootstrapBase extends Command {

+++ b/core/lib/Drupal/Core/Console/Command/RunCron.php
@@ -0,0 +1,38 @@
+class RunCron extends CommandBootstrapBase {
...
+    $kernel = $this->bootstrap->bootstrap($this, $input, $output);

+++ b/core/modules/system/src/Command/ClearCache.php
@@ -0,0 +1,53 @@
+    $kernel = $this->bootstrap->bootstrap($this, $input, $output);

When do we need the concept of a bootstrapped Drupal command vs. not bootstrapped? Shouldn't it always just be bootstrapped?

I have not looked at the code yet; but doing manual test.

The base UI still need some work:

Here are some additional opinions on the UI and UX:

When I'm updating a condition, I would like to stay on the Configure visibility, if for instance I would like to do more changes. Same goes with adding a new condition and deleting condition.

The Add a new condition should be moved down to be aligned with the button. Currently added conditions are in the middle of these.

I really, really like this feature though. It can add some additional serious power for the site builders, for instance if Layout Builder could define conditions like "Hide this field block X if field block Y is not empty", etc.