🇺🇸United States @hooroomoo

RE:#13: Cypress tests and CLI Test CI failures got fixed in 1.x.

Hi @jptaranto,

I split off the vitest-related code into its own MR, in !MR519 so I can add to it in ✨ Code Component Editor: Add a Date (and maybe also a DateTime) prop type(s) Active . Once that gets merged, could you rebase your MR!408, and I also left small comments/nits I didn't catch the first time. Thanks!

hooroomoo → created an issue.

CI is failing.

I think I know why CLI test is failing which is irrelevant to this MR. I will open an issue to fix that. But not sure why Cypress and Playwright are failing. But marking Needs work to show CI is failing.

@jptaranto Yeah I've started it

hooroomoo → changed the visibility of the branch 3568218-error-http-404 to hidden.

Thanks all! 😮‍💨

This is because when renaming from Library menu, the `selectedComponent` code component object/shape comes from /api/v0/config/component and the code to rename uses the machineName for the API PATCH call.

But the shape from /api/v0/config/component does not contain machineName. So that's why the call gets made to a 404 URL and undefined gets passed into the PATCH (https://canvas-dev.ddev.site/canvas/api/v0/config/js_component/undefined)

Closing as duplicate. Fix was merged as part of MR!475 in 🐛 Error when trying to remove code component from library RTBC

Merged MR!475 which addresses @mayur-sose's TC-05. Thanks!

I think we can close this now since it's been decided we want to use vitest for unit tests. @balintbrews confirmed this.

📌 Move cypress unit tests that can be run with vitest into vitest folder Active is issue for porting remaining Cypress tests.

Renaming this issue to port the rest of the Cypress unit tests to use vitest. I think we can just have MRs on this issue.

hooroomoo → changed the visibility of the branch 3523490-move-cypress-unit to active.

hooroomoo → changed the visibility of the branch 3523490-move-cypress-unit to hidden.

hooroomoo → changed the visibility of the branch 3523490-move-cypress-unit to hidden.

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

Looks good to me. Marking this postponed since it should be the last issue to go in as part of 🌱 Plan to allow editing props and slots for exposed code components Active .

PHP tests are not done yet as I am running into a problem but I could use another round of review to confirm this is on the right track

Not sure what documentation is expected or if issue summary update and comment #12 on this issue suffices. Removing tags

#5:

So: not using a type: string, format: uuid prop, which could then receive the host entity's UUID?

We decided to not take this approach and instead add it as a part of pageData() because

1. Can be done without needing to support UUID as a prop which isn’t supported yet.
2. Removes the need for the site builder to manually set the prop to the entity UUID.
3. Follows an existing pattern that is used to get the pageTitle and breadcrumb accessible through drupalSettings.

But because it relies on global scope, it won't actually work for other scenarios

I think the 2 scenarios you described should still be able to be done using JSON API fetch directly in the code component code. However, in the future if we find that this approach has limitations, we will revisit the UUID as prop approach.

At minimum, I think this illustrates that adding this to getPageData() is not a good idea; adding a new getEntityRouteData() or something like that would be more prudent?

I disagree, I think it makes sense to include it as part of getPageData because the entityUUID is data of the page (content entity). getPageData already provides the title of a content entity. Also I think it makes sense for a code component developer to assume that any data for a given “page”/content entity would be provided by this function. getEntityRouteData() is very Drupal-specific language and code component devs shouldn't have to know what that means.

hooroomoo → changed the visibility of the branch 3565754-provide-entityuuid-of to hidden.

hooroomoo → changed the visibility of the branch 3565754-provide-entityuuid-of to active.

hooroomoo → changed the visibility of the branch 3565754-provide-entityuuid-of to hidden.

didn't mean to change status oops

hooroomoo → created an issue.

Updated IS to add ** This should be the last issue to get merged as part of the 🌱 Plan to allow editing props and slots for exposed code components Active **

Few small things.

I also had a question, is there a consensus on what happens if someone adds a prop on a code component that has been published? The current behavior in the frontend is the new prop doesn't appear in the settings form but it shows up when you open the code editor which is confusing to users. So might need some UX changes for that but I don't think that is the job of this issue.

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

hooroomoo → created an issue.

Un-assigning myself because I have to hand this off. Updated MR description

hooroomoo → changed the visibility of the branch 3537654-automatically-detect-first-party to hidden.

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

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

🥳

Left some comments

@jessebaker do you have concrete steps to reproduce the issue on 1.x? I tried to create similar scenarios as the GIF with the components i have but i haven't been able to reproduce the issue

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

I opened 2 new issues:
#3551343: Add type-appropriate icons for LinkedFieldBox.tsx in `ContentTemplates` →
📌 Triage shape-matched DynamicPropSources: remove irrelevant ones such as any revision information Active

I am adding them to the Lower priority/need scope clarity Confirmed with product these are out of scope for 1.0 section for now... If anyone disagrees please feel free to move it. But adding them to the META so they can be tracked.

hooroomoo → created an issue.

Nice!!!

Opened 📌 Triage shape-matched DynamicPropSources: remove irrelevant ones such as any revision information Active per #27.

Removed the hard-coded Revision log message removal in the frontend in https://git.drupalcode.org/project/canvas/-/merge_requests/218#note_603115.

hooroomoo → created an issue.

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

Adding "remove feature flag" in issue title for VISIBLITY

Video

Edge case where a valid parent suggestion also has children suggestion. In this case, we render the parent suggestion in it's own submenu with a separator below it

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

adding at least 1 tag to this so this doesn't get lost in the weedz

Moved FE 📌 UI polish for `ContentTemplates` feature Active 📌 Follow-up for #3541037: Contextual panel flickers when linking prop to field Active to completed

I approved but want @jessebaker to check over my latest commit.

Yay!! And also adding 📌 Investigate updateExistingComponentValues to maybe not require component deselect / reselect. Postponed as related issue

📌 Follow-up for #3541037: Contextual panel flickers when linking prop to field Active is in! so changing status from postponed

We can close this. Can open a new issue if something else comes up.

Left a question

Tested and works great! Left some comments and a question

When deleting a component on a page, it should not redirect user outside of the page.

So this got fixed in 📌 FE follow up to Only allow deleting Code Components if there's 0 usages Active

But looks like @lauriii is asking for

When deleting a component while editing that component, it should redirect user back to the page they were editing before.

so we don't want to redirect to /editor if you were editing the code component that was deleted