🇺🇸United States @tedbow

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

@NarendraR, could you address @wimleers' review? Feel free to only address the ones you feel you have clear direction on, and then re-assign them to me to take on the others. Thanks!

re #49 I need to re-review the tests still

FYI I think @vishal.khode is working on this. I can't assign it to him

Back to @wimleers to double check this https://git.drupalcode.org/project/canvas/-/merge_requests/466#note_657291

just so we don't forget, still needs "Needs issue summary update" for #32. I don't time this second to do it

Just talked to @lauriii again. He said if contextual links are a big problem we could add a link using core's "Navigation" module instead

I think this will be easier because of 🐛 Contextual links are not displayed for nodes using Canvas ContentTemplates Active . Though I have not used the Navigation module API so I am not sure. For those using the Navigation module the UX would be better too

@vishal.khode filed this core bug 🐛 Navigation module breaks with non-routed URLs in local tasks Active found by another Canvas issue but since the plan per #26 is to make an actual route then it should not affect use.

re #32 . thanks. I don't this will be hard. It might be easier.

We will see

credited vishalkhode on https://new.drupal.org/contribution-record/11437734 because he found the bug!

1. @wimleers re #26
   

   Wouldn't it be preferable to introduce a new canvas.boot.template_with_preview_entity route (similar to the existing canvas.boot.entity route), so we can use the Contextual Links API as intended?

   I agree this would be much better.

   There is a related problem 🐛 Either canvas.boot.entity is unused or it is very confusing Needs review but I don't think that should stop us from making the new `canvas.boot.template_with_preview_entity` here. canvas.boot.entity still works for the purposes of the creating links just some logic in `\Drupal\canvas\Controller\CanvasController::__invoke` is never actually use right now. We won't have to solve that here but should make sure we don't make the same mistake with canvas.boot.template_with_preview_entity

2. We need clarity around #29 in any case. adding "Needs issue summary update" because the summary should be update once we know the answer
4. I don't the think contextual links will work until we solve 🐛 Contextual links are not displayed for nodes using Canvas ContentTemplates Active either in that issue or here
5. I tried to push up failing test coverage for ^^^^ but I don't think it is correct yet. Manually testing I have confirm this. Testing context links is pain

@lauriii

I have a question about the requirements.

The original title of this issue was "Provide a link from canonical URL to edit content template (when in use)"
then in #20 @wimleers changed it to "Provide a contextual link to edit content template (when in use)"
then in #22 @vipin.mittal18 changed this to "Add “Edit Content Template” link in contextual menu to navigate to Canvas Editor"

Notice in #22, "(when in use)" was removed. I am not sure if this was intentional, but it does change the meaning significantly.
To my understanding, "(when in use)" would mean when it is published and affecting the node the user is currently looking at when they use the new contextual link.

Currently, the MR is actually explicitly making the “Edit Content Template” link available when the ContentTemplate is unpublished also.

This matches the summary:

The contextual link must be conditionally visible only when both of the following conditions are met:
A content template exists for the node’s content type.
The current user has the administer content templates permission.

So "exists" would imply that the link should exist whether or not the template is published or not.

So this would mean as soon as you submit the "Add new template" form, then the contextual link would appear on nodes even though what you see on those nodes is not affected by the template. Is that the intention?

My guess would be we still want the "(when in use)" restriction, but we need some clarity.

re

The stylelint CI is configured to run only when CSS files change. As a result, most MR's appear to have been passing CI because they do not touch CSS, but any MR that includes CSS changes will cause the stylelint job to fail under the current CI config. The failure is therefore “hidden” until someone modifies CSS.

I created https://git.drupalcode.org/project/canvas/-/merge_requests/471 to test this out which just has 1 css change. The linting jobs passed, so I don't think the above is correct

I manually tested it and it works well, just a couple of thoughts that could be follow-ups

1. When you click "Add folder" it creates the folder but you have to click into it to rename it. Could we move focus to the input automatically instead of having the user click?
2. When you click "Add folder", because of 1), it might appear that the folder is already created and renaming is optional, but it doesn't actually save until you click the input.

   I tested on my Mac; if I create a new folder in the Finder, it creates the folder and then focuses on the name input ready for me to type. But if I do not do any other action, the folder is still already created with the title "untitled folder".

Thanks all!

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

Working on this

Setting to Needs Review for someone to double check my assumptions

tedbow → created an issue.

Re #27 I test it some more I can't get the contextual links to work when the Canvas ContentTemplate is being used. Think it because of the reason found in 🐛 Contextual links are not displayed for nodes using Canvas ContentTemplates Active and when I bring in the 1 commit from that MR it I do see the 'Edit Content Template' contextual link so I think we will need that first unless we want to do it here

Sorry, I haven't review this issue but chatted with @vishalkhode and he told me contextual links were not working at all so I created 🐛 Contextual links are not displayed for nodes using Canvas ContentTemplates Active . But it seems like this issue solves it a different way. Sorry if it is duplicate work

FWIW I can't get the context links to work using this MR but it seems like it is working for others.

tedbow → created an issue.

This would be much easier to review if the Proposed resolution and User interface changes sections of the summary were filled out

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

Pushed up a start. I have not self-reviewed yet

Looks good, thanks @vishalkhode

@wimleers re #5 looked into

$inputs->getPropSourcesUsingExpressionClass(ReferenceFieldTypePropExpression::class)

I found a couple bugs see #3566720: needsIntermediateDependenciesComponentUpdate() broken due to !empty() misuse on generators →

tedbow → created an issue.

@mayur-sose thanks for the summary update. I was able to reproduce it now. I did update the summary with a couple of key details, most importantly that the field you are linking to has to have no value for the image. Let me know if I am correct that this is needed for you to reproduce the error.

Ok, this can be fixed in the JS code
import ResponsiveImage from 'next-image-standalone';

const Image = ({ image }) => {
  if (!image) return null;
  const { src, alt, width, height } = image;
  return (
    <ResponsiveImage { ...{ src, alt, width, height } } className="my-8 max-w-full"/>
  );
};
export default Image

Added the new if (!image) return null;. Because this is a non-required field, it might be empty.

I think the problem here is that an empty value for a non-required image prop acts differently based on whether the prop is linked to an entity field or not.

 
• If the prop is not linked, it will get the value of the placeholder image.

 

• If the prop is linked, it will not get the value of the placeholder image.

This is the same as with SDC components; it just doesn't error. With the above change to the component JS, it would also not error.

I think this is a product decision on whether it is intentional that these 2 scenarios act differently.

I can see how the current behavior could be a problem.

 
1. You create a code component with an optional image prop
2. The code does not handle the case where the optional image prop does not exist
3. The component works fine in the preview; you can't actually test the case where the image prop is empty because the preview always uses the placeholder
4. The component works fine when used in Pages whether you add an image or not, no errors, and uses the placeholder when empty
5. You start to use the component in templates
6. First, you don't link the image prop, and sometimes leave it empty (imagine a component with many props so the image is just 1, which makes sense to use a placeholder)
7. Then you link the prop to the image field
8. Most of your articles have images, so you happen to only use those for previews
9. You don't see any errors,
10. As soon as you use the preview with an article without an image, you will start to get an error

The last step may only happen once your template is in use.

I am not sure of the best solution here.

The easiest solution is if the developer is simply defensive and handles the case where the optional property is empty. But there are 2 problems with this:

 
1. There is no way for the developer to test this case in the editor
2. When not linked to a field, an empty value will be replaced with a placeholder. Therefore, the developer is fine NOT being defensive. It seems reasonable that the developer would assume this same behavior for a prop linked to a field where the field has no value.

   If a non-required image field is almost always filled, it would be hard to find out that the empty behavior for the linked field is different.

@narendrar I got the sort working tags component. It had to factor "weights".

I tried to simplify things. Please review the changes. There is at least 1 comment I didn't get to also

Thanks

@wim leers see my MR question about the need for follow-up https://git.drupalcode.org/project/canvas/-/merge_requests/442#note_647978

npm install && npm run build fails on

src/components/form/formUtils.test.ts:145:5 - error TS2322: Type '{ library: "elements"; version: string; source: string; css: string; js_footer: string; js_header: string; default_markup: string; transforms: {}; name: string; id: string; metadata: { slots: {}; }; propSources: { ...; }; }' is not assignable to type 'CanvasComponent'.
Property 'broken' is missing in type '{ library: "elements"; version: string; source: string; css: string; js_footer: string; js_header: string; default_markup: string; transforms: {}; name: string; id: string; metadata: { slots: {}; }; propSources: { ...; }; }' but required in type 'PropSourceComponent'.

145 'sdc.sdc_test_all_props.all-props': {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/types/Component.ts:41:3
41 broken: boolean;
~~~~~~
'broken' is declared here.

Found 1 error in src/components/form/formUtils.test.ts:145

Unassigning because I think others could look at that

#40, I have updated the summary. This issue will not require Workspaces but does acknowledge that we will use workspaces eventually and therefore how Pages are determined to be "draft" will change. This also affects how "archived" pages are determinedbecause currently there is no field explicitly tell the difference between draft and archived both unpublished.

Content Moderation will not be used, per Lauriii

Re the change recorded I am not sure we need one, https://git.drupalcode.org/project/canvas/-/merge_requests/346#note_647461. Or if we do maybe it should just simply state you can now archive pages.

remaining questions have been answered

added steps to test

Basically this because we use ShareTempStore and set the expiration to 30 days.

I have started a Merge request that switches to a regular key value store. It need more changes because we used the "owner" and "update" functionality from the shared temp store. I am not sure if losing the "lock" functionality will have side effects

I created #3565219: Setting homepage doesn't take into account draft and delete pages → for problems I found in 1.x while testing the current issue

@NarendraR can you look that issue and

1. Double check and make sure I am actually right about the current bug in 1.x
2. If am correct, are these problems actually solved, by this issue? One or both of them?
3. If this issue does fix it do we have tests to make sure ?

Thanks

tedbow → created an issue.

Things look good. @NarendraR said he was going address a couple extra things.

It would be good if someone else could review the JS

tedbow → changed the visibility of the branch 3554205-any-auto-saved-changes to hidden.

This is a general problem.

From the video this content templates. This also should had be in the summary. Probably there needs to be even more info. But since I can't reproduce the bug I can't be the one to add the relevant info.

The more info/context the better. A video is great but the info in summary is also needed.

@mayur-sose thanks for the video. Looking back at the error "/canvas/api/v0/auto-saves/js/js_component/image" `js_component` tells me it is JS component. I have added that to the summary.

Canvas does not come with any prebuilt JS component. So I assume this one you created.

Are you using vanilla Canvas 1.x-dev?

Please provide as much information as you can.

@narendrar getting closer!

I left a review and pushed up a couple commits

re the change record https://www.drupal.org/node/3563404 →

it should not mention Content Moderation. The plan is to use Workspaces. But I am not sure exactly what we should say in the change record or if we need one. It is being added as deprecated. Right now there is not an alternative. People will have to remake their Views if they use these plugins

I am the middle of reviewing this but found this confusing UX, I also agree with #31 that is also confusing

1. 
   "published page" is page that is page that is currently published but I clicked "Archive page"
   "A page" is a currently archived page that has no changes

   Both of these having "Archived" label is confusing

2. similarly in the "Review changes"
   
   The page that will be archived says "Archived". If didn't already know who this works I would probably
   assume this is currently archived page that is going to have changes made it. I think here it should be changed to "Archive"

I am not able to reproduce this error. See #2, we need more concrete information

I do get errors displayed but no JS console errors.
Testing a required field that has no value in preview node because it was made required after node created:

1. required image: tests/modules/canvas_test_sdc/components/image/image.component.yml
   get this error
   

   Twig\Error\RuntimeError occurred during rendering of component 3df8a137-e336-45bf-93b4-de95f5e1e465 in Content template Article content items — Full content view (-): An exception has been thrown during the rendering of a template ("[canvas_test_sdc:image/image.src] NULL value found, but a string is required. This may be because the property is empty instead of having data present. If possible fix the source data, use the |default() twig filter, or update the schema to allow multiple types..
   [canvas_test_sdc:image/image.alt] NULL value found, but a string is required. This may be because the property is empty instead of having data present. If possible fix the source data, use the |default() twig filter, or update the schema to allow multiple types..
   [canvas_test_sdc:image/image.width] NULL value found, but an integer is required.
   [canvas_test_sdc:image/image.height] NULL value found, but an integer is required.") in "canvas_test_sdc:image" at line 1.

2. optional image: tests/modules/canvas_test_sdc/components/image-optional-with-example-and-additional-prop/image-optional-with-example-and-additional-prop.component.yml
   no error

Testing a required field that has a value, both cases have no error

Does this problem happen with components that have an optional image prop?

Or does it only happen with required image props?

Required image props are only supposed to match required image fields but if you add a required image field after an article has already been created it might not have an image for the required prop. Or if you make an existing field required after you already created an article without the image field value.

Also which exact component cause this problem? There are various types of image props. It would be good know an exact SDC under tests/modules/canvas_test_sdc/components that causes this problem.

for example tests/modules/canvas_test_sdc/components/card-with-local-image/card-with-local-image.component.yml and tests/modules/canvas_test_sdc/components/image-optional-without-example/image-optional-without-example.component.yml both have props that will result in images being displayed but are defined differently

@lauriii ok thanks for the clarification

@NarendraR that means we are back to MR 346 with the View integration

Is this with pages that are already published or a page that was created 30 days ago but are still in “draft” state?

@lauriii thanks for the clarification.

re

The workflow should allow going directly from Draft to Archived.

The current approach in 3556265-create-ability-to will not work in this case.
Since "Status" is boolean field it cannot record 3 different states, Draft, Published, Archive.

The current MR was getting around this by relying on

1. Published: status === 1
2. Draft: status === 0 AND no published revision exists
3. Archive: status === 0 and at least 1 published revision exits

\Drupal\canvas\AutoSave\AutoSaveManager::entityIsDraft and the views integration depend on this logic

If you can go from draft -> archive then in that case the above won't work because there can be Archived pages without having a published revision.

I am not saying we can't do it, just we have use a different approach.

In !409 I added a publication_state field which can store 3 states but I closed that MR because in #14 you said

That would be ideal but we should not implement this at the cost of making the future Workspaces integration significantly more complicated.

I thinking adding a publication_state field that we would then need to remove because after "publication state" would be managed by Workspaces would make the migration to Workspaces more complicated.

Probably the migration to Workspaces to will be complicated anyways but if we don't add any new field in this issue we won't be making it worse than 1.x

But now I don't see how we get around adding the publication_state field to Page or some sort of storage. Below I will outline a plan for that but before that I think we need to consider how this would affect editing individual nodes(can't find an issue but 99% certain is on the roadmap).

Would nodes also need the same publication state workflow as pages? I assume yes, but should get confirmation as this would probably affect how we handle Pages now. Nodes will have the same problem of having a boolean status but needing store 3 publication states.

We could use hook_entity_base_field_info_alter to add the same publication_state field to Nodes that we do for Pages. But if we are going to do that we should probably use canvas_publication_state for both so don't have conflicts on node with other modules also altering the base fields(could happen on pages too?)

The other option would be to add a canvas_publication_state content entity type but that seems like we would be starting to reimplement the Workflows\Content Moderation modules

Transition to Workspaces

Assumptions for what Workspaces integration would mean

1. Workspaces would be a hard requirement. You couldn't choose not to use Workspaces and instead use Canvas and the solution for draft/archive we come up with here. This is just a stop-gap
2. There will be at least 1 "draft" workspace
3. All pages/nodes in "draft" will be in this workspace(s?), not in the "live" site
4. Archive pages/nodes will be in the "live" sites as unpublished, not in an "archived" workspace

If those assumption are correct then in an update hook when we require Workspaces we could

1. Create a default "draft" workspace(maybe workspaces does this automatically?). We need to ensure a workspace is created if the site is already using workspaces(at least if there >0 drafts)
2. Move all draft pages/nodes, as specified by the canvas_publication_state into the default workspace
3. Remove all "draft" Pages/Node from the live site
4. Remove the canvas_publication_state from both Page and Node. We should mark canvas_publication_state and any isDraft(), isArchived etc functions as internal/deprecated when we add it here, so we are free to do this.

Even if we didn't do the current issue I would assume we would still have to do 2 & 3 we would just determine draft by status === 0 && count(published revisions) === 0

So major complication would be adding canvas_publication_state to both Pages and Node(assuming editing of nodes happens before Workspaces integration), then removing it.

So this bring back to @lauriii's comments(in Italics)

in response to

This issue assumes we need to distinguish between archived and unpublished pages. Currently they both have status = 0, only difference is draft pages have never been published

That would be ideal but we should not implement this at the cost of making the future Workspaces integration significantly more complicated.

and

We are planning to work on Workspaces support in 2026 so we should avoid making the migration to Workspaces significantly more complicated than it's today.

I do think this would make "migration to Workspaces significantly more complicated than it's today.". What I detailed is not that complicated but I think there could definitely be "known unknowns" unless we are going to plan in detail now our migration to workspaces, and even then not sure, a plan is just a plan.

So do we not do this issue at all or do abandon

The workflow should allow going directly from Draft to Archived. There are valid use cases where a user might want to archive a draft page without ever publishing it first (e.g., a page that was started but is no longer needed, or content that was created speculatively and then abandoned). Forcing users to publish first just to archive seems unnecessarily restrictive.

I do understand this a valid use case but is worth making the migration to Workspaces more complicated? question for @laurii

I think the other options for

a page that was started but is no longer needed, or content that was created speculatively and then abandoned)

is just deleting the draft or choosing not to publish and leaving it in draft incase it is needed later. Maybe not ideal but maybe an ok comprise

I added extra questions to "Remaining questions"

This could be worked on now correct?

🐛 Editing support for type: array Active stops the editing but we could still write back tests to confirm whatever approach we decide on works

I pushed another spike branch, 3561765-spike-multi-checkbox. I misunderstood how this suppose to work. The user would not select "array" as the prop type. There will be checkbox "Allow multiple values". This makes more sense

This MR only works numbers and strings but should be possible with other types

just fixed link to issue as it has move to the Canvas project from XB

I am not sure how much work it will be to get the rest of this done or what the priority of it is compared to other issues. If we think it won't get fixed in the near term should we warn users.

Options:

1. Should we actually not allow components with array props \Drupal\canvas\ComponentMetadataRequirementsChecker::check. I guess since we are passed 1.0.0 probably not
2. Should we warnin GeneratedFieldExplicitInputUxComponentSourceBase::buildComponentInstanceForm() if we find an array prop. Or do some array props wok for editing?

re #14 I tried the gallery again.

It seems to work the first time I try to add images. but if I add another it lose the values. They do not show in the preview and then if I reload the page the values are gone. If I just add attachments and reload the page the values do stay

If I try to re-order it does not work but when I reload the page the values do stick, though not re-orded

adding some known issues

tedbow → created an issue.

@lauriii

Are there drawbacks with the Views solution that we should be aware of because you're generically claiming that it might be better to go with the alternative approach?

I just generally if we weren't planning to switch Workspaces that it would easier to keep the logic of the Page publiciation state internal to the Page entity rather than relying on special views integration and \Drupal\canvas\AutoSave\AutoSaveManager::entityIsDraft. It would be more understandable and maintainable.

But since we are planning on moving to Workspaces and since we are planning to add Node edits, which also has the problem of only have boolean status, determine 3 possible publication states(if we want display "archived" next to nodes), then rely on special logic in \Drupal\canvas\AutoSave\AutoSaveManager::entityIsDraft makes sense.

So @narendrar I have close the other merge requests and brought in the Views integration into the original MR.

The views integration should solve this problem from #8

Pages listing (admin/content/pages) indicates if a particular page has been archived ❌

We will not need to add any extra fields the Page entity and keep the approach of the original MR

I think this will break some tests. `\Drupal\canvas\AutoSave\AutoSaveManager::saveEntity` tries to compare the auto-save version to the saved version to determine if we actually need an auto-save. If the only change in the auto-save is to the title and we also update the saved entity title then the auto-save might be identical to the saved version, therefore this would not show in "Review X changes". not sure how we would solve this

tedbow → created an issue.

1. changing the title to reflect "archive". If we just had to unpublish and did have to distinguish between pages archived page, pages that had been published in the past, and draft, pages that have never been published this would be easier.
2. I added a "Remaining questions" section to the summary to ask if we want to avoid adding a field, mostly because we might use Workspaces eventually. I did some of the

How pushed up another MR https://git.drupalcode.org/project/canvas/-/merge_requests/411

You may have guessed that AI made this summary, and made the code with me helping, but I think this good idea if we still plan on moving to Workspaces and don't want to add a field we will have to get rid of

We could also add the view integration in a follow-up. but good to know it is possible without too much code

Temporary Views-Only Publication State Field

This implementation adds Draft/Published/Archived differentiation in Views without storing data in the database. This approach was chosen
specifically because Canvas will eventually integrate with the Workspaces module, which provides its own content staging and workflow capabilities.

Why Not Store the Field?

Adding a stored publication_state field would create significant backward compatibility issues:

1. Data migration complexity - Would need update hooks to migrate stored values when switching to Workspaces
2. Conflicting systems - Having both publication_state and Workspace states would be confusing
3. Hard to deprecate - Can't easily remove a stored field without breaking existing sites
4. Technical debt - Would require maintaining two systems during transition

How This Works

The implementation uses SQL CASE expressions with EXISTS subqueries to compute state dynamically:

CASE
    WHEN status = 1 THEN 'published'
    WHEN status = 0 AND EXISTS(published revision) THEN 'archived'
    ELSE 'draft'
  END
  

This provides full Views functionality (display, filter, sort) without touching the entity schema.

Removal Path

When Workspaces support is added:

1. Delete the 4 Views plugin files (already marked @deprecated)
2. Update default Views configs to use Workspace fields
3. Document upgrade path for custom Views

No data migration, no stored field cleanup, no complex update hooks.

Files

• src/Hook/CanvasViewsHooks.php - Registers field/filter/sort with Views
• src/Plugin/views/field/PublicationState.php - Display handler
• src/Plugin/views/filter/PublicationStateFilter.php - Filter handler
• src/Plugin/views/sort/PublicationStateSort.php - Sort handler

All files are marked as deprecated and reference the future Workspaces integration.

@narendrar thanks update in #8, thats very helpful!

admin/content/pages is a view and it is not differentiating between draft/archived as both are unpublished state.

I think this is key problem. @lauriii should confirm but it can't imagine it would acceptable to have admin/content/pages not be able to distinguish betwen draft and archived. It would be very confusing to when you unpublished a bunch of pages they would show at the top of this listing as "unpublished"

I considered these options to solve this problem

Options Considered for Draft vs Archived Differentiation

Problem: Views can't distinguish between draft (never published) and archived (previously published) pages since both have
status=0.

Approaches Considered:

1. Computed Field (No Storage)
   • Calculate draft/archived state on-the-fly by checking revision history
   • ❌ Rejected: Views can't query computed fields without storage; performance concerns with N+1 queries
2. Revision History Queries Only
   • Keep current status field, query revision history in Views/reports as needed
   • ❌ Rejected: Complex to implement in Views; poor performance for bulk operations
3. Content Moderation Module
   • Use Drupal's built-in workflow system with draft/published/archived states
   • ❌ Not chosen: Adds significant complexity; overkill for simple publish/unpublish workflow
4. Stored publication_state Field with Auto-Sync ✅ Chosen
   • Add a stored field (draft/published/archived) that updates automatically when status changes
   • Uses onChange() hook to keep field synchronized with publication status
   • For unpublished pages, checks database revision history to determine draft vs archived

Why Chosen:

• ✅ Views-compatible (can filter, sort, display the field)
• ✅ Performance (no repeated queries, indexed field)
• ✅ Immediate consistency (updates on status change)
• ✅ Correctness (always checks actual database state, not in-memory values)
• ✅ Self-healing (automatically corrects if manually tampered with)

I pushed up new PR to explore the chosen https://git.drupalcode.org/project/canvas/-/merge_requests/409

It solves this only for Page but right now this all we support.

I think the problem with this approach is I think there is plan to some day use Workspaces. and we might not need this field any more.

I believe this requirement comes from lauriii

re 3

From my discussion from him, he is okay with the side effect that if the 10 most recent nodes of type are all unpublish you would not be able to see *any* published ones

I guess seems ok vs making the code more complicated.

@narendrar re #4 so it is the actual order now.

There is no information about how this work or the expected behavior which should be in the issue summary.

Also it needs tests

🎉 Thanks!

See my MR comment I think this need to be tested

This look ok but this file is very confusing 📌 Document why PropsSchema.json works that way it does or change it Active . Lets fix this

tedbow → created an issue.

tedbow → created an issue.

Switching the project because I was searching for this issue and couldn't find it.

Looks good to me. Tested and it does cancel the requests

I have set to auto-merge if the test pass. otherwise I will look at tomorrow

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

See MR comments as to what is left. mostly https://git.drupalcode.org/project/canvas/-/merge_requests/18#note_634116

I will continue reviewing this tomorrow but for now needs work for https://git.drupalcode.org/project/canvas/-/merge_requests/18#note_633128 and other comments.

I manually test and it works but needs work mostly for this https://git.drupalcode.org/project/canvas/-/merge_requests/354#note_633399

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

Changes look good to me.

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

I was a bit bit confused about this issue since it references "page templates" I thought this was going to be somehow related to "Content Templates"

re the summary

A prompt like “Create a template for the homepage of an ice cream shop website, with proper header and footer”

Since we have 1 thing canvas UI called "template", which are Content Templates, I was expecting this build that. I am wonder if the user would expect that? Especially if they were familiar with Canvas. What prompt would the user use if added an agent to make content templates, which though would also know as just "template"?

Did this break the php lint job in 1.x ? https://git.drupalcode.org/project/canvas/-/pipelines/675734

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

Haven't review but "Needs work" at least for test failures

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