🇺🇸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

doing followup #2 for the frontend code improvement review feedback that didn't get addressed yet here 📌 Allow CMS Author to set site's homepage from navigation Postponed

🇺🇸United States hooroomoo

hooroomoo changed the visibility of the branch 3503412-fe-followup to hidden.

🇺🇸United States hooroomoo

Thanks @mayur-sose. Good catch. It is because the frontend is checking the auto-save endpoint of staged_config_update for the homepage and after we publish the page, the auto-save endpoint doesn't have that anymore.

🇺🇸United States hooroomoo

Yep TC8 is expected behavior. We hide the "Set as homepage" option if the page is already set as the homepage. Although it should not appear again after a reload which I don't see myself.

TC10: If the homepage hasn't been set yet the api returns a 404 and we call it to know which page to render the home icon for. I'm gonna try to find a way to work around this in my FE follow-up MR.

TC11: Hm I'm not able to reproduce this. What do you mean by showing incorrectly?

🇺🇸United States hooroomoo

I discovered if you remove the outer div to the above component, it looks like it works as expected on animationend . So maybe the problem is the border is getting recalculated using the .spin-in-container parent div position instead of the .spin-in-box inner div that the animation is actually on?

Below is GIF with the parent div .spin-in-container commented out

🇺🇸United States hooroomoo

Seeing the slot rendering weird

Posting the component i was testing with here:

const SpinInComponent = ({slot1}) => {
  return (
    <div className="spin-in-container">
      <div className="spin-in-box">{slot1}</div>
    </div>
  );
};

export default SpinInComponent;
.spin-in-container {
  width: 300px;
}
.spin-in-box {
  background-color: #3498DB;
  color: white;
  padding: 20px 40px;
  border-radius: 8px;
  font-size: 1.2em;
  text-align: center;
  animation: spinIn 2s ease-out forwards;
  height: 100px;
  width: 300px;
}
@keyframes spinIn {
  0% {
    transform: translateX(-100%) rotate(0deg);
    opacity: 0;
  }
  50% {
    opacity: 1;
  }
  100% {
    transform: translateX(0) rotate(360deg);
  }
🇺🇸United States hooroomoo

Assigning to myself to work on the frontend followups. I plan to open a separate MR here.

🇺🇸United States hooroomoo

I was asked to add the frontend parts to the MR so feel free to assign to me after but i will just start on it today

🇺🇸United States hooroomoo

I want to confirm something with the src url later but the frontend part is pretty much done ready for a review.

🇺🇸United States hooroomoo

@wim-leers

Getting 422 Error from the backend when you do Add to components because the poster values we use fail the validation because it looks like the validation checks for image file name extensions:

https://placehold.co/1080x1920?text=Vertical
https://placehold.co/1920x1080?text=Widescreen

🇺🇸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

Looks good and works well! Just minor comments

🇺🇸United States hooroomoo

@wim-leers No need to apologize! 🙂I thought it'd be a simpler fix before working on it too

🇺🇸United States hooroomoo

I don't consider this a novice issue after working on it

🇺🇸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 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 😬

Production build 0.71.5 2024