Does this issue make it possible to link media images and videos with the image and video prop from Canvas?
I could have swore that issue existed and spent a while trying to look for it 🙈
I've defined some time back the mappings I believe we should be supporting in https://docs.google.com/spreadsheets/d/1aBasy3o7Z0OGRig2Xfd1_EInA2UsSgLO.... I've started creating issues for these:
- ✨ Allow linking lists to `type: string` props Active
- ✨ Allow linking unformatted text to formatted text props Active
- ✨ Allow linking telephone field to `type: string` and URL prop Active
- ✨ Allow linking email to `type: string` and URL props Active
Should we create a new meta issue for these or should we keep collecting them here?
Exactly, the problem is that the attributes from that list get added to the <a> element and not the <li>. There's a way to add attributes from preprocess but not from the render array that's constructing the list.
I've added couple more fields that we should consider as irrelevant.
User Status field you may want to use if you're building a content template for the user entity – so let's not get rid of that.
I don't see "this component prop needs an image URL" as a common use case – >99% of the time you would just use image upload / media library for that use case. I don't think this is something we should support in the code editor.
x-allowed-schemes: [http, https] doesn't seem sufficient because as I pointed out, there are many valid use cases for links that are using protocols other than those two.
The scope of this issue is both props and slots but it looks like the issue summary is only talking about props. Adding the 'Needs issue summary update' tag to expand the proposed solution to include slots.
never allowing prop shapes to be changed, unless there's zero instances
I think we should allow this to be done. Prop data will be either converted to the new prop type (when possible) or deleted (when conversion is not possible) when component with the changed prop type is loaded.
I don't really see a strong reason to why we'd have to provide a ton of flexibility to configure this especially in the UI for creating props. That would be relevant if you're implementing business logic related to handling the link but implementing that in your component doesn't seem like the right place. Even the current settings don't really make sense in my mind and they are confusing; I had opened before ✨ Allow mixing internal and external URLs Active . Since then we've changed the labels but I'm not sure I'd be able to explain the difference between the options still without looking into the implementation and referring to JSON Schema docs.
I think we should focus the default link prop to what's allowed by href attribute, i.e. what's defined in https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/a#href. Based on that URLs must be absolute (start with http:, https:, mailto:, sms:, or tel: and have a hostname) or site relative (/page/123) or a hash (#something).
#20.1I think with the improved labels it would have made sense to me what was being offered there. FWIW, I was testing this with code components so maybe the prop type in code components should be setting stronger restrictions for the stream wrapper URL to not be offered?
#20.2: Will add it there 👍
#20.3: I see. 👍 I don't think the current solution of displaying the URL outside of the entity is perfect but I'm not convinced it's bad enough to warrant significant rework. It's a bit strange when you see both levels there but people will probably just figure it out.
Testing this and this is definitely a really nice improvement already! A bit of feedback / questions:
-
Images seem to introduce some interesting. It looks like we could map the image to "image" but there's also URI and root-relative URL. What's the difference between these different mappings? -
This might be something for a follow-up but I don't think we should be exposing the author of the file entity in the UI at all. - Why does the URL show up on the second level, i.e. Authored by => URL, and not on the third level inside the entity, i.e. Authored by => User => URL?
Depth/level of indirection for suggested entity URLs: I think you mean only URLs for the entities referenced directly by the host entity? So, for example, assuming we're looking at the content template for some Node bundle:
- suggest URL to the author (User) of a Node (1 reference), but NOT URL to the user picture of the author (User) of the Node (2 references)
- suggest URL to the "primary topic" (Term) of a Node (1 reference), but NOT URL to the "resident expert" (User) of a "primary topic" (Term) of a Node (2 references)
Or did you really want the entity URL to be available at every one of those levels? (Which your phrasing seems to imply.)
I would expect this to inherit our existing limitations for level of depth, i.e. the link should be available for all of the entity references that are accessible there. So if user can map an image field to a user picture, they should be able to map a link field to the user pictures URL. Same is true for the resident expert for the primary topic.
If the answer to the above is "yes, every level", then what about the levels you previously asked to omit? You requested in #3547598: Refine API response with DynamicPropSource suggestions to provide better UX that we hide/omit the "intermediary Files", which also means linking to it becomes harder. Or did you want for a media entity both the "media item canonical URL" and "media item image file canonical URL" to be surfaced?
For images this is already working as expected. It doesn't look like mapping to media references is working yet but I would expect that it should be possible to link to the media item image file canonical URL.
Speaking of that: what do you expect the labels to become? No guidance existed for this in #3545859: Add a `host-entity-url` prop source for linking to the host entity (because it landed with zero product requirements or design), so I did the simplest possible thing that would clearly need to be improved later: I labeled it Canonical absolute URL — because it's factually true, and because it'd guarantee that we'd improve it later. But if many such suggestions would start appearing, then it'd become a problem, doubly so if to the prior point you'd answer that you expect both suggestions (which would appear as siblings) to get a unique label.
Could we just call it URL and Slug? They should not appear as siblings; they should appear under a child menu. For example, Image => URL or Authored by => URL.
Let's just omit revision_log_message. The two other examples have valid use cases i.e. displaying the author who had edited the node and when it was edited. I checked again and didn't see any other values we would have to omit.
We will be adding the option to update component instances in 📌 [later phase] When the field type for a PropShape changes, the Content Creator must be able to upgrade Postponed which we are prioritizing next after this. It seems that it will solve this issue. Given that we will be working on that soon, I don't think we have to figure out UX & implement a stopgap for this here. This certainly isn't perfect but to me it seems that this is a step forward.
Describe the props/slots for developers
I believe this is best achieved by using inline comments, which is supported in YAML. This wouldn't make it easy to display the developer focused comments in the UI but that seems like an acceptable trade-off.
Describe the props/slots for LLM for better usage in Canvas AI
There's still some work to define what exactly is needed by LLMs to effectively use components. The current thinking is that there needs to be more context than just description of props. While we could use the descriptions for this, the need to have more context is suggesting that will require something new regardless. See this for an example of what's being explored in the XB Metadata module: https://github.com/salsadigitalauorg/xb_metadata/blob/develop/web/themes.... Because of that, it seems safe to assume this will live elsewhere.
We had a client who wanted to be able to edit all fields/content type description, in case of a misuse, so those descriptions became content with a module we built.
That seems like a valid use case but doesn't seem like something we must provide as part of the core functionality. The goal is to allow the component.yml file include all of the necessary pieces for the component to be used in Drupal Canvas, making the components portable between projects. Editorial help texts is an important part of it. Maybe in future there could be a module that allows overriding this information, similar to what the Canvas AI module is doing.
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 👍