🇺🇸United States @hooroomoo

Account created on 14 September 2021, over 3 years ago
  • Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States 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.

🇺🇸United States hooroomoo

Marking stable blocker because it's a long and disruptive console warning

🇺🇸United States hooroomoo

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.

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

Tested manually and I also can't reproduce the issue anymore :D

🇺🇸United States hooroomoo

@wimleers

So in my MR I am using the HTML comments to create a map of the region id to it's corresponding <div region region--{regionId}> from the markup returned from the backend.

But when in Olivero, neither <div region region--primary-menu/> nor <div region region--header/> exist in the markup returned by the backend. But it exists for the rest of the regions.

And in Claro, the markup does contain <div region region--header/>

Is this a known bug that the header and primary menu don't come with the

wrapping element in Olivero? Or is it done on purpose for a reason? It makes it difficult do the "focus mode" on the header (especially if the header has more than one element) since there's no wrapping div I can target.

Another thing I noticed is the comment

<!-- xb-region-start-content -->

is after

in the markup. (After applying the patch from #9) Should it be the other way around to match the rest of the regions?
🇺🇸United States hooroomoo

Paired with @balintbrews and we got a working solution :)) Setting a unique key to the component list onDragEnd instead of an individual list item works.

🇺🇸United States hooroomoo

The above didn't work for me. I wasn't able to find a good solution.

1. Calling something like evt.clone.replaceWith(evt.item) would require changes to usePreviewSortable.ts where it changes the innerHTML to the rendered markup (updateData()) so it can render in the preview and want to avoid changes to that file if possible.

2. Tried #16 and similar things as the above to try to trigger a re-render of but I couldn't get it to work.

3. Confusing React by passing in a Math.random() like <ListItem key={Math.random()} .../> kinda works but is bad practice and also causes SortableJS errors 😛 lol

🇺🇸United States hooroomoo

Updating title because this affects any item under the Components list, not just exposed code components.

🇺🇸United States hooroomoo

I think what's happening is when an item is dragged out of the Components list, a clone from SortableJS replaces it in the list. But that clone doesn't have any of the React event handlers that the original item had which is why the component preview no longer works onMouseEnter.

So that's why it works when you close and re-open the Components list, since in that state, only the originals are there, no clones.

Will continue tomorrow.

🇺🇸United States hooroomoo

Nevermind! I am able to reproduce it. I was clicking to insert instead of dragging.

🇺🇸United States hooroomoo

Adding credit for @jessebaker for pointing me to the manualRetch that might not be necessary anymore and removing it fixed the regression i was looking at

🇺🇸United States hooroomoo

I may have found a regression so am looking into it. Clicking publish all changes shows the happy green smiley but then flashes back to show "Publish all changes" again

🇺🇸United States hooroomoo

Yay!

(I removed sprint-candidate tag)

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

I think this could benefit from having a storybook story for the different kinds of but we can always add that later if we want. Think it's valuable to get this in right now so it's easier to test backend changes i.e. 🐛 Once previewed in XB an entity with no changes will still show up in "Review x changes" Active

Assigning back to Jesse for screenshot/cast

🇺🇸United States hooroomoo

This is ready for initial round of feedback, please see MR description

🇺🇸United States hooroomoo

e2e test are passing after merging with 0.x 👍

🇺🇸United States hooroomoo

e2e tests failing, unit tests are also failing but the same files are failing on 0.x

Hoping merging w 0.x resolves the e2e

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

I manually tested this and can confirm that the media library styles look good when clicking from the Page data form from both Article and an XB_Page.

But I am unable to check for regressions from opening from the props form because I'm unable to add an image to XB because of this issue: https://www.drupal.org/project/experience_builder/issues/3501902 🐛 Adding the Image component results in a state considered invalid Active .

(I tried testing it with an in-progress MR from that issue and it let me place an image to XB but it does not open the media library dialog when i click "Add media")

🇺🇸United States hooroomoo

Leaving what i did here, since someone else should test this because I am getting weird colors in my XB gin media library.

My testing steps after applying patch from #2:

1. Applied gin patch from #2
2. Reinstalled drupal
3. Installed xb_dev_standard to create an article for xb
4. Changed admin theme to Gin
5. Merged in MR from https://git.drupalcode.org/project/experience_builder/-/merge_requests/643 so I am able to place an Image onto XB without it breaking.
6. clear cache
7. Place image onto XB and open media library
6. See screenshots.

🇺🇸United States hooroomoo

I checked that Claro still looks the same on this MR and 0.x.

There are just two things:
1. I'm unable to enter alt text after selecting an image for some reason. I can't on 0.x either so that should not affect this issue but just wanted to call it out.

2. My XB media library Gin looks different than the ones in the screenshots from #2 📌 Dialog + Media Library refinements to support Gin Active

Here is what mine looks like (I did apply the patch and checked that the changes applied to gin.libraries.yml and I cleared cache)

🇺🇸United States hooroomoo

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

🇺🇸United States hooroomoo

Gif that shows changing JSX, changing global style of primary color to pink, showing the error messages we added here for compile errors and if the code is missing a default export.

🇺🇸United States hooroomoo

Copied over the codeEditorSlice code to this from Allow code components to import from npm packages Active so adding credit now before i forget

🇺🇸United States hooroomoo

Oh looks like other blockers are also done now, so updating again to reflect

🇺🇸United States hooroomoo

Decrementing PP in issue title since hydration library got in

Production build 0.71.5 2024