ci failing
Closing this as duplicate in favor of https://www.drupal.org/project/experience_builder/issues/352815 → that has an MR and more context on it even though this was created earlier
adding designer credit
Marking as stable blocker to remove the band-aid that currently covers up a bug.
hooroomoo → made their first commit to this issue’s fork.
Opened an MR to remove the "Add to components" menu item from the contextual menu for now. Added a todo that points to 🐛 Component list for UI Library should return auto-saved code components if any Active which will replace the band-aid solution with a proper one.
The reason behind this bug is because in the "add to components" PATCH request, we are currently sending "...selectedComponent" along with status: true
. Ideally the frontend should only have to send the necessary changes, in this case status: true
. See
🐛
PATCH request for config entities reset values that are not sent
Active
.
The band-aid is that we send "...selectedComponent" in the PATCH. Without sending that as of now, I think the hovered preview, and props and slots breaks. I think this is because the component entities displayed in the component UI list don't have the most auto-saved version of a JS component.
Clicking from the contextual menu sends the JS component config entity (...selectedComponent) since thats what we use to render the list of code components. The JS component config entity doesn't include imported_js_components
(by backend design) and so the server complains because it requires imported_js_components
to be sent when its also sent compiled_js
The bug doesn't happen from clicking the top right "Add to components" button because ...selectedComponent
in this case is taken from our redux store of the code editor which stores the imported_js_components
property that the error is about.
Adding related issue. Added todos in there to point to this one.
It was changed back here: https://www.drupal.org/project/experience_builder/issues/3521819#comment... 🐛 Hovered preview for JS components in library not working Active in order to resolve that bug's issue.
But the change itself is a band-aid solution that solved the above issue, but causes the 422 error here. We will want to update the band-aid solution but it will require work. @tedbowman actually started on a proof of concept solution to address this: 🐛 Component list for UI Library should return auto-saved code components if any Active
I think in order to address this there's 2 options:
1. Continue on
🐛
Component list for UI Library should return auto-saved code components if any
Active
if that approach looks good
2. Remove the "Add to components" from the contextual menu from now, and make users click it from the top right in the code editor (which is working). And then return to #1 later, if we want to prioritize what's currently in the remaining sprints before GA
My opinion is doing #2 because it seems like most people have been adding components using the top right button in the code editor anyway.
hooroomoo → made their first commit to this issue’s fork.
hooroomoo → made their first commit to this issue’s fork.
Yup its much better thanks, reviewed and approved.
I noticed a regression on Firefox, the select element (initial unselected state) renders to the width of the longest option in the select. I noticed it with the select element from the test module, then tested it out by adding another property that was a long string in simple/heading/heading.component.yml
Also on Firefox, if an option is selected that is wider than the width of the settings panel, it gets cut off. This also happens in 0.x though so it's not a regression, but if it ends up being a simple fix, maybe it can be addressed in this issue since the MR deals with the select already? But if we want to hold off on it until the formal designs are implemented, that's fine with me too.
But I do think the regression I mentioned at the beginning is worth addressing here.
The select element is cut off in Firefox
hooroomoo → made their first commit to this issue’s fork.
hooroomoo → created an issue.
adding credit for helpful discussion/pointers
Looks good to me! but still needs a /ui/src/components/form/inputBehaviors.tsx approval
Hi @meghasharma, the todo is under the proposed resolution in the second bullet, but I can see how that could be easy to miss.
if you do a search for this issue number "3525907" in the codebase, there is a todo inside of /ui/src/features/code-editor/hooks/useSourceCode.ts
which is where the change should be made.
left some comments
🎉
hooroomoo → made their first commit to this issue’s fork.
Still WIP but here's what i have so far, using react-json-tree to render response in new tab in code editor
hooroomoo → created an issue.
I think it makes sense to do this after 🐛 Auto-save changes from code editor get lost if you navigate out too quickly Active which will hopefully get in soon.
There is a @todo in there that points to this issue to add checking for props/slots changes and block navigation until that's been auto-saved.
hooroomoo → created an issue. See original summary → .
Updated to check for the source global css and source global js changes as well
I still need to add checks for props/slots, global css changes and manually test it, will continue tomorrow
just fixed some missing punctuations and cspell. merged thanks!
Screenshot:
GIF with throttled network:
Yeah it still exists. I don't think it's a big problem though
hooroomoo → made their first commit to this issue’s fork.
YAY! Thanks!
Hm I'm not sure how you got the error for giving an existing name of a standard/dynamic component to a jS component, because the machine name/ID for a JS component gets appended with a "js" so if you named a JS component Breadcrumbs, then its id would be "js.breadcrumbs" therefore avoiding any conflicts.
It is by design for js components to not be allowed to have the same name. I am therefore marking this issue closed (works as designed).
https://www.drupal.org/project/experience_builder/issues/3520071#comment... ✨ Show human readable error messages when creating component fails Active will add the "js_component' entity with ID 'image' already exists." to render in the dialog, so the user will also see it right away now.
updated screenshot
checked with @lauriii, he's good with text being "Edit code" instead to match what's currently in the contextual menu in component library and layers
hooroomoo → made their first commit to this issue’s fork.
left some comments, looks great, looking forward to seeing the subsequent work
hooroomoo → made their first commit to this issue’s fork.
✨ Show human readable error messages when creating component fails Active displays errors for code component creation so renaming the title here
hooroomoo → created an issue.
hooroomoo → made their first commit to this issue’s fork.
Gonna address feedback
Follow-up issues referenced in the todo comments:
📌
Disallow deleting an XB-enabled content entity if it's currently the homepage
Active
📌
Allow CMS Author to set site's homepage from navigation
Postponed
✨
Move entity type and ID from base path and into routing parameters
Active
Adding related issue here, whoever picks this up should check that issue to see if anything can be re-used for the other dialogs.
409 ERROR: Existing name (backend response)
Client side validation for invalid formats
Closing this as a duplicate because I am implementing the suggestion from #2 in ✨ Show human readable error messages when creating component fails Active
I believe the fix for 🐛 JS component slots don't appear in the preview canvas until published Active should also solve this.