preline icon indicating copy to clipboard operation
preline copied to clipboard

Random Overlay Issue Since Upgrading

Open mikescola opened this issue 1 year ago • 1 comments

Summary

Random Overlay Issue Since Upgrading

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

mikescola avatar Oct 21 '24 03:10 mikescola

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

  1. 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.
  2. 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:

  1. Check for valid overlay.element: Added a check at the beginning of the loop to ensure overlay.element and overlay.element.overlay exist.
  2. Check for valid backdrop: After selecting the backdrop with document.querySelector(), we check if it exists before proceeding.
  3. 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.

Root-acess avatar Oct 23 '24 05:10 Root-acess

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

jahaganiev avatar Nov 01 '24 16:11 jahaganiev