A prop may not be of type "object" unless it has a $ref defined.
There's two related issues already for this:
- 🐛 References must not be required to guess props' JSON schema Active
- 📌 [PP-1] Support `{type: object, …}` prop shapes that require *multiple* field types Postponed
Required props must have examples and/or default
This would be resolved by 🐛 DX & authoring experience: for SDC+code components, the example is treated as the default, may not be desirable Active
Canvas always requires schema, even for theme components.
This one is still remaining without an issue. Let's repurpose this issue for this?
I don't have strong opinions on what to do about 1 & 3. Isn't 1. just a feature that isn't currently supported by Canvas? And I have no idea why we have the restriction for 3.
These changes seem like ones that could be done in a minor release. With the descoping of 2. from here, I'm not sure why this needs to block a stable release.
Per @pameeela, this should be a Drupal CMS release target.
Also a general question - wouldn't this also be useful information on the manage form display page? e.g. it would make it easier to group required fields together. Doesn't need to block but could maybe be decided here and then a follow-up to implement.
I think it's a great idea to expand similar functionality to the manage form display page. I think we can work on it separately from this.
Not sure what this means for 'Single' but to be honest I think saying '1 value' might be more self documenting than 'Single'. And wonder whether 'unlimited' should be 'unlimited values'?
I agree that unlimited value / single value would be more self documenting. I went with 'single to make scanning for those easier. I tested adding 'value' to the end but it makes the whole UI really wordy because these pills get repeated multiple time on the page.
FYI, I worked on this with @ckrina on Slack: https://drupal.slack.com/archives/C1AFW2ZPD/p1760259963507569
Wouldn't the benefit of a computed URL field be that the URL would be available in JSON:API response too? It's currently pretty difficult to get the URL for a given entity from JSON:API. IMO it would make sense to provide it as an API there too.
lauriii → created an issue. See original summary → .
I've been testing the AI and it's still generating props that use Tailwind classes:
I've confirmed from config that this fix was in place.
I've responded to both questions linked in #17 🐛 Deleting or renaming SDC prop will break xb pages using old version of the component Active . Let me know if anything else is needed from me 😊
This has been completed from my perspective – the image component looks good. There might be a follow-up discussion to be had on best practices regarding the use of attributes object.
What's the use case for this? Why are you navigating to the actual page?
The main requirement that is important from my perspective is that past revisions with component usage should not prevent deletion. This is too restrictive because you may have to prune hundreds or thousands of past revisions when phasing out usage of a component. This is something that can be done in code but doing it manually is not reasonable.
It's also protecting against something that I believe users don't care as much about. We have graceful handling for the use case where a component has gone missing, which we need anyway because we can't enforce dependencies for SDCs—since a theme author could simply decide to delete one of the components. I also don't think it should be on the theme author to provide an upgrade path for these use cases, since how you'd want to handle a deleted component depends entirely on the specific use case.
Because of this, we should prevent deletion with these uses:
- Components used in default/current revisions (content users actively see)
- Components used in forward/draft revisions (content editors are actively working on)
- Components used in auto-saves (content editors are actively working on)/li>
However, forward revisions is just not a priority at the moment since Canvas doesn't fully support forward revisions due to these being handled by auto-saves.
lauriii → created an issue. See original summary → .
It seems that this is mostly just documenting the reality. It doesn't seem like there's a massive opportunity in adding additional support for Windows so I think this is fine as is.
Realized 11.2.0 shipper already and it wasn’t a dev branch. I don’t think this qualifies for a backport so keeping 11.x only.
I've merged the MR to 11.x. We need a separate MR for 11.2.x because the MR doesn't apply cleanly on it.
FYI, I provided some direction in #3548322-13: [needs design/product] Follow-up for #3541037: Improve how list of field suggestions is displayed in the UI for `ContentTemplates` → for the UX.
This is a major bug which would be good to fix soon but it doesn't have to block the RC. Based on discussion with the team, this is still relevant.
FYI, float has been removed from UI 📌 Move Float fields to contrib Active which leaves decimals as the primary way to store fractions with Drupal. I think supporting two decimals will cover most of the use cases for UI building.
Thank you! Indeed it would be great to support being able to render the 'human readable' value of list fields through a text fields. This allows decoupling components from specific list fields in the content model when it's not needed.
I think we should do something along the lines of this (which is really close to what @hooroomoo had posted on 📌 Refine API response with DynamicPropSource suggestions to provide better UX Active ):
- We should sort the fields based on default form display order (when there is a form display to reference)
- Fields not on the form display should be displayed below the ones that are on form display in alphabetical order
- In the scenario of file entity in images, we should simplify the structure and bring the file entity properties to the second level
- We should capitalize properties
- We should not display 'only' as a prefix
.
I don't think we'd necessarily need the block in Canvas but I think I think it's fine to keep the block in Canvas given it's already in the MR so long as it doesn't show up as an option in Canvas.
Please ignore #26 🐛 ComponentInstanceForm displays default value in the component form when value is empty. Active . The problem was not caused by this MR and was specific to my component. Sorry for the noise!
We should make sure to open the follow-up for #25. ✨ Allow linking a component prop of a template to a dynamic field Active . I think it's fine to merge this as is but we should prioritize that follow-up next.
I don't think this is working as expected. Testing latest HEAD, now default value shows up in the preview but not on the right panel form when textfield is made empty:
I can still reproduce #50 when testing the MR from here.
Updated the proposed text (added to issue summary) based on findings from @thoward216.
I think it is duplicate! I didn't find it because it's using the term default value instead of example value 🙈
I tested to generate a pricing table. For some reason the features are a single field so it doesn't render nicely 😅 The styles look pretty close but there are some differences. For example, the headings are using 3xl size but we're actually using 4xl for other sections. I'm not sure why is it not using the heading component directly so that it would be consistent with other section components.
I've tested this first with the prompt "Can you please create a CTA component that allows me to use images with a bit of text and to include a CTA to link to other pages?" which design wise gave me surprisingly good results! However, I ran into 🐛 AI should not generate props using Tailwind classes Active .
I updated the prompt to "Can you please create a CTA component that allows me to use images with a bit of text and to include a CTA to link to other pages? Please do not use tailwind classes as props and instead provide pre-defined color options" and for some reason I started getting results that are not really consistent with the site and don't look as good. I tried this 5 times and I got pretty much the same result every time. I even tried deleting the old CTA components to make sure it didn't use those as context.
Here's a screenshot where you can see the first CTA which looks good, and then the two following it which is essentially what I got with the updated prompt:
Is it possible that the agent stopped considering the style guide after I instructed it about the color options?
I don't see why #11 couldn't be implemented in a nested tree of dropdowns? It is just that in the case of a reference field that doesn't allow additional properties, you'd have to go through another level of nesting which seems fine as the first iteration of this – it's still much better than showing them all as a single list. We can potentially apply some optimizations on that in future.
I update the MR to not render disabled components in the incompatible tab.
I saw a video from @narendrar on the current progress. Great to see this moving forward! Some feedback on the progress so far:
- The progress updates are too technical and too focused around the internal details of how the system works. We need to translate these to user centric language. For example, "main orchestrator agent started" should say "Creating a plan" and "Title generation specialist agent started" should say "Generate a title"
- Some of the steps seem unnecessary. Why would we show something like "Message from orchestrator to x agent"?
- Could we show the prompts generated by an agent below the relevant step instead of just showing it on the top?
@thoward216 What is displayed in the right hand panel when you select the deleted component?
It looks like the CLI tool is not usable at all when using first party imports because even download is not including importedJsComponents properly. We should update the issue summary to reflect that.
Let's actually simplify the hover text to:
Can't delete components used on published pages
The number of uses isn't as important in this context vs finding the specific uses so let's prioritize getting that done soon.
I'll help with the dialog text once we've tested the behavior 👍
It won't do that. We can't automatically update all past revisions. (There could be a million such revisions, they might have slots that contain their own component trees, etc.)
How does the deleted component appear if someone reverted one of these revisions?
Are you now saying that it should only provide a count, and not list the specific ones?
I think we should provide a separate interface to list the specific instances which would also allow the user to easily navigate to those instances. We can assume that's out of scope for this issue.
This does not make sense: the ability to delete a code component will simply not be offered in this case.
The UI should still explain why it cannot be deleted. This could done for example by disabling the button and showing the proposed text on hover.
I'd recommend the following text in the dialog:
You are about to permanently delete the [Component Name] component.
This will remove the component from [X] past versions. Reverting to past versions that rely on this component will appear broken.
We need to add the forward revisions exception in future once that becomes supported.
The text when the user cannot delete a component because it's in use could be:
You cannot delete the [Component Name] because it's being used on [X] published pages.
I don't think we have to prioritize the soft deletion for now. I'd like to hear from user feedback if that's something they'd need. We should prioritize first being able to easily find all the instances of a component within Canvas UI. This would require updating the API endpoint to only list usages within current revisions. This can happen in a follow-up and we can prioritize it some time soon but let's land this data integrity issue first.
Have we tested what happens if user deletes a component and then reverts to a revision with that component? Just wondering if we need to do something to explicitly handle this scenario.
I'll comment regarding the approach to UX on 📌 FE follow up to Only allow deleting Code Components if there's 0 usages Active .
The difference between enabled / disabled tabs is pretty confusing too because disabled in the tab actually means that the component is not compatible with Canvas. I tried to focus this on the most obvious small thing to fix but I agree that there would be opportunity for more improvements here. I'm not sure why would we split the disabled components to two tabs? It seems like it could be just two lists in the disabled tab where one list is for those that are eligible to be enabled and one list for those that aren't.
This is not a beta target but certainly something that could be merged if its ready.
You would run into the problem if you provided slogan as an optional property of the page which passes it to the card from the page. This would not trigger validation errors when slogan is provided but would trigger the validation error when page is not receiving slogan.
I originally planned to build a new widget for making it possible to customize which meta tags were shown on the edit form, but realized it was pointless when you could just add regular fields and then use tokens to output the tags as necessary.
The point is not necessarily to configure which fields are available but to allow user to choose which metatags they want to provide. This is relevant specifically for pages because they contain unstructured data and the different metatags needed might vary page by page.
Other benefit is that metatag is also specific to the actual HTML that get printed vs fields which would be an abstraction away from the HTML tags. This would make it challenging to implement preview for metatags like we have here:
Also, what would happen if user provided a prompt "Make the header sticky so it stays at the top on scroll"? This seems like a common use case which we should think through. Usually you'd probably have some kind of header component which the AI probably should modify in this scenario. Alternatively if the header is an SDC, it could create a new component that wraps the existing component to add the sticky header behavior.
Have we considered refinements to the header and footer here at all? For example, "I'd like to add a 'Start free trial" button in the header that links to #" or "I'd like to add a 'Contact' link that links to '/contact' to the header".
This is inline with what we have discussed before. 👍
This isn't really all that relevant when using SDC in render array but when using SDCs within Twig. Twig doesn't have the concept of undefined unless you use Twig strict_variables turned on (which Drupal is not doing). So when you are passing in undefined properties, they become null. I don't think there's a practical way to keep the variables as undefined and really as a frontend developer, you shouldn't have to care.
Merged the follow-up! I also opened 📌 Add docs for image code component Active .
Merged! Thank you @heyyo for pushing this forward! 👏 We can still iterate on this so feel free to open follow-up issues with improvements!
Could we please split this to two separate issues? I'm not sure that adapted prop sources is what's needed here. You should be able to map date field to a format: date string. If that's not the case, that's a bug on it's own.
There's a small bug with this. After you delete a page, you are taken to a 404 page:
Yep, admin theme should be a composer dependency and not part of the site template. I think a theme directory for the site template theme makes sense conceptually. It could be either a self-contained theme or a theme depending on another theme via composer dependency depending on the site template and its needs.