🇺🇸United States @effulgentsia

.

.

Organizing list of issues in summary into sections.

Closing as a duplicate of 📌 Frontend changes to allow editing props and slots for code components Active per #3.

Closing this as a duplicate of 📌 Frontend changes to allow editing props and slots for code components Active per #3.

.

No backend changes are needed for this and 📌 Frontend changes to allow editing props and slots for code components Active includes the frontend changes.

.

I believe we discussed at some that tags is something we we'd likely want to support (and it's supported by UI Patterns already). However, if it's something we'd, we'd do it in a later issue.

Given #62, why not add tags in this issue and document expected as being able to contain SDC IDs or tags, but not group?

Opened ✨ Allow linking integer timestamps to `type: string, format: date` Active for the last sentence of #25.

effulgentsia → created an issue.

Ah, thanks! I missed that the group key was already a thing. I'm happy to RTBC or merge this once #58 is resolved.

There's still the last sentence of #56: we're saying that expected can be a tag or group, but what should display builders match that against? Shouldn't this issue also standardize on that: e.g., a tags or groups or some other top-level key of *.component.yml file?

+1 to expected.

For minimum and maximum, would minItems and maxItems be closer aligned to JSON schema semantics, which uses the latter pair for arrays whereas the former pair is only for numbers?

expected with a list of values: SDC plugin IDs (if they have a colon inside) or tags/groups (if no colon inside)

For the tags/groups case to work, do SDCs already have a way to identify which tags/groups they're in? If not, can this issue's scope also include defining such a property?

Adding what this is postponed on as a related issue.

@lauriii and I discussed this at one point, so I can answer for him.

The children of a deleted slot should be removed when the host entity (Page, Content Template, etc.) is edited (i.e., what's in auto-save storage is updated). Until then, the children should remain in the server-side data but not get rendered or returned in HTTP API responses.

If someone adds back a slot with the same name before the above happens, then the children should be back in action as though the slot never got deleted.

We may want a follow-up to figure out if there are any conditions under which orphaned children are problematic and what to do for those cases.

The other MR doesn't (and shouldn't) clean up the Canvas code that forwards refs, so repurposing this issue to that.

#4 also reminded me to open ✨ Enhance the UX of using Tailwind classes within the Code Editor Active

effulgentsia → created an issue.

Should the scope of this issue also include configuring our CodeMirror instance to use tsxLanguage?

This is a great start. I haven't tested it yet, but my hunch is that this won't work end-to-end because the Astro island won't hydrate the children prop correctly when calling the React code. The fix to that might be as simple as changing AstroIsland::generateTemplate(), where it currently has an if statement for $slot_name === 'default' to change that to 'children'. If the previous sentences are all true, we should probably add an E2E test for this that actually tests the component working as desired in the browser.

One question I have is whether to have the if statement be only $slot_name === 'children' or whether to make it do that logic for both 'children' and 'default'. The question is where within Drupal core, or within SDC documentation, if anywhere, do we define the semantics for a slot named 'default'. If those semantics aren't defined anywhere, then we can limit default slot behavior to only 'children', since 'children' is the only React-defined prop for this concept. If Drupal defines semantics for 'default' as well, then we might need to somehow reconcile between 'default' and 'children'.

https://www.drupal.org/blog/drupal-canvas-is-now-available-inside-drupal... → :)

This would have been really nice to fix before 1.0.0 since if you manage to run into this, you end up with invalid data saved with the component instance, and data integrity bugs are always very annoying.

However, the mitigating factor here is that you don't run into this unless you're bypassing the UI (e.g., with the canvas_ai module which is a hidden module) or there's a UI bug elsewhere, which there were when this was opened but those were fixed already.

Give that mitigating factor, I'm downgrading this from a stable blocker to a post-stable priority.

Given #3547579-40: Changes to media types should force re-invoke of hook_storage_shape_prop_alter → , I wonder if we can make any clearer that even the hooks documented in canvas.api.php aren't public API yet. Perhaps an @internal on them as well? And/or explicitly calling that out in API.md and/or the first line of canvas.api.php. Or maybe even just deleting canvas.api.php entirely until we're ready to make those hooks public API?

The main reason we were trying to get this resolved before the 1.0.0 release is that it's making a breaking change to a hook that's documented in canvas.api.php. However, we can document in 📌 Document our @internal API policy Active that this isn't a public API yet.

Also, someone could run into this bug when adding media types after installing Canvas. But, even though that's annoying, a full cache clear resolves it.

So, downgrading this from a stable blocker to a post-stable priority. This issue is close. We'll hopefully get it fixed next week and can release a 1.0.1 or 1.1.0 with it.

I worry that immediately after this effort of sprinkling @internal over hundreds of files

Unless I'm counting wrong, I only see 19 interfaces, 15 traits, and 10 *Base classes in our codebase.

Here's my recommendation for what we should declare as our public API:

• PHP interfaces unless they're annotated @internal.
• PHP classes that end in Base unless they're annotated @internal.
• PHP traits unless they're annotated @internal.
• Everything annotated @api.
• Hooks declared in canvas.api.php.

.

• Paths declared in openapi.yml that start with /canvas/api/v followed by a number >= 1.
• Config schemas.
• No JS code at this time.
• Nothing in any submodules, such as canvas_ai, at this time.

Meaning, the above, and only the above, is what we apply semver to (i.e., raise the major version number if knowingly breaking).

If there's agreement on the above, next steps would be to:

• Document that somewhere (maybe an API.md file in the codebase?)
• Add @internal to all of our interfaces, base classes, and traits.

Tagging this as a stable blocker, so that we comply with semver, which states "Software using Semantic Versioning MUST declare a public API." It's okay for that pubic API to be close to nothing, or maybe even nothing, but what it is needs to be declared in some way.

effulgentsia → created an issue.

PHP 8.5 now has a spec-compliant URI parser, and there's also a polyfill for it for PHP 8.1+. I think we should file an upstream issue for JsonSchema to switch out their incorrect regex for this. In the meantime, does JsonSchema support us overriding its UriValidator?

RC5 was released today and we're now very close to a 1.0.0 release. We're intending to release it on Dec. 4. Rewrote the issue summary accordingly.

This is the last remaining issue tagged "rc target" and we're down to only a few issues tagged "stable blocker" so there's no longer a meaningful difference between the two, so I'm re-tagging this so that we have a single list of stable blockers.

The purpose of a Canvas 1.0.0 stable release is for people to be able to confidently use Canvas in production, and it seems like this bug would significantly detract from that confidence, so tagging it as a stable blocker.

I'm not entirely sure that "auto, 100vw" — assuming it was truly missing a comma — is the best default value. Looks like auto can only be used with loading="lazy", and that it also needs width and height

• The missing comma is a typo. Yes, we should add the comma.
• I think it's the correct default value, but if we want, we can add logic inside image.twig that checks if it starts with auto, and if it does then also check loading, width, and height, and if they're not compatible with auto then strip that part out. I thought browsers implicitly did that, but if they don't then that's fairly easy logic to add to our Twig file.

And lol, to illustrate the point, the status change in #6 was also an x-post.

#3 doesn't have a comment saying why the "stable blocker" tag was removed, so I'm assuming it was unintentional and due to a d.o. cross-post, so restoring it.

🐛 Image lost when mapping a field Active landed a while ago, so moved it to the Completed list.

🐛 Linking and unlinking a field does not correctly reset the form view to it's initial state Active is newly discovered. Adding it to the Remaining front-end section though I don't know at this time if it's a front-end or back-end bug.

If this happens in all cases, then I think it should block a stable release, so tagging for that, and also adding it to 🌱 [META] Content templates Active . If, however, this only happens under certain circumstances, we could consider not blocking a stable release on it.

I think the "absolute or relative URLs" part of this issue should be stable blocking per #4, since that affects shape matching to uri vs uri-reference props. I think supporting other options and link templates can be a post-stable follow-up unless it's trivial to include here as part of solving for the absolute option.

RTBC+1. I approved for the code groups that I'm a code owner of but that still leaves a couple groups that need approval.

I like it!

Thanks for highlighting #14. Can you give an example of a description that you would write differently for the 3 cases? My current assumption is that it's better for SDC developers to only have a single description to fill in and that SDC developers should just be thinking about how to best describe the prop/slot without concern as to whether it's developers, AI agents, or content creators that are reading that description. Or am I being naive about that?

Discussed this with @lauriii and seems like the stable blocking parts of this have already landed so what remains doesn't need to block a stable release.

Given the issue title, "won't fix" is the more accurate status. I probably should have changed the commit message in #19 to something else, but oh well.

I'm able to reproduce it now. They key is to set a static image, then refresh the browser page, then link a different field.

I'm not able to reproduce this. Are others seeing this?

I discussed this with @wim leers and @lauriii. Ideally, since beta1 marked the point where we started promising to not break existing site data, we'd get this update path in before RC1. However, this will be the first update path that requires updating content entities, so it might take some time to write it and make sure it works properly, and DrupalCon is starting next week. Meanwhile, the lack of this update path doesn't result in a broken site per se, just some component instances (ones with HTML props) not being editable and needing to be replaced with new instances of the same components, copy the values from the old instance to the new one, and delete the old one. Definitely annoying, but beta1 hasn't been out that long, so not that many sites affected by it, and for the sites that are, it should only require a few minutes of manual effort. Therefore, downgrading to an RC target, meaning a high priority thing to work on after we tag RC if it doesn't make it in before.

Thanks, @tedbow! Per #44, this was tagged as "RC blocker" until we knew what the issues were. Now that we do, I'm downgrading it to "RC target". I think the current MR is great, so if other reviewers agree, let's merge it and make the RC that much better, but if reviewers spot problems with it, we don't need to hold up the RC on it.

To summarize:

• #42.TC03 happens with a Drush delete of the component entity, not the component's source (Twig file, Block plugin, or js_component entity). I don't think that needs to be RC blocking: people aren't as likely to be deleting the Component entity during component development, they're more likely to be deleting component sources.
• #42.TC08 was fixed in #3548922: A component appearing in the list of components to instantiate that is broken during development should convey that → .
• #46 and #51 are fixed by the current MR. Let's merge it if it looks good and not likely to cause other problems.

I think conceptual integrity is very important.

I agree. If I were writing this in spec language, I'd say Content templates SHOULD make use of at least one dynamic prop source, but I hear you that you'd prefer it to be Content templates MUST make use of at least one dynamic prop source

Over a year ago, I argued that an MVP feature for the Content Templates UI should be to visualize which fields have not yet had their data mapped.

I disagree that it's needed for MVP, but I love the idea for that to be added in the future sometime. And I think that would be a very helpful way to guide template builders towards the SHOULD without making it a MUST.

Merged the MR to get the fix into the RC, but back to NW for some kind of test to prevent regression.

I pushed a commit to change it from [] to {}. Rerunning tests.

Would it be possible to add a test to catch this from happening again in the future? Could be a follow up.

Code looks great. Tests pass. #19 confirms it addresses what Drupal CMS needs.

Yay that the fix itself got in for beta2! The update path doesn't need to block RC. In the meantime, people can manually edit and re-save their existing content that uses HTML props that are getting double escaped. Though since we're already past beta, it would be courteous for us to provide the update path relatively soon and certainly before a stable 1.0 release.

📌 Follow-up for #3541037: Improve how list of field suggestions is displayed in the UI for `ContentTemplates` Active landed, which was the last issue needed to take Content Templates out from requiring canvas_dev_mode to be enabled to just being an enabled feature out of the box, so in a way, the 1.0 scope of this meta issue is done!!!!

Still keeping this meta issue open for the 2 test coverage issues that are still remaining and to settle on 📌 Require content templates to have at least one dynamic property source Active which @Wim Leers and I are still debating on that issue whether to do it or not.

You already say what the problem is: it's not a template. That's it. So why would we allow it?

I think where we're stuck is that the arguments for allowing it aren't very strong, but neither are the arguments for disallowing it, so it's a judgment call of which set of weak arguments is stronger.

Doesn't that mean you'd still need to pass — at minimum — pass the UUID of the targeted entity into the instance of the code component?

Not for a Page Title block or a JS component that gets the page title from drupalSettings. But perhaps Page Title is a relatively unique exception where we can just expand this issue to include that as well: i.e., dynamic or host-entity-url or page title? But then I wonder how many more things this will end up needing to expand to, and ultimately what's the benefit that's gained from figuring out and implementing that list of things to check for.

As an analogy, we refer to Twig as a templating language and our Twig files as templates, but nowhere do we enforce that every Twig file has to render at least one variable. Are there any good use cases for Twig files that don't use or output any variables? Maybe it's debatable, but would it be more of a benefit or a detriment to stop people from doing that if they want to?

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.

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.

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.

"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?

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.

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?

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?

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.

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.

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.

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.

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

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?

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.

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.

This is blocked on 📌 Refine API response with DynamicPropSource suggestions to provide better UX Active .

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?

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

📌 Refine API response with DynamicPropSource suggestions to provide better UX Active was missing from the issue summary. Adding it because 📌 Follow-up for #3541037: Improve how list of field suggestions is displayed in the UI for `ContentTemplates` Active depends on it.

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

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?

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.

effulgentsia → created an issue. See original summary → .

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

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.

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?

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".

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.