🇺🇸United States @effulgentsia

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

Merge Requests

Recent comments

🇺🇸United States effulgentsia

Ah, so what #13 means is that we can maybe not have a scratch vs draft distinction, and just do the same thing we do for nodes, pages, and page templates: only have two concepts:

  • autosave = not an entity save = draft = what people see within XB including XB preview
  • "Publish All" = entity save = published = what people see on the actual site (outside of XB)

What that means is that content editors who are editing within XB might see an interim state of a code component (e.g., not the old design and not the new design, but whatever the person editing the code component currently has in their code editor, see #10), but we're making the decision that that is okay at this point in XB's development, because once we have Workspaces we'll be able to solve it by having the code component editor and the content editor working in different workspaces, and until then we're not expecting early XB testers to be having a content editor and a code component editor working concurrently, and even if they are, then since they're just testing XB rather than using it in production, it's okay if the content editor is seeing interim states of a code component that's actively being worked on.

I'm okay with all that. However, there's still a wrinkle that at a given point in time there can be code in the code editor that is invalid in the sense of it doesn't compile to valid JS. For example, JSX tags might not be closed or other JS syntax errors. I think we'd still want to autosave this invalid JS code, so that if the component code editor's browser crashes and they restart it, they can still pick up where they left off. But then, what do we save into the compiled_js property as part of this autosave?

  • Option 1: we save NULL (or something approximating NULL like an empty component that renders NULL) into the compiled_js property. This would then mean that content editors either see nothing within XB for that component or some placeholder message that we can insert explaining that the component's code is invalid.
  • Option 2: we retain whatever the old value for compiled_js was. Meaning, the last valid compiled code. I think this would have the better UX of showing content editors something more useful, but maybe it's weird to save an autosave record where the compiled_js property isn't an accurate reflection of the source_code_js property?
🇺🇸United States effulgentsia

"global CSS" (which perhaps needs a different name)

Yep, naming is hard. "global" here refers to "not tied to a single component". I think "global CSS" is a common term in front-end development, but "global" here isn't meant to imply back-end globality (like there only being one for the entire site, regardless of theme, etc.).

One possibility is we name the new config entity type "style_guide"? Even though we don't know exactly what this concept will entail yet, but we can start it off with a css property and find out later what other properties to add to it? And we start off with one Style Guide entity per theme, and perhaps later there'll be a product requirement to allow >1 style guide per theme?

🇺🇸United States effulgentsia

https://github.com/mozilla/standards-positions/issues/1010 is the issue to follow for updates on Firefox's plans to support importmap integrity.

🇺🇸United States effulgentsia

Are in-browser code components supposed to be theme-agnostic or should they only be available within a particular theme?

I don't even think that that's relevant. CSS for a code component will live in the code component's config entity. So the question in this issue is just whether the global CSS should be per-theme or not, and I think that's independent of whether code components are per-theme or not. @lauriii in #7 already said that global CSS is per-theme, which makes sense to me.

One problem with tying the global CSS to the page template, is then does that mean that someone who doesn't check the Use-XB-for-regions checkbox also doesn't get global CSS? Or is that not an issue anymore with 📌 PageTemplate: allow configuring which regions are exposed Active , since now we can change our logic to always create the page template and just not opt it into any regions until the user opts those regions in?

Also, in the future we may well want to have >1 page template for the same theme sharing the same global CSS, but if we don't mind moving the global CSS to a different config entity type at that point in time, when hopefully we'll have more clarity about which config entity type to move it into, then I think it's fine to put it in the page template for now as long as the paragraph above has an answer.

🇺🇸United States effulgentsia

Does @jspm/generator not do that? The CLI's jspm link can be pointed to an HTML file, but not sure whether it's only the CLI that can parse or if the generator library can as well.

Babylon seems fine if we need a separate parser. In case it helps, we'll be using https://swc.rs/docs/usage/wasm to compile. I don't know if it has any API to access the imports, but perhaps it makes sense to parse the compiled JS to find the imports, in which case something lightweight like Acorn might suffice?

🇺🇸United States effulgentsia

If we don't like .draft and .usable as the names, another pair of names could be .scratch and .draft (where "draft" is what gets used for rendering content within the XB preview, and "scratch" is what gets autosaved as someone is typing within the code editor).

🇺🇸United States effulgentsia

Preview inside Experience Builder (returned by the /api/preview endpoint) is generated with the auto-saved changes taken into account.

Is this right? My understanding was that there were actually 3 states: not just "autosaved" and "published", but also one in between the two, let's call it "usable" for now (I'm sure we can come up with a better name later).

So let's say you already have a published component, and then you start making changes to the code, these changes would get "autosaved" for the purpose of the code component maintainer not losing their work, but would not actually be seen by content editors that are using the component, even within the XB preview. When the code component maintainer is satisfied with their changes, they would click a button to update the component that's in the component library. Now these changes are in the "usable" state, meaning this is now the code that's used for rendering within the XB preview, but it's not yet "published" to the live site. Publishing to the live site would happen as part of the "Publish All" process.

The above is what I understood the designs to be the last time I saw them, but it's possible my understanding is out of date. But I would find it odd as a component code maintainer to have all my stream-of-consciousness typing into the code editor getting automatically applied to what content editors are seeing. Seems like that would also be distracting to the content editors. If I'm removing 2 Tailwind classes and adding 3 others, seems like content editors should see the old design until the new design is ready, not all the interim steps in between (one TW class removed, then the other, then one of the 3 added, etc.).

If I'm understanding correctly, then I think it could be implemented as two separate autosave values. In other words, we could have one autosave key for KEY_FOR_COMPONENT.draft and another for KEY_FOR_COMPONENT.usable. The "Update Component" button would copy from .draft to .usable and the "Publish All" button would copy from .usable to the config entity. Yeah, maybe it's a bit of a misnomer for the .usable one to be in "autosave" storage, since it's not actually autosaved (it's only saved when a button is clicked), but it lets us reuse a lot of our existing autosave infrastructure and publishing code.

With this approach, everything in 📌 Code Components should render with their auto-saved state(if any) when rendered in the XB UI Active is still accurate, so long as it's based on the .usable key, not the .draft key.

🇺🇸United States effulgentsia

Having thought about the import map problem space a bit more, I don't think it makes sense as global or per-theme config, but rather as something that gets merged dynamically based on information that's in each code component. I opened Allow code components to import from npm packages Active with more detail about that, but the point is, it means there's no other configuration that we currently know of that would be "per style guide" (see #7) other than the global CSS.

#7 says we probably don't want to combine this with the page template entity. I'm not completely clear on why we would or wouldn't want to do that, so let's go along with @lauriii's intuition on this.

Which means, we're talking about a new config entity type whose only property is the global CSS. So for naming, I guess we should go with *.global_css.yml? Possibly in the future we might figure out the details of a *.style_guide.yml and would need to update from the former to the latter, but #8 indicates that that's okay?

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility. This one I'm tagging with "spike" rather than "sprint", because our goal isn't to complete the implementation of it this sprint, but to figure out if the proposed approach is viable or if we need to change it in some way.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility. This one I'm tagging with "spike" rather than "sprint", because our goal isn't to complete the implementation of it this sprint, but to refine its details.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility. This one I'm tagging with "spike" rather than "sprint", because our goal isn't to complete the implementation of it this sprint, but to refine its details.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

🇺🇸United States effulgentsia

Oh, right, I forgot that button was still there. So originally, that button implemented direct-to-entity-save (without going through autosave), so that we could work on issues having to do with saving entities in parallel with figuring out how we wanted to handle autosaving.

Once we got autosaving working, I think we opened an issue to change this Publish button to not send the entity data in the request body, but instead to just send the autosave key (or the info needed to figure out the autosave key), and the server-side controller to save from the corresponding autosaved data instead of from the request body. However, I forget if we completed that issue.

So, if that is in fact how the Publish button now works, then I do think it makes sense, as either part of this issue or as a followup that can proceed right after this issue is merged, to make sure that block settings get validated and transferred correctly from the autosave data to an entity revision. Because presumably the work to do that would be exactly the same as what will get triggered by the "Publish All" button once that's in.

However, if the Publish button is still working the old way where it's publishing from the request body rather than the autosave data, then I don't think it makes sense to make that old way work with block settings. Because once the "Publish All" button is in, we'll remove the "Publish" button.

🇺🇸United States effulgentsia

Where can I read the rationale behind that decision?

Right here is the first place :)

Back when we were doing the PoC referenced in #16, we hadn't yet come up with the concept of ComponentSource plugins. At that time, I was thinking that all components (e.g., Twig-based SDCs, Blocks, JS components, no-code components, etc.) could be exposed to the SDC plugin manager as SDCs. That would have required submitting an MR to change core's SDC plugin manager, or for XB to decorate core's SDC plugin manager. Which from an implementation perspective would have been fine if we had thought that was a good approach.

Part of why I was thinking along those lines is that core's SDC plugin manager is named ComponentPluginManager and the way you render SDCs is with '#type' => 'component', so I was envisioning this as being the contract for all components, not just "stateless UI objects implemented by a Twig file and a YAML file colocated in the same directory".

However, when it came time to implement Blocks as XB components, I think @pdureau said it best in #3462241-9: [PP-1] Decorate the SDC plugin manager and allow components defined in code :

Block plugins are stateful, context-aware, applicative objects.
SDC components are stateless, context agnostic, UI objects.
Different beasts.

Which led to us now having ComponentSource plugins.

Now that we have that concept, we can ask: what kinds of components should be SDCs, and what kinds of components should be some other kind of ComponentSource plugin. If we use @pdureau's definition above, then (in-browser-editable) (JS) Code Components could still be thought of SDCs as far as meeting that definition. However:

  • "SDC" stands for "single directory". There is no directory for the Code Components desired by this issue and its siblings: all the info is coming from the config entity described by this issue's summary.
  • Core's implementation of SDCs is coupled to there being a (non-config) YAML file and a Twig file; we'd have to change that implementation if we want the metadata coming from somewhere else (like config) and the code coming from somewhere else (like JS exclusively, no Twig).

So our choices are:

  • Change the name and implementation of SDC so that it's no longer restricted to the concept of "single directory".
  • Change the implementation of SDC so that it's no longer restricted to the concept of "single directory", but retain the name despite it no longer being accurate.
  • Keep SDC as it is, and implement a new ComponentSource plugin for config-backed JS components.

All of those options seem reasonable to me. #3 feels like a good pragmatic low-resistance choice.

Which aspects are expected to be identical to SDC, and which ones are not?

  • Different: The metadata for an SDC lives in a non-config YAML file. The metadata for Code Components lives in the config entity proposed by this issue. Identical: The syntax and semantics of props and slots is the same.
  • Different: SDCs are rendered via Twig. Code Components are rendered (without Twig) to an <astro-island> element that references JS code from the config entity (detail about that to be discussed in Create a ComponentSource plugin for JS components Active ). Identical: Both SDCs and Code Components meet @pdureau's definition of "stateless [with respect to server state], context agnostic, UI objects."

Of the aspects that are expected to be identical, do we expect to chase upstream SDC changes (i.e. commit to maintaining compatibility)?

Ideally, yes, and if we can keep that parity without code duplication (i.e., use the same validation logic for props and slots, and the same way of mapping prop schema to field widgets, and the same props edit form, etc.), that would be best.

In practice, some drift is acceptable. For example, when new JSON schema options for props get added for SDCs, it's okay if Code Components don't benefit from that until someone notices the discrepancy and submits an MR to bring them back to parity.

🇺🇸United States effulgentsia

It's only auto-saving (to ephemeral storage), not really saving (to an XB field or the PageTemplate). Which means it's not really stored, nor passing validation.

How are you determining that without [PP-1] Implement the "Publish All" button Postponed being done? Are you testing the publish step some other way?

🇺🇸United States effulgentsia

Besides changing this endpoint, is there any other complication we can foresee with breaking the 1:1 relationship between what the UI shows in the pending changes list and what corresponds to an entity? Hopefully, it's no big deal, but just wanting to ask before we go down that road.

🇺🇸United States effulgentsia

Also the global CSS is more of a singleton use case

Is it? Or perhaps someone might want multiple, each representing a different look? For example, perhaps a university site where each department is branded a bit differently?

I think our initial implementation can be based on having just one, but I think conceptually having a config entity type for this that could in principle allow multiple config entities of that type would be good.

We need to come up with a name for this config entity type.

Also, would it make sense for the same entity to have any other property besides the CSS? For example, another global or semi-global thing might be a javascript import map (mapping bare package names of dependencies that the JS components might want to use, like perhaps components from Radix UI, to CDN assets, for example perhaps generated with https://jspm.org/). But I wonder if that belongs in the same entity as the global CSS (are the two each aspects of the same higher level concept like a theme or design system) or if global CSS and import maps belong in different entities / entity types (are they aspects of different concepts such as global CSS being an aspect of a theme whereas import map is an aspect of a component library)?

🇺🇸United States effulgentsia

As far as know is 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active isn't required for 0.3.0

Yeah, that issue and its children should be moved in the IS from the "save content" requirement to the "blocks" requirement. It's possible that the entirety of that isn't required for 0.3.0, but the parts that are blocking 📌 Implement saving block settings forms Active are.

🇺🇸United States effulgentsia

In addition to 📌 [PP-1] Send page data to Drupal for storage in auto-save store Postponed: needs info being under the "Save (draft) content" requirement, it's also the last remaining item for "Content editing of meta fields", so landing it would give us another nice green checkmark in front of a top-level requirement.

🌱 [META] Pages Active is still listed in the IS under "Content editing of meta fields" but the remaining children of that are being tracked separately under that meta and don't need to block this issue.

🇺🇸United States effulgentsia

Changing to Active because it's not blocked on anything, but the XB team is not currently working on this.

🇺🇸United States effulgentsia

The composer.json change introduced here raised the requirement to 3.16, while Drupal 10.4 is 3.15. Was that intentional or a typo?

🇺🇸United States effulgentsia

Also, I think that version 1.0 of XB doesn't need to be used on content entity types other than page and node. I think putting a reasonable amount of time into this issue is good, but if we get stuck on a generic solution here, we could punt it to version 2.

🇺🇸United States effulgentsia

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

🇺🇸United States effulgentsia

I don't know what the scope of this issue was originally intended to be, but I think we have all of it covered in other issues:

🇺🇸United States effulgentsia

If Radix doesn't accept the PR, is there a way we can accomplish it via ref? Does Radix already forward refs to the form element we'd want our attributes on?

🇺🇸United States effulgentsia

[PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed could also be a good example for verifying what gets implemented here.

🇺🇸United States effulgentsia

#3491978-7: Implement saving block settings forms raises an interesting question about block settings forms having some characteristics of props forms and some characteristics of page data forms, and I'm wondering if the work done in this issue helps make that clean.

🇺🇸United States effulgentsia

Has the work in this issue turned up specific cases of 📌 Discover cases where is no 1:1 map between field, prop and widget values Active that could be documented on that issue? Also, I'm hoping some of the work here helps elucidate how to then implement those cases as they're discovered.

🇺🇸United States effulgentsia

Not wanting to take the time right now to figure out how to best organize these within the issue summary, but the following issues are still needed as part of "Place Blocks as Components":

I wonder if we should turn each of the top-level requirements, such as "Place Blocks as Components", into their own Plan issue, so that we can make those the parent of individual issues like the above.

🇺🇸United States effulgentsia

@balintbrews: When you create the new meta, please make Create a ComponentSource plugin for JS components Active a child of it. I opened it just now realizing that's a piece that wasn't part of the PoC at all. There's probably some other pieces too that'll require entirely new work rather than just polishing what's in this MR.

🇺🇸United States effulgentsia

I agree with all of #29. I'm conflicted about whether to mark this Fixed (as in, we learned what we needed to from the PoC) or Outdated, but choosing the latter to reduce potential confusion in someone thinking that we delivered any usable implementation as of yet.

🇺🇸United States effulgentsia

However, we shouldn't make Node module a hard dependency of XB. So if it's not efficient to do it as a soft dependency or abstract the solution to content entity types in general, then an xb_node submodule could be a pragmatic choice.

🇺🇸United States effulgentsia

In case there's confusion between #9 and why I opened 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active , the difference was in XB providing default configuration that turned it on for article nodes, which I wanted moved to a submodule, especially since XB doesn't fully work with nodes yet (in the sense of not being able to use fields within the XB canvas).

With the approach being proposed here (an opt-in checkbox for Node types, or potentially expanded to bundles of other content entity types as well), I think it's fine to keep that within the main module.

🇺🇸United States effulgentsia

I mean running the Hub binary (custom build of Caddy), whether on its own or as part of FrankenPHP or via the Cloud version. Not everyone is on hosting that lets you run a long-running binary process or will want to pay for the Cloud version, and that's fine, those people can run XB without real-time enhancements. But there's a growing number of hosting services that make either VPS or container-based hosting available at all the price points from super cheap to enterprise, and https://github.com/symfony/mercure should make it easy to integrate a Drupal site on any PHP server with a Mercure hub.

🇺🇸United States effulgentsia

I'd say for the scope of this issue, just make their text color red. We can defer a better design to a followup issue after this one is done. Better to get this one in sooner than hold it up on figuring out that design.

🇺🇸United States effulgentsia

Retitling to reflect the part that was done in this issue.

🇺🇸United States effulgentsia

With 0.2.0 delayed, do we want a followup for automating this with CI? Or do we think that's too much effort compared to a manual step with every tag?

🇺🇸United States effulgentsia

It was decided in 📌 Remove the XB demo from the 1.0.x branch Active to not ship an XB demo recipe in Drupal CMS 1.0, and instead to have a Drupal CMS 1.1 release in February. Therefore, updating the issue title and summary here to reflect that.

This gives us an opportunity to polish up some of the UI around recently landed XB features (like editing content in global regions, placing and configuring blocks, and others), so perhaps we'll be able to include those even with demo_mode=TRUE for XB 0.2.0 / Drupal CMS 1.1.

If we end up getting all of 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active done before Drupal CMS 1.1, then that will end up being 0.2.0 and the 0.3 number will become available for new scope. But for now, I'm keeping this issue and that one separate, since we might need or want a 0.2 release ahead of completing everything from that other issue.

🇺🇸United States effulgentsia

Another question: should we make experience_builder module itself a hidden module (add hidden: true to its info.yml)? Otherwise, a Drupal CMS user could enable it via admin/modules without going through the recipe. Which wouldn't necessarily be terrible in that they wouldn't be able to actually use it that way (without a page entity getting created which there's no UI for creating it), but would that interfere with #27?

🇺🇸United States effulgentsia

should we disable the entire "XB Component admin UI"? i.e. /admin/structure/component?

Yes, I think we should remove/disable that route when in demo_mode. Nice catch, sorry I didn't catch that earlier.

🇺🇸United States effulgentsia

The following might be moot depending on what happened with #19 and #20, but in case it's not moot, here's a response to #7:

This doesn't get into any configuration which xb might affect but doesn't own, but if there is any, that's another massive can of worms.

With 📌 Add no_ui=TRUE to XB's field type definition Active and 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active , we won't have any. Those field definition ones would be the worst ones to deal with, but those issues should remove that concern. With demo_mode, the only XB field definition will be the one in Page::baseFieldDefinitions(), and that's code, not config.

So if for example a config entity type is renamed, then I don't think Drupal will be able to successfully delete the old config entities as part of the uninstall process.

With the various other issues in the requirements list, I think we'll be down to no saved config entities (while in demo_mode) other than Component. Consider whether to store Component entities outside of config when in demo_mode Active might be a way to safeguard that one.

🇺🇸United States effulgentsia

Added another nice to have: 📌 When demo_mode, add messaging in the UI explaining what it is, what its purpose is, and maybe a link to the issue queue Active .

We might come up with some other requirements or nice-to-haves in the next day or two, but I think the list is mostly complete at this point.

There might also be other tasks for Drupal CMS itself, but this issue is just tracking the XB-specific stuff.

🇺🇸United States effulgentsia

I moved 🐛 Empty global regions add unnecessary spacing to preview Active and 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active from required to nice-to-have, because they're not surfaced unless the "Use Experience Builder for page templates in this theme" checkbox is selected, and 📌 When demo_mode, remove the "Use Experience Builder for page templates in this theme" checkbox Active is in the required list.

I left them as nice to have, because some people might install 0.2.0 outside of Drupal CMS, and fixing those "things look broken right away" bugs would be great to do for those users.

🇺🇸United States effulgentsia

I tested manually and the answer to #17 is that when the "Use Experience Builder for page templates in this theme" checkbox is off, then the canvas renders correctly (no whitespace coming from margins on empty regions). However, when I click the checkbox, then whether or not I select/deselect the per-region checkboxes, it has no effect on the canvas: the layers panel correctly hides the uneditable regions, but the canvas still seems to show whitespace for those uneditable empty regions. Is that expected? Perhaps this MR only improves the whitespace situation when used in conjunction with 🐛 Empty global regions add unnecessary spacing to preview Active ?

🇺🇸United States effulgentsia

Does this fix 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active (or does that already not surface to begin with) in the case where someone doesn't check the "Use Experience Builder for this theme" checkbox? In that case, the regions aren't shown in the Layers panel, but they're still shown in the canvas, so my question is whether dynamically empty ones are shown there with extra margins?

🇺🇸United States effulgentsia

I have not yet seen designs that answer that question, so not sure where that currently is in the design / user-research process.

🇺🇸United States effulgentsia

My initial reaction is whether the denormalization proposed by @catch would get out of sync and lead to all the problems we've had over the years with file_usage. But presumably we could add code somewhere that can rebuild it fairly easily, which I think would alleviate that?

🇺🇸United States effulgentsia

Fair enough. I was thinking it could assist developers/testers who want to test for example, the difference in how the Page Data form looks on pages vs nodes, but locally people can run drush entity:create xb_page xb_page --validate=0 instead. But a hidden module could, for example, be enabled with the Tugboat config. But fine to postpone this if the effort isn't worth the benefit.

🇺🇸United States effulgentsia

The tugboat config should probably also be updated to enable the new module.

🇺🇸United States effulgentsia

I think this looks great. I approved the MR for the subsystems I'm a code owner of, but it still needs additional approvals.

I can imagine a UI at some point that supersedes the need for this config (for example, eye icons within the Layers panel that toggle visibility), but I think this is a great step until that point, and even if we end up needing to deprecate this config after we provide upgrade paths, this should be easy config to deprecate following existing core patterns of deprecating config.

🇺🇸United States effulgentsia

I did, because I thought it wasn't time yet to be tracking general "fix broken stuff" issues here yet. But I just now restored it under a new "Miscellaneous high priority fixes" heading.

🇺🇸United States effulgentsia

Retitling to focus on the goal. An iframe might or might not be the best solution for it.

🇺🇸United States effulgentsia

This looks great, but if there's time, what do you think of adding a test for this? Mostly just to prevent this line from accidentally getting deleted in some future MR.

🇺🇸United States effulgentsia

Added 🐛 Empty global regions add unnecessary spacing to preview Active and 🐛 [Needs design] Previews of pages containing (dynamically) empty blocks are malformed Active as blockers, since we don't want the first tagged version since DrupalCon Barcelona and the first version in Drupal CMS to look immediately broken.

🇺🇸United States effulgentsia

I haven't reviewed this MR so don't know it's full scope or how close it is to landing. In case it helps with landing all of or parts of this, or descoping all of or parts of this, I just want to add some background that our long-term goal (actually, short-term goal, but possibly not in time for 0.2.0) is for regions without any blocks or components in them to not be shown in either the canvas or the layers panel. Instead, the only way to move a block into a region that doesn't already have blocks in it would be via a right-click. However, since that might not be feasible in time for 0.2.0, the MR here might still be a good improvement worth merging. Also, what I just wrote is just about regions without any blocks/components in them, that wouldn't address issues with blocks that have empty content.

🇺🇸United States effulgentsia

I think we should make two quick hard-coded fixes for now, and then figure out how to abstract and provide UI affordance in a follow-up:

  • Don't render the Highlighted region within the canvas. It can still be in the layers panel.
  • Don't render the Help block within the canvas. It can still be in the layers panel.

My premise here is that what editors generally want to see in the canvas is what their site visitors will see under typical circumstances, not what only other admin users will see or what site visitors will see only under rare circumstances. There's plenty of gray area around this, but the above two items seem pretty clear cut to me, and I wonder how much closer the canvas will get to the preview with just those two fixed.

🇺🇸United States effulgentsia

I added 📌 Add no_ui=TRUE to XB's field type definition Active and 📌 Move the addition of the XB field on the Article bundle to a hidden submodule Active as child issues, and to the list in the issue summary. The combination of them will keep people from being able to use XB on anything without some other code enabling it somewhere. In the case of Drupal CMS, that enabling code is the creation of a single Page (not node) entity as part of its recipe. In the case of XB developers/testers, that enabling code would be the installing (via Drush) of either the xb_test_node or xb_test_page (hidden) modules.

In the case of Drupal CMS, this means that when we later change the field schema of the field (which we likely will), we can just delete the single page entity, and not worry about the XB field being present anywhere else.

Production build 0.71.5 2024