🇺🇸United States @hooroomoo

Account created on 14 September 2021, almost 4 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States hooroomoo

hooroomoo changed the visibility of the branch 3525592-cli-command-to to hidden.

🇺🇸United States hooroomoo

Some visual regressions for the other dialogs should get addressed

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

hooroomoo changed the visibility of the branch 3525587-cli-command-to to hidden.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

hooroomoo changed the visibility of the branch 3525580-cli-command-to to hidden.

🇺🇸United States hooroomoo

Gonna continue this, would like this changes in for CLI tool work

🇺🇸United States hooroomoo

I noticed that this focus color for the TextField is different from the focus color of TextAreas. (because text area hasn't been de-radixified yet and I think is just using the color defined in the top level Radix theme wrapper)

I understand the appearance will be updated after designs are finalized, but until then, I think the focus colors should still be consistent across TextField and TextArea since they were consistent before.

Maybe we can we just change the TextField focus color to be the radix default for now instead of updating an out of scope TextArea here.

🇺🇸United States hooroomoo

crediting @lauriii for the PoC this MR is based off of

🇺🇸United States hooroomoo

This is ready for an initial review for the /cli changes.

Note: it is built on top of an older version of Implement OAuth2 authentication for API endpoints related to code components Active .

🇺🇸United States hooroomoo

hooroomoo changed the visibility of the branch 3525583-cli-command-to to hidden.

🇺🇸United States hooroomoo

This is pretty much ready for a review. It still needs Implement OAuth2 authentication for API endpoints related to code components Active to get in though since this MR has changes from there.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

would re-roll myself but can't risk needing to re-install my drupal instance at this current moment 😬

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

Marking as stable blocker to remove the band-aid that currently covers up a bug.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

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.

🇺🇸United States hooroomoo

Adding related issue. Added todos in there to point to this one.

🇺🇸United States hooroomoo

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.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

Yup its much better thanks, reviewed and approved.

🇺🇸United States hooroomoo

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.

🇺🇸United States hooroomoo

adding credit for helpful discussion/pointers

🇺🇸United States hooroomoo

Looks good to me! but still needs a /ui/src/components/form/inputBehaviors.tsx approval

🇺🇸United States hooroomoo

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.

🇺🇸United States hooroomoo

Still WIP but here's what i have so far, using react-json-tree to render response in new tab in code editor

Production build 0.71.5 2024