🇺🇸United States @effulgentsia

Account created on 2 September 2006, about 19 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States effulgentsia

The original MR was merged before beta, but tracking the newly discovered failures in #42 as an RC blocker. We don't necessarily need to fix them before RC, but we should determine what the cause of the errors is and then make a determination based on that whether or not to hold up an RC on fixing them.

🇺🇸United States effulgentsia

While Prop implies component (things that aren't components can have properties, but only components have props), Input does not. So I'd suggest ComponentInputSource.

🇺🇸United States effulgentsia

So I guess one future possibility is if in the future we decide to allow block plugins to opt into Canvas-generated forms (instead of the block plugin's form), in which case we'll be able to use what are currently called PropSource(s) to populate those inputs. So if we want to be forward-thinking towards that future, then yeah, renaming away from the Prop terminology would be good.

🇺🇸United States effulgentsia

"Explicit input" is the generic name for SDC/JS props and Block settings. But the concept of PropSource only applies to the former, not the latter, so I think keeping it as Prop rather than Input would actually be better. Unless we're thinking of a future where prop/input sources can be used to populate inputs that aren't props? But what would be an example of that?

🇺🇸United States effulgentsia

Thanks! Looks great so far, but needs an upgrade path (post_update function?) for existing Component config entities (anywhere else?) unless those are automatically regenerated with this new expression on cache clear.

🇺🇸United States effulgentsia

This also came up in Slack: https://drupal.slack.com/archives/C072JMEPUS1/p1759680539277759. Not just making whether or not the prop is required conditional on another prop, but also making whether the prop (conceptually) exists conditional on another prop.

I think it's up to display builders to implement the UI for this, but I think SDC's need to standardize on how to represent/encode the desired connections. Perhaps we should use json schema's if/then/else syntax for this?

🇺🇸United States effulgentsia

Enum labels are being handled in 📌 Add Support for Defining Labels and Values for List Options in SDC Code Editor Props Tab Active , so should we scope this issue down to just prop machine names?

🇺🇸United States effulgentsia

We definitely don't want people using the |raw filter in Twig to work around this, so tagging as an RC blocker. @larowlan and @Wim Leers have some prior art from a different issue for how to fix this, so assigning to @larowlan because his workday starts sooner. If he can't get to it, we'll make sure that someone else does early this week.

🇺🇸United States effulgentsia

Thanks for reporting this. I added FormAjaxException to the scope of 🐛 Lack of support for EnforcedResponseException breaks form component submission Active which was already in the process of fixing the same problem for EnforcedResponseException. So with that adjustment to that issue, closing this one as a duplicate of that.

🇺🇸United States effulgentsia

Let's add FormAjaxException to this as well. It's the same problem, just the other type of exception that's thrown by FormBuilder. Reported in 🐛 Unable to submit form via Ajax in a Canvas page Active which I'll close as a duplicate.

🇺🇸United States effulgentsia

The challenge in #8 is that this is how Olivero (and any other theme doing something similar) determines what region a block is in:

function olivero_preprocess_block(&$variables): void {
  if (!empty($variables['elements']['#id'])) {
    /** @var \Drupal\block\BlockInterface $block */
    $block = \Drupal::entityTypeManager()
      ->getStorage('block')
      ->load($variables['elements']['#id']);
    if ($block) {
      $region = $block->getRegion();

      if ($variables['base_plugin_id'] === 'system_menu_block') {
        $variables['content']['#attributes']['region'] = $region;

That last line then allows menu rendering to know what region the menu is in.

However, in Canvas, $variables['elements']['#id'] is the component instance's UUID, which does not correspond to a block config entity, so you don't enter the if ($block) conditional, so the menu then doesn't know its region.

Not yet sure what the best fix for this is. This might also be a slightly different issue than this issue's original title and summary, but I think it's covering the same general territory of the consequences of rendering blocks without the config entity that Block UI normally creates.

🇺🇸United States effulgentsia

Thank you for opening this. It would be great to get this resolved before RC1 or RC2 if we can, so tagging for that.

🇺🇸United States effulgentsia

I think we should go the other way and make sure that when a Canvas field is rendered on the published page, it's rendered without all the theme('field') stuff. The mental model should be that on a Page and on a Content Template (and when we get to it, within exposed slots of nodes), the Canvas defines the entirety of what's being rendered within the "content" region and/or exposed slot.

The use case is to add a CSS class that sets default spacing on all direct children below it

The <body> has a canvas-page class, and the content region has a region--content class, so can you achieve this use case by using .canvas-page .region--content as your selector or prefix?

🇺🇸United States effulgentsia

Since there's what I think is a working MR here, I'm assuming this is "Needs review", not "Active", but please correct the status if I'm wrong about that.

🇺🇸United States effulgentsia

I think #6 is great and we should go with it. If/when anyone wants to design it differently than that, we can open a follow up for further refinement.

🇺🇸United States effulgentsia

We have 📌 Follow-up for #3541037: Contextual panel flickers when linking prop to field Active and 📌 Follow-up for #3541037: Improve how list of field suggestions is displayed in the UI for `ContentTemplates` Active as separate individual issues. Is there any other UI polish needed that this issue is helpful for tracking or can we close it?

🇺🇸United States effulgentsia

🐛 500 Error When Editing Code in Custom Code Component under Article Template Active was missing from the issue summary. Adding it.

🇺🇸United States effulgentsia

For easier scanning, moved remaining "must have" issues for 1.0 to the top.

🇺🇸United States effulgentsia

Which presumably means the only way to fetch this information today would be to iterate over all Workspaces, perform a query for each using \Drupal\workspaces\EntityQuery\Query::prepare() to check usage in the latest per-workspace revision?

Does core's Workspaces module allow the same entity to be in more than one workspace? If not, then latestRevision() should be sufficient for at least that. What are the most popular contrib modules that enable multiple branches of forward revisions that we can use for testing the more robust approach?

🇺🇸United States effulgentsia

The MR that implemented what's in this issue's title was already merged in #47. For the follow-up, I opened 🐛 Prevent deleting Code Components if there are usages in forward revisions Needs work , but perhaps an issue summary update here or somewhere is still needed, so I'm setting the status to "Patch (to be ported)" rather than Fixed to indicate that its current scope as indicated in the issue title is done but that this issue still needs housekeeping.

🇺🇸United States effulgentsia

What's the reason to use a node type instead of the Page entity type for that?

🇺🇸United States effulgentsia

I was under the impression that exposed slots were post 1.0 - is that not the case anymore?

No, that is still the case, so the concern is valid that sites needing exposed slots won't be able to adopt Canvas 1.0 and will need to wait some additional amount of time.

🇺🇸United States effulgentsia

for little gain outside of those using Olivero theme

I think this is misunderstanding what the purpose of this issue was due to an out of date issue summary. The purpose here wasn't to fix a theming mismatch, though that's how this issue was initially opened as, it's that we're not wanting to support/maintain the concept of an individual node using Canvas other than via an exposed slot in the content template.

this severely hamstrings early adopters

You mean between now and when exposed slots are implemented? What are early adopter use cases that you know about where Pages and Content Templates (without exposed slots yet) are insufficient?

🇺🇸United States effulgentsia

A ContentTemplate that is either empty or contains only StaticPropSources no longer is a template for presenting structured data.

I agree with the spirit of this but not the letter. Per #8, you can have a content template that renders structured data from the node, just not via a DynamicPropSource. For example, a Block, such as the PageTitle block or a block that renders some other field from the node, or a JsComponent that does similar via drupalSettings or a json:api request.

I don't think it's pragmatic to try to come up with a way to validate for this broader set of ways of rendering a node's data. But even if it were, we'd have to contend with:

maybe you just want to preview how a static template would look like

And auto-save might not be enough for this. For example, say you're in the process of building a site that you haven't deployed yet, and you're building a View of teasers. During that process you might want to create your Teaser template with only static design elements to start with but you'd want to publish it, so that you can see what it looks like on the Views page.

Beyond the preview use-case, there's also the "quick way to hide content from a published site" use case. For example, say you have a site with a certain node type, like Issues, or Case Studies, or whatever. And for some reason you need to temporarily hide the content from all of those nodes until you resolve whatever problem needs resolving. And maybe you want to customize some message to show instead. One way to do that could be to edit the template for that node type, replace its contents with a static component with a static message, and publish that. Does that mean this isn't technically a template anymore in the purest sense of the term? Yes. But should we stop you from doing this?

As far as whether this as a data integrity issue, it's not like entering "foo" for an integer, or into a reference field where there isn't any "foo" entity to reference. In those cases there's a reasonable expectation of an error being triggered without validation to guard against it. But what reasonable error would we be guarding against if someone publishes a template without anything dynamic in it? Sure, they will have created a "template" that's not a "template" in the normal meaning of the term, but what real problem arises from that?

In summary, I think it's too much trouble to try to validate for "there's something dynamic being rendered" that covers all the ways that dynamic things can be rendered, and there's also some semi-legitimate use cases for creating a purely static "template" even if doing so is counter to the meaning of "template", and there's nothing about how we render templates that we could reasonably envision breaking if nothing dynamic gets rendered, and so for all those reasons, I think this should be "won't fix".

🇺🇸United States effulgentsia

There are very likely zero fields in existing Drupal sites that contain inline-only markup (i.e. with no

,

    and other block-level elements). So any such SDC props are unlikely to be able to get populated by structured content.

I think inline HTML props should match to both TextItem and StringItem, just not to TextLongItem or TextWithSummaryItem.

For TextItem matches, I think we'll need to take the ->processed property and run it through an additional strip_tags() that retains only inline HTML. That might mean we need to implement 📌 Allow use of same-shape-adapters ahead of general adapter support in #3464003 Active first.

CKEditor 5 currently assumes
is supported at minimum

Yeah, we need to either get this fixed upstream or somehow hack around it.

🇺🇸United States effulgentsia

Is the "preview LIES" part of this issue title still accurate? The way I read the comment linked in #3 is that there are situations where you can publish something while it still have invalid values. If those cases still exist, shouldn't those be separate issues? Meanwhile, if you can't publish with invalid values, then what do we mean by the preview lying?

🇺🇸United States effulgentsia

Not every stable blocker needs to block an RC, but this one does.

🇺🇸United States effulgentsia

Not every stable blocker needs to block an RC, but this one does.

🇺🇸United States effulgentsia

Not every stable blocker needs to block an RC, but this one does.

🇺🇸United States effulgentsia

Beta blockers are by definition also stable blockers, but removing the latter tag so as to make it easier to scan a queue of stable blockers that aren't also blockers to beta or RC.

🇺🇸United States effulgentsia

Beta blockers are by definition also stable blockers, but removing the latter tag in order to make it easier to scan a queue of stable blockers that aren't also blockers to beta or RC.

🇺🇸United States effulgentsia

Transferring beta blocker tag from the duplicate issue.

🇺🇸United States effulgentsia

No arguments were made since #20 for keeping this issue open, so marking it Fixed.

🇺🇸United States effulgentsia

#33 doesn't need to block a stable release unless we already know of or suspect a problem.

🇺🇸United States effulgentsia

I think #13 answers #12's question to @lauriii.

And yes, implementing what the issue title now says needs to block the beta1 release, since it's a significant BC break of site data for sites that have already created component trees for individual nodes (articles). We'll need to include release notes that instruct people to manually copy (yay for copy/paste functionality in Canvas!) those component trees from nodes to page entities before they update their site to beta1 (or alphaN if there's an alpha tag that includes this removal prior to the beta1 tag).

🇺🇸United States effulgentsia

The sooner we do this, the sooner we get to reap the benefits, but I don't see why we should block a 1.0.0 release on it. Seems like the kind of thing that could non-disruptively go into 1.1.0 or even 1.0.1. Therefore re-tagging from "stable blocker" to "stable target".

🇺🇸United States effulgentsia

I'm all for naming things well. This rename can happen either before or after a stable release.

🇺🇸United States effulgentsia

Per 🌱 Milestone 1.0.0: Production Sites Postponed this is a requirement for an XB 1.0.0 stable release, so tagging it as such.

🇺🇸United States effulgentsia

We can release XB 1.0.0 without this, so switching from "stable blocker" to "stable target".

🇺🇸United States effulgentsia

Within the preview/canvas iframe, global assets are rendered via drupalSettings.xb.globalAssets, which includes jsFooter. Can you clarify steps to reproduce the bug you're seeing?

🇺🇸United States effulgentsia

In fact, since we haven't released the beta yet, we should get this into that as well.

🇺🇸United States effulgentsia

If the hypothesis in the issue summary is accurate, this is a stable blocker, so tagging it as such.

🇺🇸United States effulgentsia

I discussed this issue with @bnjmnm. I believe the correct status of this issue is "duplicate" because per #12.1 it was a symptom of 📌 Redux-aware form input components should be aware of direct element manipulation Active which got fixed.

However, this issue was not the one encountered by the Webform block. Instead, Webform was running into 🐛 Autocomplete machine name handling does not match to machine name Active , which was a symptom of 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active so is correctly marked as a duplicate of that one.

Unrelated to either this issue or to 🐛 Autocomplete machine name handling does not match to machine name Active , there is a remaining bug described in #3523496-40: Block component instance form values not processed by validation/submit handlers .TC3. @bnjmnm is working on diagnosing that one and will open a new issue for it based on what he learns.

🇺🇸United States effulgentsia

For MySQL itself, it's a bit more vague: https://launchpad.net/ubuntu/noble/+package/mysql-server but de-facto you still get 8.0 it looks like. However I wonder if they're going to switch that mid-release to 8.4 (supported until 2029 according to https://endoflife.date/mysql) as 8.0 goes EOL). So maybe it would be fine to require 8.4 according to #8 if ubuntu installs get updated anyway.

I don't see what's vague in that link. It looks clear cut that Ubuntu 24.04 is on MySQL 8.0. I don't think there's any precedent for Ubuntu to raise the minor version of MySQL (or any other significant package) within an LTS branch that has already been released. Based on all of their track record, I expect Ubuntu to security patch MySQL 8.0 for as long as 24.04 is supported (till 2029 minimum and beyond that for extended support). I think #8 would clearly dictate having Drupal 12 require MySQL 8.0, not 8.4. I don't know if we have consensus on #8, but I'm still in favor of that. Most people are not going to upgrade from Ubuntu 24.04 to 26.04 within the first year of Drupal 12 being out, and while it's true that those people can remain on Drupal 11, I don't think we gain enough from raising the MySQL minimum to 8.4 to be worth the tradeoff in holding back Ubuntu 24.04 users from upgrading to Drupal 12.

🇺🇸United States effulgentsia

Oh! Well that seems like something that's both relatively easy to do and would reduce confusion for beta testers, so tagging it as a beta target, but not a beta blocker.

🇺🇸United States effulgentsia

Closing this as outdated. The Undo and Redo buttons work, and if there are future designs to change their location, new issues can be opened for that then.

🇺🇸United States effulgentsia

Untagging "stable blocker", because XB 1.0.0 does not require a public extensions API. That can come in future minor 1.x releases. The goal of XB 1.0.0 is to get it into people's hands for building and deploying real websites with it, and they don't need to extend the XB UI to do that.

🇺🇸United States effulgentsia

Initially we were only planning to postpone Workspaces to a later 0.x version, but now we're postponing it till after 1.0.0, so re-titling this issue, and we may want to adjust the language in the MR as well.

Where Workspaces will become increasingly crucial is for editing individual nodes within XB, but that's also something we're postponing until after 1.0.0. 1.0.0 will just be for Pages and Content Templates.

I agree in principle that ADRs are valuable and that they're more valuable sooner than later, so I'll try to make time for this when I'm able to, but we're not going to block a stable release on getting this ADR published.

🇺🇸United States effulgentsia

Removing "stable blocker" tag but we'll want to fix this as part of the Content Templates work, so setting the issue parent accordingly.

🇺🇸United States effulgentsia

See #3503038-15: Refactor GeneratedFieldExplicitInputUxComponentSourceBase and FieldForComponentSuggester to need only SDC's ComponentMetadata, not SDC plugin instances . I'm very much in favor of this issue, but we're trying to pare down the "must haves" for a stable 1.0.0 release in order to unblock a stable Drupal CMS 2.0 release as soon as possible.

🇺🇸United States effulgentsia

I read #10, but I don't think this is a stable blocker. We'd be happy to merge this once it's ready though, so by untagging it I have no desire to slow this issue down.

For XB, the definition of stable is different than core's. We'll be adding @internal to almost everything and relaxing that in 1.x minor releases. The main goal of an XB stable release is to unblock a Drupal CMS 2.0 stable release, allowing people/agencies to build and deploy real sites with that. But XB 1.0.0 won't yet have a proper PHP API surface for contrib projects to perform more advanced integrations than what DCMS 2 exercises, except to the extent that issues like this one just happen to get done ahead of a 1.0.0 release.

🇺🇸United States effulgentsia

Now that we're effectively in beta, changing route names requires a BC layer

I haven't reviewed the MR to have an opinion on whether or not we should change the route names. But if it seems like the new names are better, then we can do so without providing a BC layer. For XB, beta only means data stability, not API stability.

🇺🇸United States effulgentsia

Question about the tags added in #5: what contrib project is this blocking?

🇺🇸United States effulgentsia

Removing the "stable blocker" tag, because we have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker, and that's the only part of this that seems like a must have before a 1.0.0 release. Implementing #2 from this issue's summary or a hybrid solution allowing some blocks to be #1 and others #2 seems like it could happen after a 1.0.0 release. Please re-tag if you disagree.

🇺🇸United States effulgentsia

Untagging "stable blocker" because I don't see why we couldn't add client-side validation at any time, including after a 1.0.0 release, but please re-tag if you disagree. We already have 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active as a stable blocker for the server-side part.

🇺🇸United States effulgentsia

Closed 🐛 The navigation prevention inside the preview doesn't always work for code components Active as a duplicate of this. That one was tagged as a stable blocker, so transferring that tag to here.

🇺🇸United States effulgentsia

Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.

🇺🇸United States effulgentsia

Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.

🇺🇸United States effulgentsia

Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.

🇺🇸United States effulgentsia

Although we're working on Personalization in parallel to trying to get a stable XB 1.0.0 released, the former is not a blocker to the latter. If Personalization itself isn't sufficiently stable by the time we're ready to release XB 1.0.0, we can put it behind a feature flag.

🇺🇸United States effulgentsia

Downgrading priority and removing the "stable blocker" tag because I don't think there's too many cases left where uncaught exceptions make it through to the generic "unexpected error" message. We can re-raise priority or re-tag as a stable blocker if we start seeing more of those. Regardless of priority or whether it's stable blocking, addressing this would be good for maintainability, so leaving that tag in place.

🇺🇸United States effulgentsia

For XB 1.0.0, we don't need to support editing individual nodes, or other entity types other than Page, within XB. That's separate from being able to make content templates, so I commented on that separation in #3518272-3: [PP-1] Validate that content templates are attached to content entity types (+ bundles) that fulfill XB's requirements .

We can add the ability to edit individual nodes, custom blocks, paragraphs, etc. within XB after 1.0.0. Until then, those things can be edited outside of XB.

🇺🇸United States effulgentsia

Also, what might make sense here is for XB 1.0.0 to only allow content templates for nodes, and punt any support for creating templates for other entity types (e.g., paragraphs, custom blocks, etc.) to after 1.0.0.

Production build 0.71.5 2024