🇺🇸United States @effulgentsia

Account created on 2 September 2006, over 18 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States effulgentsia

The issue summary now lists 80 of the 110 issues tagged "stable blocker". I still need to go through these 30 .

🇺🇸United States effulgentsia

Ok, so that makes sense that that's a core bug, but why does that matter to XB's shape matching? What are we doing that varies by what isComputed() returns?

🇺🇸United States effulgentsia

Yes, once we implement dynamic prop sources at all, it should be possible to "link" (the terminology that the UI will use for this) component props to computed fields as well as to computed properties of field items.

I think we need a core issue to flag those comment fields as computed

Oh, is the core bug that it's a computed field but not declared as such (isComputed() returns false)?

🇺🇸United States effulgentsia

The comment_last_name field is computed, so marked that as unsupported (core bug).

What's the core bug? Is "core" referring to Drupal core, or some other meaning of "core"?

🇺🇸United States effulgentsia

I started writing up 🌱 Milestone 1.0.0-beta1: Start creating non-throwaway sites Active . Note that that summary is very much a work in progress. Half the issues there probably don't really need to block a beta and possibly 80% of the stuff that we do want to get into the beta milestone isn't yet on that list. But hey, it's a start. Hopefully it will start resembling a proper roadmap within the next few weeks.

🇺🇸United States effulgentsia

#32.2 would leave room for also being able to identify the model of the slot distinctly from either the containing component or the slotted component. For example:

name: Table
model: [table]
slots:
  rows:
    model: [tbody]
    slotted:
      model: [tr]

Don't know if there's practical use cases for that though.

🇺🇸United States effulgentsia

Other terminology brainstorming...

We could shorten intendedFor to just for:

name: "(Table Row)"
is: [tr]
slots:
  cells:
    for: [td]

Or, if we like the idea of using model in both places, we could qualify the one for the slot with slotted:

name: "(Table Row)"
model: [tr]
slots:
  cells:
    slotted:
      model: [td]
🇺🇸United States effulgentsia

I like #29, but I think it's confusing for the same word, model, to mean something different for a component (what the component is) than for a slot (what the slot is intended to contain).

Personally, I'd suggest renaming it to something like is at the component level and intendedFor at the slot level.

I think the keywords within each can also include HTML tags where appropriate. For example:

name: "(Table Row)"
is: [table_row, tr]
slots:
  cells:
    title: "Row cells"
    description: "A sequence of cell components."
    intendedFor: [table_cell, td]

The above example might seem redundant, but something like [card, div] might not be.

🇺🇸United States effulgentsia

Responding to earlier comments, not #29...

It's totally possible for XB to add an extra yml file to SDCs

+1. I moved my proposal, with this suggestion incorporated, into #3513563-12: [later phase] Component slot restrictions ("which?") + limits ("how many?") .

intendedFor

Love it! I incorporated it into that issue comment.

Traits with conditions I'm not so sure about.

I think we need it to support use cases like #14, because whether a component is wide or narrow, light or dark, often depends on prop values. From an Experience Builder perspective, what this would mean is as long as the component has some prop values that are compatible with the policies, we'd let you put the component into the slot and then restrict the props form to only the policy-compatible values.

A simpler structure

I agree with simplifying how to express the conditions. I incorporated that into #3513563-12: [later phase] Component slot restrictions ("which?") + limits ("how many?") .

One downside to such a flexible system is we are bordering on violating the concept that everything you need to know about a component is in the directory

The key to #19 / #3513563-12: [later phase] Component slot restrictions ("which?") + limits ("how many?") is that everything that's about the component is still in the SDC's directory. Whereas the policies aren't about individual components, they're about component traits. For example, a policy about contrast ratios or width compatibilities is based purely on comparing the slot's computed traits (its own traits plus ones inherited by ancestors in the layout tree) with the component's computed traits. Even a policy that says "there must be an intersection between a component's is trait and a slot's intendedFor trait" isn't about a specific component, just about that general rule.

🇺🇸United States effulgentsia

Here's a proposal that continues the line of thinking in #3514072-19: Define how to handle SDC component filtering in the different display builder UIs (aka. What is possible to add in a SDC slot) (read that comment for details), but makes the following changes:

  • Moves all the new keys proposed in that comment into a new file COMPONENT.traits.yml in the same directory as the SDC.
  • Renames slotted to intendedFor.
  • Simplifies the condition syntax by starting with the prop and within that nesting the trait values for a given prop value.

accordion_group.traits.yml

slots:
  items:
    width: full
    luminance: 80
    intendedFor: [accordion_item]

accordion_item.traits.yml

is: [accordion_item]
width: full
luminance: 20

heading.traits.yml

is: [heading, text]

props:
  color:
    light:
      luminance: 80
    dark:
      luminance: 20
🇺🇸United States effulgentsia

I think I missed some IRL discussions somewhere.

Maybe, but not in relation to comment #19. My proposal in that comment came directly from reading this issue's comments.

The issue summary gives the example of an accordion_group component containing a slot into which only accordion_item components make sense to be slotted. The IS also points out that the concept of an accordion_item isn't unique to just one SDC: you can have multiple SDCs (e.g., one from a module, one from a theme) that are all accordion items. #18 provides a similar use case of 'cards', but is even more explicit about the concept that multiple SDCs can be "card-like". I think #19's suggestion of is and slotted addresses those use cases: a components can identify what it is, and a slot can identify what kind of slotted content it's designed for.

That addresses the direct slot/slotted relationship, but comment #14 mentions additional use cases of context that should affect more distant descendants. #19 proposes traits as the way to represent that.

#9 argues for the restriction rules to live in a different level than the SDCs, so #19 incorporates that via the separation between SDCs just declaring information about themselves, while the policies that act on that information being defined in Drupal config.

Additionally I'm thinking that adding all this at the slot level in SDC will make it very hard share SDCs across themes/modules.

The is, slotted, and traits keys I'm proposing in #19 would all be optional. SDCs that are designed to be super generic and flexible don't need to populate those keys. But what other than the SDC itself should be declaring that it's an accordion item, or card-like, or that it's full-width, or has a certain luminance, for the cases where the SDC author knows that information and wants to make that information available, so that policies can be created around that information?

doesn't belong on the SDC layer for me, especially now that we agree this is a UI concern, not a technical (as in render-time validation) concern

The SDC YAML already provides information about itself that isn't a render-time concern. For example, examples and description. A big selling point of SDCs is that everything about the SDC is colocated: the YAML, the Twig, the JS, the CSS. What's proposed in #19 is additional optional metadata about the SDC: why break the colocation advantage of SDCs by forcing that additional metadata to be somewhere else?

🇺🇸United States effulgentsia

Trying to synthesize all the comments here, here's a proposal. I hope the terminology is clear enough to convey the concepts, but we can refine the terminology as needed.

Within the SDC's YML, we introduce two new top-level keys (i.e., information about the component):

  • is
  • traits

And for each slot, we also introduce two keys:

  • slotted
  • traits

And we define a set of traits. This can grow over time, but to give some examples:

  • width
  • luminance

An example of what an accordion_group component might look like:

name: Accordion

props:
  ...

slots:
  items:
    title: ...
    description: ...
    slotted:
      - accordion_item
    traits:
      width: full
      luminance: 80

An example of what an accordion_item component might look like:

name: Accordion Item

is:
  - accordion_item

traits:
  width: full
  luminance: 20

props:
  ...

slots:
  ...

Traits, at the component level, or at the slot level, can also be conditional on the values of enum and boolean props. For example:

name: Heading

is:
  - text

traits:
  luminance:
    -
      condition:
        prop: color
        value: light
      value: 80
    -
      condition:
        prop: color
        value: dark
      value: 20

props:
  color:
    type: string
    enum:
      - light
      - dark

slots:
  ...

Trait values descend. Meaning, if a slot doesn't specify a trait, it inherits the value from its component. If a component doesn't specify a trait, it inherits the value from the slot that it's in.

All of the above is info that's in the SDC's YAML. In addition, we define a Drupal config entity type that can store policies. Themes (or modules, recipes, etc.) can include these policies within their config/install directory, just like any other default config. Exact syntax for these policies TBD, but for example, there could be:

  • A policy that says: a component can only be placed in a slot if there's an intersection between the component's is and the slot's slotted.
  • Another policy that says: a component can only be placed in a slot if its luminance has a contrast of at least 4.5 with the slot's luminance.

Policies can be enabled/disabled, and they're only applied by builder tools (e.g., Experience Builder), not when rendering.

What do you all think? Is this a reasonable partitioning of concerns?

🇺🇸United States effulgentsia

I don't see how #3514910: Real-time preview for props changes of JS components would A) help this issue...Yes, the resolved value is currently not yet actively used on the client-side, and we could in theory remove it.

My thinking was that the help it would offer this issue is by having an implemented use case (with tests) of the resolved values being used. So that, a) it prevents them from being removed, and b) it allows refactoring of where they're kept if such refactoring is desired per #20 while continuing to ensure that such refactoring is compatible with the use-case they need to serve.

But I'm totally okay with this issue being worked on first and not postponing it on Real-time preview for props changes of JS components Active .

🇺🇸United States effulgentsia

Is there a reason we don't include the preview HTML in the undo stack? Thereby allowing all undo operations (regardless of what kinds of components are on the page) to optimistically render before the server response?

🇺🇸United States effulgentsia

Even though my quote referenced in the issue summary was in the issue about the "page data" form, it was actually a statement about the component inputs form and just used as a point of comparison. So, re-titling this issue accordingly.

Currently, there is no client-side transformation that runs when widget inputs are updated on the page data form, so this issue isn't applicable there, though it's possible that at some point we'll want to open a separate issue to add client-side transformations on the page data form. I don't think there's currently a use-case for that though, since the fields on the page data form are intentionally the ones that (for the most part) don't have any representation in or impact on the preview (with the exception of potentially changing the content of Blocks, such as the page title block).

🇺🇸United States effulgentsia

If we're starting to Drupalify the term "slot" by adding some implicit features

I don't think that specifying rules for which components can go into which slots is about Drupalifying, I think it's about encoding the rules of a design system. If you're building a design system for broad consumption, like Bootstrap, DaisyUI, etc., then you probably don't want to be overly prescriptive, and instead leave as much creative freedom to put almost any component into almost any slot as possible to the users of the design system. However, what if you're building a design system for a single customer, such as a university (or any other type of organization with different units within it), and that university wants every department's website to have some creative freedom, but also stay on brand. So even though functionally you could put any component into any slot, because after all, it's all just HTML, the creator of this type of design system wants to impose some rules. Where should those rules be encoded? It could be in Drupal config entities, but I don't think these are Drupal-related rules, they are an aspect of the design system. Should they be in the SDC's YAML files? Or at some other level, but if some other level, what is that level?

🇺🇸United States effulgentsia

Do we only need resolved values for component preview and final render? I have concerns that if we are storing/sending both raw and resolved then there is a chance of them getting out of sync

I agree. Ideally only the raw values would need to be passed from the client to the server. As to where within the React code the resolved values are needed at all, as well as whether or not it's needed for resolved values (in addition to raw values) to be passed from server to client, I opened Real-time preview for props changes of JS components Active . That issue should be relatively straightforward to do, and I wonder if it would help with this issue's implementation to do that one first, to make sure resolved values are being used somewhere within the React code, even if not as part of the HTTP API.

🇺🇸United States effulgentsia

I don't think we have an issue for it yet, but one feature we still need to add is the ability for a JS component to import from another JS component. So, for example, if you have a Card component, and it wants to include a Button (as part of its design, not as something a content editor places into a slot), the code for the Card component would have:

import Button from '@/components/button'
...
// Now in my code I can do: <Button ...>

The string following the last '/' in the import statement is similar to a machine name. One possibility is we use the config entity's machine name instead of needing yet another identifier. However, if we do this, we'd want it to be mutable. Just like when you're editing code in an IDE it's nice to be able to refactor your file names / class names / function names, as you gain more clarity on what purpose that file/class/function is fulfilling.

I don't know if this is the only use case for wanting a mutable machine name, but it's potentially one of the use cases.

🇺🇸United States effulgentsia

The approach proposed in the issue summary looks great to me, except for 3 minor points:

Follow-up: later still: detailed customization

I agree with #39 that we shouldn't do this for the same reasons as explained in that comment. But since it's listed as a follow-up that's even later than all the other follow-ups, we don't need to come to a consensus on it here.

x-format

May I suggest changing it to x-formatting-context? Although that's more verbose, it grounds it in a specification with a precise meaning. And if it's not specified, we should default to block. @pdureau: Would that resolve your concern with it? If not, why not? Why do you see it as violating separation of concerns for the component author to specify that the component is designed for HTML prop foo to be contained in an inline formatting context?

xb_inline text format (restricted to <strong> <em> <u> <a href>)

I think we should allow <br> as well, since that's an inline-level element and example 1 in comment #10 seems to have one between "Developers" and "Download", unless those are intended to be two different props.

🇺🇸United States effulgentsia

MR looks great, but needs a test.

probably add a test calling \Drupal\Core\Validation\ConstraintManager::getDefinitionsByType to verify no crash?

Yep, that would be a good one.

🇺🇸United States effulgentsia

Is this a known bug that the header and primary menu don't come with the

wrapping element in Olivero? Or is it omitted on purpose for a specific reason?

It's due to some of Olivero's region--*.html.twig files adding a wrapping <div> and some not. I don't know if the ones without a wrapping div had a reason to be like that, but I think in general, whether to add wrapping elements or not is a per-theme decision so we can't rely on themes to follow a single root element for each region rule.

It makes it difficult do the "focus mode" on the header if the header has more than one component since there's no wrapping div I can target.

Is this a unique challenge for regions, or do we run into a similar issue (maybe not "focus mode" specifically but similar issues with selection) for components? We similarly can't assume that components follow a single root element for each component rule. How did we solve it there?

🇺🇸United States effulgentsia

This will improve the accuracy of the preview, because the markup structure would no longer be changed. The subtle difference in markup structure can currently cause CSS to not apply, resulting in inaccurate previews!

Yep. See #3507631-11: Page mangled and warning from block components when enabling for global template for an example.

🇺🇸United States effulgentsia

I'm not able to reproduce the block_theme_suggestions_block() warning.

I do see the page mangling when I enable "Use Experience Builder for page templates in this theme." in Olivero's settings, especially the page title block getting all skinny like in the issue summary's screenshot. I think that's due to the extra <div> that XB is adding around regions that 📌 Implement HTML comment annotations for Regions, replace HTML wrappers Active aims to remove, so I think this issue is a duplicate of that one.

When the checkbox above is not enabled, the markup around the <main> element is:

<div class="main-content__container container">
  <main role="main" class="site-main">

When the checkbox is enabled, that markup is:

<div class="main-content__container container">
  <div class="sidebar-grid grid-full">
    <main role="main" class="site-main">

In Chrome inspector, if I remove the classes from that middle div, it fixes the mangling of the page title.

🇺🇸United States effulgentsia

I want to correct what I wrote in #25:

Widget::massageFormValues() is a server-side widget-to-field (W->F) transform.

Reviewing this MR reminded me how wrong this is. If the above were true, then the denormalizers in this MR could just call the widget's massageFormValues() method. But the reason they can't is that W->F is actually client-side widget values -> server-side widget values -> field values, or WC->WS->F. WS->F is massageFormValues(), but WC->WS is spread throughout Form API callbacks (#process, #value_callback, #element_validate, and possibly others). That's why the denormalizers in this MR duplicate logic that's in the widget and the form elements created by the widget, but can't easily de-duplicate it. And that's why if we want the denormalizers to be optional, we need to figure out how to make the programmatic form submission approach work.

🇺🇸United States effulgentsia

I really wanted to like this MR, especially after #28 and the related conversation that @larowlan and I had in Slack about it. However:

The reliance on normalizers (and having to define custom ones for some widgets) is kinda breaking the promise that @lauriii and @effulgentsia wanted XB to uphold: that your existing widgets would continue to work.

Ouch. Yeah, that's a deal breaker for me, unless we've exhausted every other option.

Of course, the client-side transforms we introduced [in #3499554] already is sort of breaking that, but this feels … worse/a bigger burden.

I think what's worse about this MR is that at least in-theory the client-side transforms should be optional. If a contrib/custom widget doesn't include one (or opt into using the no-op one), the system should fall back to a server request that uses Form API + Widget API to go from widget input to field value and then the prop expression to go from field value to prop value. In this way, Drupal's existing ecosystem of widgets continue to work, and ones that include client-side transforms (or can use the no-op one) just work faster (without a network request once we implement 📌 Implement endpoint for realtime preview Active ). Note that I don't know if the current state of 0.x implements this automatic fallback to server, but even if it doesn't currently, in principle we should be able to do this.

Whereas this MR makes server-side (de)normalizers required for every widget that requires any transformation from widget input to field value, and it's not clear from this MR how we'd be able to add a fallback to Form API + Widget API for widgets for which we lack that (de)normalizer.

This sidesteps a number of issues we've experienced in previous attempts here AND a big one that we haven't even started on yet which is to do with multi-users publishing in bulk. The form system relies on tokens. Tokens are session specific. To get form API working, we were having to store the form build ID (And keep it up to date)

I want to look into this. I don't see what's so bad about storing the form build ID and keeping it up to date. And I don't see why CSRF tokens are required when doing a programmatic form submission. Access checks, however, might be an issue, if the person publishing has different permissions than the person who last triggered an autosave.

If we can make the Form API approach work, I'd prefer we start with that. But that wouldn't make the normalizer work in this MR wasted. I think we can still add those in later, as optional improvements for widgets for which we write them. For example, widgets that come with normalizers could perhaps support concurrent editing (or rather, the anti-concurrent lock could be just for the widget, not the entire entity form) whereas widgets without normalizers would have to anti-concurrent lock the entire form. That would be a nice way to gently incentivize widget maintainers to add normalizers rather than flat out making them not work in XB.

🇺🇸United States effulgentsia

In Slack, @larowlan confirmed that just like in #3509528-2: Adding to components doesn't appear to update component list - show loading indicator , this only happens when the Redux dev tools browser extension is in use (though for both Chrome and Firefox). Re-titling accordingly and downgrading to Normal. We could potentially decide to "won't fix" this, except not being able to use Redux dev tools would be an annoyance for any React developers wanting to contribute to XB.

🇺🇸United States effulgentsia

This would be helpful, for example, on Experience Builder, where we do 2-week sprints, and we start the sprint by tagging the issues we planned with the "sprint" tag, then at the end of the sprint it would be great to be able to untag all of them before tagging the next set. It's not so bad doing this manually when there's 10 issues to untag, but sometimes there can be 20 or more, so bulk untagging via VBO or similar would be really nice.

🇺🇸United States effulgentsia

Ah yes, #28 sounds right. However, does it let you side step the Form API problems entirely, or do you still have many of those same problems for AJAX interactions? For example, doesn't interacting with the Media Library still require the form build ID and all that? Or did you come up with a way to side step it there as well?

🇺🇸United States effulgentsia

Oh ok, yeah the scope of #26 makes sense, and given that scope, server-side makes sense too. But in that case, are you proposing:
- client has W
- client makes call to server to convert W to F
- client gets back F and makes another call to server to save F to entity

If so, what's the benefit of that vs.
- client has W
- client submits W to server
- server saves W to entity using Form/Widget API

I'm assuming the latter of the above is what was tried earlier in this issue and it ran into problems? If so, is there a way to quickly summarize what those problems were? I think the former is a fine way to go if there are compelling reasons why that's superior to the latter.

🇺🇸United States effulgentsia

I hope the following comment makes sense. I'm basing it on what I've been able to glean from issue comments, but I haven't yet inspected the code that's in 0.x or in this MR, so it's possible that some of what I write here is incorrect, in which case, apologies for that.

Backing up a bit, here's what we have in Drupal core, outside of XB:

  • Widget::formElement() is a server-side field-to-widget (F->W) transform.
  • Widget::massageFormValues() is a server-side widget-to-field (W->F) transform.
  • Formatter::viewElements() is a server-side field-to-html (F->H) transform.

XB aims to enable component-based design, and component-based design is about ditching the F->H concept (i.e., formatters), and replacing it with components, which are props-to-html (P->H) transforms, which then means we need field-to-props (F->P) transforms, which XB implements server-side as "prop expressions". So, with XB, the above becomes:

  • Widget::formElement() is a server-side field-to-widget (F->W) transform.
  • Widget::massageFormValues() is a server-side widget-to-field (W->F) transform.
  • Prop expressions are server-side field-to-prop (F->P) transforms.
  • Components are server-side (if SDCs) or client-side (if JS components) props-to-html (P->H) transforms.

What I think (please correct me if I'm wrong) 📌 Move clientside assumptions about prop form data shape into a series of prop specific transforms Active added is client-side widget-to-prop (W->P) transforms. Note that this doesn't have any server-side analog in the above lists.

What I think #23.2 is suggesting is to add client-side widget-to-field (W->F) transforms, which if feasible, would be nice in terms of parity with what happens server-side.

If we do #23.3, then do we still need the W->P transforms, or could we replace those with client-side prop expression evaluators, i.e., F->P transforms, so that W->P could be refactored as W->F, then F->P? Because if we can do that, then that will result in full parity between client-side and server-side for everything except F->W (Widget::formElement()), which we don't yet have a use case for needing a client-side version of (or do we?).

🇺🇸United States effulgentsia

I think the MR looks great. Should we upgrade to React 19 and take advantage of the nicer syntax for refs?

🇺🇸United States effulgentsia

Is the intent of this to make sure that the component you're currently editing is re-autosaved when global css is changed? But other components don't automatically get recompiled with the new global css, right? I think that's fine if that's what we want the scope of this issue to be, but just want to make sure that's explicit if that's the case.

🇺🇸United States effulgentsia

I think the fix for this is to add a JS file to the astro.hydration library that adds an event handler for the astro:only event that Astro fires after hydration completes, and in this event handler, remove the <astro-slot> and <astro-island> wrapper elements (i.e., remove them as wrappers by moving their children up to their parent). This would then match what SSR will do once we add SSR support. This would only be for components that don't depend on client state. Reactive components would still need these wrappers. But so long as this works for non-reactive components, people can solve grid issues by, for example, nesting non-reactive grid container components inside a reactive component that needs a grid.

In the meantime, the specific component in the issue summary can be made to work with this trick:

const gridVariants = cva(
-  "mx-auto grid w-full max-w-[1360px] place-items-center gap-4",
+  "mx-auto grid w-full max-w-[1360px] place-items-center gap-4 [&>astro-slot]:grid [&>astro-slot]:grid-cols-subgrid",
  {
    variants: {
      cardLayout: {
-        "2 columns": "grid grid-cols-1 gap-4 sm:grid-cols-2 md:grid-cols-2",
+        "2 columns": "grid grid-cols-1 gap-4 sm:grid-cols-2 md:grid-cols-2 [&>astro-slot]:col-span-2",

Hat tip to @lauriii, @larowlan, and @balintbrews for figuring out the above subgrid approach.

🇺🇸United States effulgentsia

1 E2E failure that's much more likely to be random than related to this MR, so merged to 0.x.

🇺🇸United States effulgentsia

Sorry, I missed tagging this sooner. It's already been in our sprint in practice, just missing the tag.

🇺🇸United States effulgentsia

Merged to 0.x. I'll open a follow-up for the further optimizations suggested in the MR reviews.

🇺🇸United States effulgentsia

In addition to fixing the layout shift, this also fixes a bug with not being able to drag into the slots, so bringing it into the current sprint.

🇺🇸United States effulgentsia

This MR fixes the layout shift entirely within the XB canvas, because the canvas does the 2 iframe trick where the iframe isn't swapped until the page load event fires, and adding the script tags delays that page load (which script tags do even without blocking=render).

This MR doesn't quite fix the layout shift on the published site and documents why in the code comment. I think it's worth merging this first (since it improves the in-XB editing experience), and then investigating the @todo separately.

🇺🇸United States effulgentsia

effulgentsia made their first commit to this issue’s fork.

🇺🇸United States effulgentsia

Thanks for adding those tests, Wim. They look great. Back to needs work for the rest of #19 (docs + the error on 10.4.x).

🇺🇸United States effulgentsia

@balintbrews or @effulgentsia to confirm 135ac01 is correct

Yes, that change is correct. Though as a side note, I'm curious under what conditions it's reachable. Are default slots a thing in SDCs? I only see examples of named slots, or did I not search thoroughly enough?

🇺🇸United States effulgentsia

\Drupal\experience_builder\Config\XbConfigOverrides should not only only apply to that route, but also only to users who have sufficient permissions: if they lack the administer code components permission, they should not be able to see draft (auto-saved) states of edited code components.

I don't think that's correct. If they have permission to use the code component within XB's page builder (if we don't yet have granular permissions for this, this is currently the same as if they have permission to use XB's page builder at all), then they should see what their component instance looks like in their content, using the draft state of the JS and CSS.

🇺🇸United States effulgentsia

Per #3497543-40: Milestone 0.2.0: the one for Drupal CMS 1.1 , we ended up not tagging a 0.2.0 release for inclusion in Drupal CMS. Therefore, this issue can go back to using that number.

We expect to tag the 0.2.0 release by the end of the week (which is also the end of the month), assuming we can wrap up these issues by then.

🇺🇸United States effulgentsia

Per 📌 Remove the XB demo Active , XB won't be added to Drupal CMS until it's more stable. In the meantime, there's https://github.com/phenaproxima/xb-demo. Therefore, this issue is now outdated, but we'll be tagging a 0.2.0 soon in 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active (which I will rename back to 0.2.0 now that that number is available again).

🇺🇸United States effulgentsia

I don't think we want the autosave (aka preview aka draft) JS and CSS files written to disk. It would mean a bunch of short-lived files that we'd need to purge at some point. What if we define a route for those to serve them dynamically? We'd need the filename to contain the ID of the component in that case. For those, I'd think we wouldn't need the hash, since we'd want the preview to just show whatever is latest in autosave storage even if that got updated between the time that the preview HTML is sent and when the browser makes the request to the asset.

It would be great if the preview asset URL was some/path/JS_COMPONENT_MACHINE_NAME. That would make a future issue with import maps nicer. For example, we'd be able to map @/components/ to some/path/ and code in one component that does import Button from '@/components/button' would work without needing every component to be listed separately in the import map. We don't need to add anything import map related to the scope here, I'm just giving context for why some/path/JS_COMPONENT_MACHINE_NAME is preferable to some/path/HASH.js, at least for the preview/autosave case. The current behavior of HASH.js written to disk is still good for the live site (real config save) case.

🇺🇸United States effulgentsia

multiple importmap scripts is coming to browsers which would also help, but that's a way off yet

I don't think it's that far off. It's in Chrome 133 and Safari HEAD (Tech Preview). https://bugzilla.mozilla.org/show_bug.cgi?id=1916277 is the issue for Firefox, they're just choosing to block it on landing import map integrity verification first.

🇺🇸United States effulgentsia

props="{"name":"bobby"}"

Astro receives props as tuples, so this would need to be props="{&quot;name&quot;:[0,&quot;bobby&quot;]}". Code Components as Block Overrides, step 1 Active contains a more generic fix for non-scalar prop values by using 'raw' instead of 0. In any case, this is pre-existing in 0.x and not this MR's responsibility to fix.

🇺🇸United States effulgentsia

This MR looks great to me. I'm not a code owner for the React code, but because I feel confident about the code being changed here and that it's fairly self contained, and because there's other work in another issue that overlaps with this, I merged it bypassing code owner approval to unblock the other work. If I made a mistake by doing this, we can revert if needed.

🇺🇸United States effulgentsia

This adds the breadcrumb block, so now this has all 4 desired for the initial scope.

🇺🇸United States effulgentsia

Note, if you're testing the wip patch, make sure to:

  • Install the xb_dev_js_blocks module (included in the patch), since that's what provides the default JS component entities.
  • Within the theme settings, click "Use Experience Builder for page templates in this theme.", since the JS component entities only override rendering of blocks that are rendered via XB dynamic components, not blocks that are rendered via Block UI entities.
🇺🇸United States effulgentsia

This includes page_title and system_branding. Still need to do breadcrumb.

🇺🇸United States effulgentsia

Here's an initial patch (yeah, I know, MRs are better, but old habits die hard) with system_menu_block implemented. I'll work on breadcrumb, site branding, and page title next.

🇺🇸United States effulgentsia

For terminology, I propose library instead of list. In other words, per #19, we'll have 4 libraries:

  1. ID=elements, label=Elements
  2. ID=extension_components, label=Components
  3. ID=dynamic_components, label="Dynamic Components"
  4. ID=primary_components, label=Components

Note that per above two different libraries can have the same label. #1 in the above list won't be in XB 1.0: that will come later when a visual component builder gets worked on.

If we follow the metaphor of Library, as in buildings with books, then libraries don't have positions, they have locations, so I also propose renaming what this MR calls position to location. This would then allow weight to be renamed to position.

Now here's where things get tricky...

On a superficial level, there are two locations: the left and the top. Continuing the metaphor with brick-and-mortar libraries, we can think of this as the main library (on the left) and satellite libraries (on the top). The area in the top bar for these satellite libraries is called tools 📌 Define the 3 areas the Top Bar will provide Active . So the above could be conceptualized as:

  1. ID=elements, label=Elements, location=tools, position=0
  2. ID=extension_components, label=Components, location=tools, position=1
  3. ID=dynamic_components, label="Dynamic Components", location=tools, position=2
  4. ID=primary_components, label=Components, location=main

On the surface, the above seems really good in terms of meeting the goals of comment #4: it would allow PHP code to add more libraries to the Tools area and have them show up without needing any change to the React code.

However, the are two problem with this approach:

  • The UX will need to intersperse other things that aren't component libraries in the Tools area. For example, we'll want a menu for managing Text styles, and we'll want this menu to be to the right of the Elements library and to the left of the Extensions library. Perhaps we could solve this by assigning documented position values for these non-library tools? So, for example, if we assign Text Styles as position=100, then #2 and #3 in the above list could be changed to position=101 and position=102.
  • Within the Extensions menu, there are other things besides the extension_components library. For example, there's also plugins. This probably makes extensions an entirely separate location from tools, since we don't want to place the extension_components library directly in the tools area, but rather inside the Extensions menu that's within the Tools area.

So that then leaves us with:

  1. ID=elements, label=Elements, location=tools, position=0
  2. ID=extension_components, label=Components, location=extensions
  3. ID=dynamic_components, label="Dynamic Components", location=tools, position=201
  4. ID=primary_components, label=Components, location=main

With the above, how much value are we even gaining from #1 and #3 being in the same location, if we need out-of-band knowledge about the position values? Would it be more robust to do:

  1. ID=elements, label=Elements, location=elements
  2. ID=extension_components, label=Components, location=extensions
  3. ID=dynamic_components, label="Dynamic Components", location=dynamic_components
  4. ID=primary_components, label=Components, location=main

But if we do the above, then we're down to a 1:1 relationship between library and location, which means we don't need location at all. We could go back to:

  1. ID=elements, label=Elements
  2. ID=extension_components, label=Components
  3. ID=dynamic_components, label="Dynamic Components"
  4. ID=primary_components, label=Components

and have the React UI hard-code where each library ID goes. Which I think is what makes the most sense given the UX we want, but it would remove the flexibility desired in comment #4.

@larowlan, @longwave: Since you were the two most wanting greater flexibility, what are your thoughts on this?

Production build 0.71.5 2024