Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[useOverlay]: Mount overlay layer on KDS installation even if DOM Content is still loading #878

Open
AlexVelezLl opened this issue Dec 27, 2024 · 2 comments · May be fixed by #879
Open

[useOverlay]: Mount overlay layer on KDS installation even if DOM Content is still loading #878

AlexVelezLl opened this issue Dec 27, 2024 · 2 comments · May be fixed by #879
Assignees
@AlexVelezLl

Comments

@AlexVelezLl
Copy link
Member

Product

Kolibri.

Actual behavior

Kolibri plugins and components are being mounted before the DOMContentLoaded event is triggered, which causes issues when we want to get the overlay layer element on component mount:

For example, now that we want KSelect to render its dropdown on the overlay layer, we cant always get it because in some cases when KSelect is mounted, the overlay layer hasnt been mounted yet as the DOMContentLoaded event hasnt been triggered.

Image

We haven't noticed this before with KModal, because almost all Kolibri plugins load data before mounting their components. So in this data loading time, DOM Content has time to load and mount the overlay layer before the plugin components are mounted.

In the specific case of the previous image, we didnt had to load data before mounting the plugin components (in the my_download plugin), so thats why the components were mounted inmediately, before the overlay layer was mounted.

Expected behavior

I think there are some options to better coordinate and ensure that the overlay layer is mounted before other components that depend on it. But as KDS is always being installed before all plugins components are mounted, I think the easier solution should be to avoid waiting for the DOMContentLoaded event to be triggered, and rather mount the overlay layer and live region right away.

In any case we can have wrap the mounting layers logic in a try catch and add the listener for the DOMContentLoaded if an error was raised. Something like:

  if (!isNuxtServerSideRendering()) {
    const mountKDSLayers = () => {
      _mountLiveRegion();
      mountOverlay();
    };

    if (document.readyState === 'loading') {
      try {
        mountKDSLayers();
      } catch (e) {
        const onDomReady = () => {
          mountKDSLayers();
          document.removeEventListener('DOMContentLoaded', onDomReady);
        };
        document.addEventListener('DOMContentLoaded', onDomReady);
      }
    } else {
      mountKDSLayers();
    }
  }

But as we are just adding a div to the body, we just need the body to be available, and that happens right after the browser parses the <body> opening tag, we dont need to wait for all DOM Content to be loaded. The only case it would fail is that if we load the plugin script in the <head>, but thats very unlikely to happen.

Additional information

Environment

  • OS:
  • Browser version:
@rtibbles
Copy link
Member

This seems the neatest way to handle this - I don't think there is a completely foolproof way to handle this if the script is run from a <head> tag, as even if we used something like MutationObserver, we would have no way of guaranteeing that its listener would trigger prior to an attempt to attach to the overlay node.

@rtibbles
Copy link
Member

Or - remind me, why don't we just try to mount the overlay when we try to use it?

@AlexVelezLl AlexVelezLl linked a pull request Dec 27, 2024 that will close this issue
Open
@AlexVelezLl AlexVelezLl self-assigned this Jan 7, 2025
@AlexVelezLl AlexVelezLl linked a pull request Jan 7, 2025 that will close this issue
Mount overlay layer if it needed on getOverlayEl #879
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexVelezLl AlexVelezLl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Mount overlay layer if it needed on getOverlayEl AlexVelezLl/kolibri-design-system
2 participants
@rtibbles @AlexVelezLl