I made Broken components (components where component.broken === true have a ⚠️ icon, show as disabled and prevented them from being dragged or right clicked.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → changed the visibility of the branch 3540965-add-a11ytest-labels to hidden.
I've managed to find a bug - see https://git.drupalcode.org/project/canvas/-/merge_requests/139#note_597085
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
Because even though Video 1 is clicked in step 2, its form hasn't loaded yet.
I don't think that's correct. Firstly, Video two has a width of 150 (it's default width), not 190. Secondly, immediately before setting the width, the Duck video was chosen in the media browser and correctly applied to video 1 in the preview. So the correct form was displayed when the Duck video was chosen and the correct element in the preview was updated but immediately after that, the width was typed in and was not sent to the server (an empty array was sent instead).
I'm not sure there is actually much value in landing this "temporary" fix ASAP vs working though and solving the underlying problem. When looking at the Before/After gif on the MR description it took me several loops of both gifs to even see the difference. The whole form content still flickers - it's just that the white background no longer does.
I think merging this just muddies the waters and creates more overhead as we'll need another follow up anyway and that will have to unpick the work in this MR.
When an entity is deleted, shouldn't the suggestedEntityId be updated to point to a different node that does exist? Or does that happen already and this is to catch the case where the entity was deleted but the UI hasn't been refreshed yet (or if the user visits an out of date url with the id in the params?)?
jessebaker → made their first commit to this issue’s fork.
I've approved and merged everything so far. I suggest leaving this issue open until the end of our sprint as a bucket to catch any other small things that come up.
jessebaker → made their first commit to this issue’s fork.
This seems to have been caused by adding the line aria-label={`${item.name} preview thumbnail`} in ui/src/components/list/ListItem.tsx. Removing the aria-label prop resolves the issue (but will introduce a regression in the a11y check Playwright test).
It seems that putting aria-label on the Radix Tooltip <Tooltip.Content /> does some shenanigans where it renders a hidden div inside the tooltip with the label text in and this is somehow interfering with the height calculation?
Had a little play around with this. It seems like drag/drop in the UI is generating a POST to canvas/api/v0/layout-content-template/node.article.full/3 with the correct Layout/Model JSON (you can see the order has changed) but no new autosave is generated (note no changes button in the top right doesn’t update) and the preview is refreshing but the markup doesn’t reflect the change.
What is stranger still is that if I just have 3 components on the template and re-arrange the order of them that works fine, it only seems to stop working once you have a component with populated slots.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
jessebaker → created an issue.
jessebaker → created an issue.
jessebaker → created an issue.
jessebaker → made their first commit to this issue’s fork.
jessebaker → made their first commit to this issue’s fork.
I spotted a small bug in the Manage Library tab: SDCs and Block components have cursor: default on hover but JS Code components have cursor: grab;
Next steps:
Now that Drupal is no longer aware of which entity is being edited because the Route is handled by the React SPA we need another way of getting the drupalSettings.canvas.entityTypeKeys
@bnjmnm suggested instead loading the keys for all the different entityTypes (keyed by the entity type) https://git.drupalcode.org/project/canvas/-/merge_requests/4#note_581234
Before this MR it was possible to do this
const titleInput = entity_form_fields[`${canvasSettings.entityTypeKeys.label}[0][value]`];
With the change above we would be able to do
const titleInput = entity_form_fields[`${canvasSettings.entityTypeKeys[entityType].label}[0][value]`];
More context
(Probably too much context, but in case it's useful)
drupalSettings.canvas.entityTypeKeys was previously based on the current entity being edited and looked something like
{
"id": "id",
"uuid": "uuid",
"revision": "revision_id",
"label": "title",
"langcode": "langcode",
"published": "status",
"owner": "owner",
"bundle": "",
"default_langcode": "default_langcode",
"revision_translation_affected": "revision_translation_affected"
}
We would take the value of entityTypeKeys.label (in the above example "title") and that would let us know how to get page title from the entity_form_fields which look like e.g.
"entity_form_fields": {
"langcode[0][value]": "en",
"title[0][value]": "Untitled page",
"path[0][alias]": "\/untitled-page",
"path[0][langcode]": "en",
"image[media_library_selection]": "",
"description[0][value]": ""
},
Closing this. 🐛 Cannot use `h-screen` from Tailwind with XB Active landed which addressed the main problem described by this issue. The two remaining issues that I can see are not accurately described by the issue summary here and can be fixed in their respective issues
🐛
IFrame height breaks when using min-h-screen class on component
Active
🐛
100% height on body or html element breaks viewport height calculation
Active
jessebaker → changed the visibility of the branch 3537807-previous-page-search to hidden.
jessebaker → changed the visibility of the branch 3502887-prepare-for-avoiding to hidden.
Good idea! Done.
jessebaker → created an issue.
jessebaker → made their first commit to this issue’s fork.
penyaskito → credited jessebaker → .
penyaskito → credited jessebaker → .
penyaskito → credited jessebaker → .
penyaskito → credited jessebaker → .
penyaskito → credited jessebaker → .
jessebaker → created an issue.
All the work in this MR exists in and is then built upon over in ✨ Move entity type and ID from base path and into routing parameters Active so I'm closing this. Merging it will only cause that other issue to have a bunch of conflicts as it's subtly different.
balintbrews → credited jessebaker → .
Over in 📌 Add Playwright tests for XB AI module Active (in MR 1466) the test "Should verify AI panel API request and response payload" was removed in an effort to reduce flakiness.
I'm dropping this comment here to acknowledge that it probably needs to be reinstated as part of this ticket in some form. However @justafish mentioned in chat that "I've commented out the API endpoint test as I don't think this is the appropriate place to test the structure of an API response" so perhaps a different approach would make sense.
jessebaker → made their first commit to this issue’s fork.
jessebaker → created an issue.