Random Overlay Issue Since Upgrading

mikescola commented 1 month ago

Summary

Steps to Reproduce

So I should preface this, I am using Turbo in my application -- which I know can cause some issues when items are not properly cleaned up. I believe this is a side effect of that. My live error reporting keeps catching this error:

Argument 1 ('element') to Window.getComputedStyle must be an instance of Element

Which ties back to:

`overlays.forEach((overlay) => {

    const overlayZIndex = parseInt(

        window

            .getComputedStyle(overlay.element.overlay)

            .getPropertyValue('z-index'),

    );

    const backdrop: HTMLElement = document.querySelector(

        `#${overlay.element.overlay.id}-backdrop`,

    );

    const backdropZIndex = parseInt(

        window.getComputedStyle(backdrop).getPropertyValue('z-index'),

    );

    if (overlayZIndex === backdropZIndex + 1) return false;

    if ('style' in backdrop) backdrop.style.zIndex = `${overlayZIndex - 1}`;

    document.body.classList.add('hs-overlay-body-open');

});

};`

Specifically:

window.getComputedStyle(backdrop).getPropertyValue('z-index'),

I'm pretty sure this will end up getting fixed when 2.6 comes out with proper cleanup, but I wanted to bring it to your attention so it doesn't get missed in the release. I will note it doesn't appear to be really effecting anything as far as UX.

Demo Link

https://preline.co/docs/preline-javascript.html

Expected Behavior

No response

Actual Behavior

No response

Screenshots

No response

Root-acess commented 1 month ago

The issue you are encountering seems to be related to overlays and how their z-index is calculated in conjunction with backdrops. The error you're seeing:

Argument 1 ('element') to Window.getComputedStyle must be an instance of Element

typically happens when the DOM element being passed to window.getComputedStyle() has either been removed from the DOM or is null, which is often the case when items are not properly cleaned up—particularly in a scenario like yours, where you're using Turbo, which can cause elements to be left in an invalid state.

Problem Breakdown

The error occurs at this point:
```
window.getComputedStyle(backdrop).getPropertyValue('z-index')
```
This suggests that the backdrop variable is null or not a valid element at the moment when getComputedStyle() is called, which is why the error occurs.

Possible Causes

Element not found: The selector document.querySelector(#${overlay.element.overlay.id}-backdrop) might be returning null if the element is not present in the DOM at the time of execution.
Turbo-related cleanup issues: As you've mentioned, Turbo could be contributing to this, especially if elements are not properly removed or updated in the DOM when navigating between pages or loading content asynchronously.

Solution

To handle this issue, you can add a check to ensure that both overlay.element and backdrop are valid DOM elements before calling getComputedStyle() on them. Here's how you can modify your code:

overlays.forEach((overlay) => {
    // Ensure the overlay element exists before trying to access its properties
    if (!overlay.element || !overlay.element.overlay) return;

    const overlayElement = overlay.element.overlay;

    // Get the z-index for the overlay
    const overlayZIndex = parseInt(
        window.getComputedStyle(overlayElement).getPropertyValue('z-index')
    );

    // Query for the backdrop element
    const backdrop = document.querySelector(`#${overlayElement.id}-backdrop`);

    // Check if the backdrop exists before continuing
    if (!backdrop) return;

    // Get the z-index for the backdrop
    const backdropZIndex = parseInt(
        window.getComputedStyle(backdrop).getPropertyValue('z-index')
    );

    // Ensure the z-index logic works correctly
    if (overlayZIndex === backdropZIndex + 1) return false;

    // Update the backdrop's z-index if necessary
    if ('style' in backdrop) backdrop.style.zIndex = `${overlayZIndex - 1}`;

    // Add class to the body for open overlay
    document.body.classList.add('hs-overlay-body-open');
});

Explanation of Changes:

Check for valid overlay.element: Added a check at the beginning of the loop to ensure overlay.element and overlay.element.overlay exist.
Check for valid backdrop: After selecting the backdrop with document.querySelector(), we check if it exists before proceeding.
Return early on invalid elements: If any element is missing, we return early to avoid the error.

Expected Behavior After Fix:

This should prevent the error Argument 1 ('element') to Window.getComputedStyle must be an instance of Element from occurring.
Your application should continue to work as expected without any noticeable impact on the user experience, as you mentioned the issue doesn't seem to affect UX.

Further Steps

You may want to monitor for any additional issues related to Turbo and DOM cleanup. Once version 2.6 of your library is released, it might resolve this problem more permanently, but the above code should work as a patch in the meantime.

jahaganiev commented 2 weeks ago

@mikescola hope the above suggestion addresses the issue you are facing. Thanks for the input @Root-acess

htmlstreamofficial / preline