I think we should be able to do this purely on the frontend
hooroomoo → made their first commit to this issue’s fork.
I've seen the same assertion fail on different MRs as well as on 0.x so I think it might just be the flaky test and there was an issue filed for it 📌 Cypress test publish-validation is flaky Active .
The fail is definitely unrelated to the frontend change. But maybe @tedbow could confirm if the fail is unrelated to the backend changes
wim leers → credited hooroomoo → .
Expected because it's just updating versions but manually tested adding global css and tailwind classes in the code editor all good still in both code editor preview and layout preview.
I pulled this down and tried to reproduce the error by switching the routes consecutively but couldn't.
@balintbrews theory is very solid and the code is well commented so I think it makes sense to get this in and see if it solves the issue on 0.x since there are no exact steps to reproduce it.
I think the issue i was seeing in 16.2 had to do with my css aggregation being on or something else weird because I tested it again with css aggregation OFF, and I'm able to correctly see the autosaved component in my page preview. Also renders correctly after publishing.
I think this MR is good to go. I added the frontend changes to change the PATCH request to only send wanted changes and it works as expected.
This MR should fix both:
🐛 Adding component to component library results in component code and configuration being lost Active 🐛 Cannot add component to the component library using the contextual menu Active
So just to preface, all my testing has been with sending PATCH requests to /xb/api/config/js_component/${id}
, not the auto-save because that's what's currently in the code.
Could you confirm if client side PATCH requests to change the status or the label should still be sent to /xb/api/config/js_component/${id}
or does it need to change to send it to
/xb.api/config/auto-save/js_component/${id}?
<strong>1. WITHOUT frontend change</strong>
This issue I was seeing on this MR (with no frontend changes) turned out to also exist in 0.x. The bug (let's call this Bug #1) is
<ol>
<li>Create and add a code component to your library (using top right Add to components button in code editor)</li>
<li>Make an edit to the code from the library</li>
<li>Exit out of the code editor</li>
<li>All the styling (CSS) is gone from the component in the layout preview. </li>
</ol>
Since the bug also exists in 0.x, that means this MR is not the cause of it.
<strong>2.) WITH frontend change to PATCH request to only send necessary changes.(Currently in 0.x, PATCH requests are still sending the entire component )</strong>
<ol>
<li>Create a new code component</li>
<li>Trigger an auto-save and then click "Add to components" from the top right in the code editor</li>
<li>Add the component to your page preview. It doesn't render anything. </li>
<li>If you look in your <code>/sites/default/files/astro-island
nothing is generated from your newly created code component
This behavior exists on both 0.x and this MR, WITH the frontend change to the PATCH request of only sending wanted changes.
----------------------------------------
So my conclusion: I don't think merging in this MR will cause any regressions that don't already exist in 0.x, HOWEVER, we would need to open a second bug report for the 2nd issue I described because the frontend needs to eventually change the PATCH requests to only send wanted changes.
Correct, step 3 - Make any change to the code component's JS or CSS.
Steps to reproduce:
1. Checkout this MR https://git.drupalcode.org/project/experience_builder/-/merge_requests/906. This MR contains frontend changes to only include desired changes in the PATCH requests AND #3519634's MR from yesterday.
2. Create a new code component
3. Make any change to the JS or CSS.
4. Click "Add to components" using the top right button in the code editor (NOT from the contextual menu)
5. After Adding it to components, the compiled JS and CSS file doesn't get generated in /sites/default/files/astro-island
. When you click Add to components, the expected behavior is those files gets generated in that directory.
#12. Thanks, I applied your MR 937 from 📌 Add a route for PATCHing both a config entity and its auto-saved version together Active and it fixes the issue reported in the issue summary and it shows the current auto-saved JS and CSS when I open the code editor for the component.
But when I add the component to the layout page preview, the CSS isn't there. The CSS file for the component inside of /sites/default/astro-island never gets created.
🐛 Adding component to component library results in component code and configuration being lost Active is supposed to update the client PATCH requests to only include the wanted changes instead of also including the compiled/source_js in the request which I started on in this MR: https://git.drupalcode.org/project/experience_builder/-/merge_requests/9...
But currently doing that results in the component not working in the preview, I think due to something with the auto-save. I am hoping 📌 Add a route for PATCHing both a config entity and its auto-saved version together Active will unblock 🐛 Adding component to component library results in component code and configuration being lost Active which would then fix this issue.
Stylelint green on 0.x now
Got some feedback on UI from @lauriii
Going to make changes to UI
No side effects, it's just an interface. Selecting a component from the select just generates the import statement in a code block
Thanks @penyaskito!
Nope sorry I was mistaken, #4 🐛 Cannot delete components from library Active doesn't apply to the Remove from components action. Marking it back to Active
This issue should get fixed by 📌 Add a route for PATCHing both a config entity and its auto-saved version together Active then 🐛 Adding component to component library results in component code and configuration being lost Active . Marking it postponed for now.
#3 🐛 Cannot delete components from library Active .2
🐛 Cannot delete JS components due to component depending on them Active will address the DELETE bug
"remove from components" is a PATCH request, and currently, PATCH requests are sending the entire component's data over which instead of only the wanted changes. So the compiled and source JS in the request body triggers this error:
The 'imported_js_components' field is required when 'source_code_js' or 'compiled_js' is provided
I believe 📌 Add a route for PATCHing both a config entity and its auto-saved version together Active needs to be addressed first. Then we can update the PATCH request bodies to not include any of the JS in 🐛 Adding component to component library results in component code and configuration being lost Active .
Testing steps with example components
1. Create a new code component named 'My Button'
2. Add this code and also add a prop named 'name' of type 'text' in the component data window
const MyButton = ({name}) => {
return (
<button className="bg-blue-500 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded">
{name}
</button>
);
};
export default MyButton;
3. Create a second code component named 'My Card'
4. Add this code:
import MyButton from '@/components/my_button'
const MyCard = () => {
return (
<div className="max-w-sm rounded overflow-hidden shadow-lg bg-orange-500 p-4">
<div className="font-bold text-xl mb-2">Card Title</div>
<p className="text-gray-700 text-base">
Here is a simple description of what this card is about. Add anything you like here.
</p>
<div className="pt-4">
<MyButton name="FOOOO"/>
</div>
</div>
);
};
export default MyCard;
5. MyCard component imports the button component created in step 1 and you can pass whatever name into the name prop of the button component
hooroomoo → made their first commit to this issue’s fork.
GIF in https://www.drupal.org/project/experience_builder/issues/3518198#comment... ✨ Set code component dependencies based on import statements Active
Seeing some strange behavior after adding a code component to the library, the JS doesn't load...
#34 Nevermind! The reason I was seeing client requests (PATCH requests for Rename, and Add to component) from the code component contextual menu fail was because the request always included compiled/source_js which caused the error of "missing imported_js_components property"
But because 🐛 PATCH request for config entities reset values that are not sent Active now unblocks 🐛 Adding component to component library results in component code and configuration being lost Active , the frontend can now change the PATCH requests to only include necessary changes, therefore it won't send the compiled/source_js on Rename, Add to component so the error shouldn't happen anymore.
tedbow → credited hooroomoo → .
I grabbed a func over from @larowlan's poc
I think we should also update the docs in ui/README.md
if we want XB DDEV users to use
still have some todos left
@balintbrews I ended up with the same idea before I even saw your comments. That's validating to me haha
@effulgentsia Thanks!! Was running into a security error on my very WIP-y test commit, need to give that a try
Unless are you talking about if the frontend can get the machine names of imported JS components from its source code? Then the answer is yes.
I believe there is an expectation that the author has to write imports in the JS code editor like this:
import Whatever from "@/components/button"
import FooWhatever from "@/components/heading"
Where the import statement has to be "@/components/machineName"
@wim leers No it can't determine machine name from only the JS source code. What prompts this question?
Here I have a component I named "My Component" and the machine name the backend sets is "my_component" but the machine name is not in the source code.
left a few comments
hooroomoo → made their first commit to this issue’s fork.
I pulled in the most recent 0.x and cannot reproduce the issue anymore. Perhaps with all the front and backend changes in the last couple of days this went away?
Will leave this open for a few more days if I or someone else ends up seeing the issue, then if not will close it as "cannot reproduce"
hooroomoo → created an issue.
wim leers → credited hooroomoo → .
hooroomoo → made their first commit to this issue’s fork.
Noticed this when working on sections too.
hooroomoo → created an issue.
Yep #2 is correct, it was refetching itself after the DELETE call.
It looks like after the layer expands, it freezes when I try to drop the item in.
bnjmnm → credited hooroomoo → .
@omkar-pd Are you using chrome?
Looks like it works on Chrome after testing it, but I just tested it on Firefox (my default browser) and it looks like the preview canvas is receiving the drag event and not the layers menu so the preview is scrolling instead of the layers menu.
Marking stable blocker because it's a long and disruptive console warning
hooroomoo → created an issue. See original summary → .
A huge roadblock with replacing the div wrappers around the region with the HTML comments is that there is no way to initialize SortableJS (drag and drop functionality) for a region without a parent element aka if a twig template only prints out {{content}} or it has no root parent element.
This is because SortableJS requires a parent element to be passed in.
@jessebaker and I talked about this and he said that 📌 Investigate drag-and-drop solution that removes the need to drop items into the preview iFrame Active should unblock this so therefore marking this issue as postponed.
hooroomoo → made their first commit to this issue’s fork.
Left a suggestion