🇬🇧United Kingdom @jessebaker

🐛 Previous page search results flash briefly when opening the Navigator Active Created a follow up to address #13 "Users momentarily see the previous results list before the full list is displayed again"

Do we need another follow up for the "unpublished pages appear in the search navigator" or is that working as intended?

jessebaker → created an issue.

jessebaker → created an issue.

hooroomoo → credited jessebaker → .

MR!1338 addresses both

Pressing enter/submitting the search form reloads the page
Re-opening the Navigator resets the search input value but not the search results.

justafish → credited jessebaker → .

It seems like this issue is now the catch all for VH related issues. I've added two other more specific issues as related but I think they will be fixed by @bnjmnm's solution in the MR which is not specific to Tailwind classes.

I've created a follow up 🐛 Investigate the use of React fragments in JS components Active in response to the issue with React fragments mentioned in #4.

jessebaker → created an issue.

I've just created a new MR that I think solves this. I've added e.stopPropagation to the AI wizard container onKeydown.

I think it might be better to put this code somewhere closer to, or on, the actual div that is being used as a text area.

jessebaker → made their first commit to this issue’s fork.

I've just seen there was a collision when I added the file when updating the IS. So adding the files again just in case.

More detailed steps to reproduce

1. Have a page with 16+ JS components, with slots on. I have 8 nested inside one another and then another 8 nested inside one another below that.
2. Have Chrome's Dev tools open
3. Rapidly update a prop and wait until the preview updates but is missing 1 or more component.
4. In Chromes dev tools I found emulating a slower (4x slow) CPU helped make this happen more often

I used this code for my JS component (needs a slot called mySlot).

const SlotComponent = ({mySlot}) => {
  return (
    <div className="max-w-sm min-h-36 p-2 font-bold rounded-lg text-2xl gap-4 relative mx-auto flex w-full flex-col items-center justify-center bg-[#ffc423] text-[#12285f]">
      <div className="slotc w-full bg-[hotpink]">{mySlot}</div>
    </div>
  );
};
export default SlotComponent;

Then for the inner-most component (8 layers deep) I have used

const AnimateInComponent = ({
  title = "Animate In Component",
}) => {
  return (
    <div className="anim-in">
      <h3>{title}</h3>
    </div>
  );
};

export default AnimateInComponent;

With this CSS

.anim-in {
            width: 300px;
            height: 250px;
            background-color: #ffc423;
            display: flex;
            align-items: center;
            justify-content: center;
            font-size: 20px;
            animation: loopAnimation2 2s 1 alternate ease-in-out;
        }

        @keyframes loopAnimation2 {
            0% {
                transform: scale(0.2) rotate(-20deg);
            }
            50% {
             transform: scale(1.2) rotate(20deg);
            }
            100% {
                transform: scale(1) rotate(0deg);
            }
        }

Approved so this can be merged but would like to see a follow up created to address Lee's comment and my concern about performance before this is closed.

(oh I've just seen #72 which was posted as I was writing this!)

Taking the decision to postpone this for a while until a more concrete plan is in place. @balintbrews' comment in #24 has raised some questions that need to be well thought out before we can continue.

The following issues might as well proceed and land ahead of this now.
🐛 Using animation css on sdc affects slot position and size Active
🐛 Component/slot overlay borders can be misaligned when nested Active
🐛 Sometimes Astro slots fail to render/hydrate Active

Fundamentally I think this is all working and the MR is good but there are a few small bits that should be addressed.

Given the relative urgency of this, I think the remaining threads on the MR could be shifted to a follow up if there isn't time to fix them.

Adding some related issues that should be merged after this lands:

🐛 Using animation css on sdc affects slot position and size Active
🐛 Component/slot overlay borders can be misaligned when nested Active
🐛 Sometimes Astro slots fail to render/hydrate Active

I have gotten to a point where I can fairly easily reproduce this error. Unfortunately, so far, that hasn't lead to any revelations as to what the cause is, but it's certainly progress.

The major factors in making it easy to reproduce is

1. Have a page with 16+ JS components, with slots on. I have 8 nested inside one another and then another 8 nested inside one another below that.
2. Have Chrome's Dev tools open
3. Rapidly update a prop and wait until the preview updates but is missing 1 or more component.

To make it really obvious when the rendering has failed I have given the inner most components an animation so as soon as the preview updates, if no animation plays I know that something hasn't loaded.

As described in the IS, the issue appears to be that the element exists, but it has nothing inside it. There are no network or console errors.

One thing of note - it happened when adding a new component that I just created for the first time. It could have just been a coincidence.

Investigation continues.

Follow-up created ✨ Move Publishing error reports to the top of the list Active

jessebaker → created an issue.

jessebaker → made their first commit to this issue’s fork.

@laurii the issue is that if you customise the look (background colour, border, rounded corners etc) of the "picker" (the dropdown that opens with the list options when you click the select) then you lose the positioning of the element (which is dictated by the OS I believe). So the options are

1. Have the picker look like the browser/os native in both look and feel and position
2. Have the picker with a custom look and feel, but without control over the position (it just appears below like a more traditional drop down element, or offset by a fixed amount of pixels).

I adjusted the options dropdown position (only in Chromium browsers where this styling is available) to be more "traditional" and appear directly below the select element.

Firefox still shows the default Mac/Browser styling as we don't have control over that.

This seems to only be an issue in Chrome on my Mac. Firefox renders selects with the native browser
element styling so the position of the options list is as expected.

@lauriii answered my question in #2 on a call: the issue is mainly happening in the XB preview. It tends to happen on pages with more components that have higher levels of nesting. He HAS seen it on the front end rendered page, but not for a long time and it is certainly more rare (probably because it is not reloading over and over like the editor iframe does).

Is this issue happening when viewing the page in the preview iframe in XB or on the front end/published version of the page?

jessebaker → created an issue.

I've pushed a slightly experimental MR. It's not optimised very well and is a bit hacky but I think the concept is along the right lines. I'll continue to work on it but thought I'd push it up asap in case you want to try it out.

I've had a bit of a play around with how easy to implement this is and it's fairly trivial to get the component overlay to correctly update after the animation has finished but not so easy to trigger a recalculation of all the children (a component that animates and has a slot will fire the animation end on the component but the slot, as a child, doesn't receive that same event).

A thought I had was to have the border recalculation occur on mouseEnter of the overlay component so that even if it gets out of sync, it will 'snap' to the correct size if you attempt to interact with it. This works for components but the overlay for empty slots has pointer-events: none;. I need to investigate why that's the case.

I suspect this is because the animation is not actually changing the observable size of the component - it's using transform/translate etc. That means that when the animation finishes the resize observer is not fired again to recalculate the overlay sizes.

In animate-height.gif attached you can see the border is correctly tracking the size of the element (this element has a looping animation that changes the css height property.

This might be solved/improved if we also recalculate the overlay size on the animationend (and/or animationiteration) event. We already call the recalculation on transitionend in ui/src/hooks/useSyncPreviewElementSize.ts.

effulgentsia → credited jessebaker → .

@libbna - I see you already reviewed/tested this earlier.

My bad, I didn't correctly assign the ticket to me when I began first thing this morning so you tested before I was ready! I've had to refactor the code since you tested which unfortunately probably invalidates the test you did. If you have an opportunity to re-test, that would be hugely appreciated.

jessebaker → made their first commit to this issue’s fork.

jessebaker → made their first commit to this issue’s fork.

jessebaker → created an issue.

jessebaker → made their first commit to this issue’s fork.

Follow up for persisting the selection as described in #20 and #21 created here ✨ Selective publishing selection should be more persistent (across page loads etc) Active

jessebaker → created an issue.

RE: #20 I think an improvement to UX generally would be to ensure that the selected items are stored in something a bit more long term than just memory - local storage for example. Otherwise a similar issue of trying to review pages one by one and check them off is difficult because each time you navigate to a new page the selection is lost.

I think perhaps a single follow up to address both scenarios would be best to ensure the solution enables both things (maintaining after error, maintaining after page load)

I've not addressed "If they somehow access it, the error message should have a clear close button and subsequently redirect the user appropriately."

The user without permissions should only be able to get to that error screen by typing in the URL. Now that the "Edit code" options is correctly wrapped, I don't think there are any other ways to get there so I don't think it's necessary. They will have to just click the Drop icon in the top left and navigate out of XB entirely (or edit the URL)

jessebaker → made their first commit to this issue’s fork.

wim leers → credited jessebaker → .

This is technically a duplicate of 🐛 Components with heigh in VH can cause the preview iFrame to expand infinitely in height Active but as no solution for that has been put forward yet, perhaps we can put in a hard-coded Tailwind specific fix for this class name.

I have been testing this and the issue described is fixed, at least partially. When returning to the Page Data tab the selected image is correctly displayed. However after leaving and returning to the page data tab, the remove image button no longer functions correctly as the entire widget is removed from the DOM (as shown in attached video).

Furthermore, the same issue described in the IS is also present for media widgets in the component instance form (leaving the form and returning means the selected image is no longer shown).

I don't believe this is a duplicate of 🐛 Dropdown List in Page Data Area of XB Platform Not Scrollable, Preventing Access to Taxonomy Term Options Active .

This issue is that the autocomplete suggestions is limited to only 10 results and that you cannot scroll the list to see >10 results.

The other issue is a style/position issue where if the list of suggestions reaches the bottom of the page, it is clipped/displayed off screen and you cannot scroll the page to access all the results shown. (the list should pop upwards if their is not enough space below the input element).

jessebaker → made their first commit to this issue’s fork.

jessebaker → made their first commit to this issue’s fork.

jessebaker → made their first commit to this issue’s fork.

I'm going to close this - I think ultimately the context menu and it's position is 99% 3rd party code coming from the Radix library and we have to assume they have their own tests.

In the original implementation of our canvas there was a quirk that meant the scaling was somehow introducing errors in the positioning logic but I don't see that we will be going back to that implementation and a test is only really going to be testing that some hypothetical future change isn't going to introduce an unknown bug.

I'm not sure it's possible to actually get into those situations (yet?!) but good catch. At the moment, you can't even access XB if you don't have permission to edit XB pages so if you can access XB then you will always have at least one option in the menu.

Anyway I've added some more robustness around the UI when people have no permissions or can delete things but don't have any other items in the menu which should keep this looking tidy in the future no matter what configuration of permissions people end up with!

jessebaker → made their first commit to this issue’s fork.

Rather than just removing the option, I took it upon myself to make a basic version of this work as it may help with a11y etc.

Now the "Move Into" menu item displays a sub menu that contains a list of slots belonging to the siblings of the component in context. Clicking one will move the component into the chosen slot.

The functionality is basic but offers a basis onto which we can add later improvements as the need arises or if there is input from UX at some point in the future. I can already predict now that you can "move into" it would be helpful to "move out of" - but I don't want to grow the scope more than I already have.

jessebaker → made their first commit to this issue’s fork.

Finally got to the bottom of a number of issues that were causing a lot of friction in writing the Playwright tests for this work. All resolved now and ready for another round of review.

The summary on the MR is the most up to date place for all info on this work.

penyaskito → credited jessebaker → .

jessebaker → created an issue.

jessebaker → made their first commit to this issue’s fork.

There is a simpler approach to this using our already existing useDebounce hook. Hopefully the comments on the MR explain clearly how to implement it!

I have merged, this can be tested on 0.x now @neha_bawankar

I'm afraid I don't remember why I added the min-height to the dialogs. I have removed it and everything seems to be working find so I'm going to chalk it up to me trying something that I later backed out of but forgot to remove.

In the process of reviewing this, I worked with @callumharrod to refine some of the designs to better account for the weights fields and improve the look of the "remove" button.

In working with him, we decided to change the media items to stack in rows instead of the 3 columns. It provides better readability, a better use of space and accounts for media items with longer file names much better.

I've gone ahead and implemented his suggested changes in this MR. I've also given it some pretty thorough testing and a code review and it's all looking good!

(I found one bug that is also present in 0.x - if you have a media widget on the page data side bar, pick some images and then switch to a different sidebar and come back the selected images are no longer shown).

wim leers → credited jessebaker → .

jessebaker → made their first commit to this issue’s fork.

The format of contentEntityCreateOperations which was merged is useful - it's easy to iterate over to build the list of "New x" buttons in the UI. Your proposal in #16 means, to build the list of "New x" buttons, I would have to iterate over the permissions object, filter to just keys that starts with 'create:' and then split the key by : to get the entity type and bundle info.

Maybe there is some value to having the permissions listed in the permissions object as you propose, but I think only if that was in addition to still having the info in the original format elsewhere e.g. drupalSettings.xb.contentEntityCreateOperations

"contentEntityCreateOperations":{
    "xb_page":{"xb_page":"Page"},
     "node":{"article":"Amazing article"}
   }

I'm slightly too late to the party but I've just merged in upstream changes over at ✨ Create React Permission Utilities Active and the shape of the permissions object is now more complex as a result of this and as a result is going to make the FE code more complex to work around it.

Previously xb.permissions was a simple

interface permissions {
  [key: string]: boolean;
}

This made it easy for the FE to quickly assert if a user had or did not have a given permission by simply checking that list of booleans.

Can this new contentEntityCreateOperations object be moved into the root of drupalSettings.xb instead of inside the permissions object?

jessebaker → created an issue.

@larowlan I'm unclear what "this code" is referring to in the following:

Make this code more resilient so that every source plugin doesn't have to conform to a particular twig format in order to get the HTML comments

Do you mean make the code that inserts the HTML comments more resilient (so change that a Source plugin can render its output without using xb_slot_ids and xb_uuid but still have the Twig extension output the comments.) or are you talking about making the FE code more resilient (e.g. handle the situation where the layout/model data suggests there is a slot but the markup does not have comments)?

Marking as postponed pending a bit more info as I am unable to reproduce.

@lauriii Do you think this is a speed/race condition situation? It seems like, in the gif, the issue occurs when quickly clicking the link immediately after refreshing the browser? Do you think that's the reason why it's only happening "in some instances".

For clarity the way the link interception works is that there is a drupal behaviour loaded in the page when it's in the preview that preventsDefault on the click event and uses a postmessage to communicate up to the parent of the iFrame (our React app).

My suspicion is that some kind of race condition either means you are able to click the link before the event handler has been bound to the link OR the event handler is not getting bound to the link as a result of it running before the link is rendered on the page (by Astro).

Embedded screenshot.

Adding related issue https://www.drupal.org/project/experience_builder/issues/3529623 📌 Style Autocomplete suggestions Active
Removing related issue around the extensions dialog - The solution there is unfortunately not applicable here.

Adding related issue - perhaps this can be addressed at the same time?

🐛 Dropdown List in Page Data Area of XB Platform Not Scrollable, Preventing Access to Taxonomy Term Options Active

In all honesty I found this very difficult to reproduce in any kind of consistent way. Following the steps in the video lead to the issue being exhibited maybe 10% of the time.

With my fix (which is a bit of a stab in the dark) I've not been able to reproduce the issue - but I'm not confident I've identified the reason the issue is occuring so maybe it just makes it better but there is still some underlying race condition that could still happen.

jessebaker → made their first commit to this issue’s fork.