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.

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

Popover Menu Not Loading Correctly Inside Shadow DOM #1319

Closed
taran2k opened this issue Jun 20, 2024 · 10 comments
Closed

Popover Menu Not Loading Correctly Inside Shadow DOM #1319

taran2k opened this issue Jun 20, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@taran2k
Copy link

taran2k commented Jun 20, 2024

Current Behavior:

The newly added menuContainer property is being ignored, and popovers are still added as the latest div element instead of being within the scope of the provided menuContainer element, making it so that the chapters popover as well as the settings popover are unusable when a video is being played inside a modal using the HTMLDialogElement API.

Expected Behavior:

Expected behaviour is for the given menuContainer element to be the container inside which the chapters and settings menu render.

Steps To Reproduce:

import '@material/web/button/elevated-button';
import '@material/web/iconbutton/icon-button';
import '@material/web/icon/icon';

import {LitElement, html, css} from 'lit';
import {customElement, property, query} from 'lit/decorators.js'; // eslint-disable-line import/extensions

import {type MediaPlayerElement} from 'vidstack/elements';
import {VidstackPlayer, VidstackPlayerLayout} from 'vidstack/global/player';

import vidstackStyles from 'vidstack/player/styles/default/layouts/video.css?url';
import defaultStyles from 'vidstack/player/styles/default/theme.css?url';

import {DisableBodyScroll} from './mixins/disableBodyScroll';
import {type ModalElement} from './modal';
import './modal'; // eslint-disable-line no-duplicate-imports

@customElement('video-content')
// eslint-disable-next-line wc/file-name-matches-element -- impossible filename
export class VideoContentElement extends DisableBodyScroll(LitElement) {
  @property({type: String, attribute: 'video'})
  public accessor fileUuid = '';

  @query('#modal')
  private accessor modal: ModalElement | null = null;

  @query('#video-player')
  private accessor videoPlayer: HTMLVideoElement | null = null;

  private _vidstack: MediaPlayerElement | null = null;

  static styles = css`
    video:not(.vidstack:fullscreen video) {
      max-height: calc(90vh - 2rem); /* This is a hack to ensure proper sizing */
    }

    #video-player {
      aspect-ratio: unset;
    }
  `;

  render() {
    return html`
      <link rel="stylesheet" href="${defaultStyles}" />
      <link rel="stylesheet" href="${vidstackStyles}" />

      <md-elevated-button
        class="dialog-button"
        @click="${this._openModal}"
      >
        Open Video Modal
      </md-elevated-button>

      <modal-element
        id="modal"
        @open="${this._onOpenModal}"
        @close="${this._onCloseModal}"
      >
        <span slot="title">Demo Video</span>
        <vidstack-player id="video-player"></vidstack-player>
      </modal-element>
    `;
  }

  private async _initializePlayer() {
    const menuContainerElement = document.getElementById('modal');
    const player = await VidstackPlayer.create({
      target: this.videoPlayer!,
      src: 'https://files.vidstack.io/sprite-fight/720p.mp4',
      viewType: 'video',
      streamType: 'on-demand',
      logLevel: 'warn',
      crossOrigin: true,
      playsInline: true,
      title: 'Sprite Fight',
      poster: 'https://files.vidstack.io/sprite-fight/poster.webp',
      layout: new VidstackPlayerLayout({
        thumbnails: 'https://files.vidstack.io/sprite-fight/thumbnails.vtt',
        menuContainer: menuContainerElement,
      }),
      tracks: [
        {
          src: 'https://files.vidstack.io/sprite-fight/subs/english.vtt',
          label: 'English',
          language: 'en-US',
          kind: 'subtitles',
          type: 'vtt',
          default: true,
        },
        {
          src: 'https://files.vidstack.io/sprite-fight/subs/spanish.vtt',
          label: 'Spanish',
          language: 'es-ES',
          kind: 'subtitles',
          type: 'vtt',
        },
        {
          src: 'https://files.vidstack.io/sprite-fight/chapters.vtt',
          language: 'en-US',
          kind: 'chapters',
          type: 'vtt',
          default: true,
        },
      ],
    });

    return player;
  }

  private async _openModal() {
    this.modal?.open();
  }

  private async _onOpenModal() {
    this._disableBodyScroll();
    this._vidstack?.play(); // If the video already exists, resume playing it.
    this._vidstack ??= await this._initializePlayer(); // Otherwise initialize it.
  }

  private _onCloseModal() {
    if (this._vidstack) {
      this._vidstack.pause();
    }
    this._restoreBodyScroll();
  }
}

Environment:

Video Reconstruction

Schermopname.van.2024-06-20.08-22-55.webm
@taran2k taran2k added the bug Something isn't working label Jun 20, 2024
@taran2k taran2k changed the title Settings and library popover always being appended as last div element, because mediaContainer is being ignored Settings and library popover always being appended as last div element, because menuContainer is being ignored Jun 20, 2024
@mihar-22
Copy link
Member

Thanks, I'll look into it!

@taran2k

This comment was marked as off-topic.

@taran2k
Copy link
Author

taran2k commented Jun 20, 2024

@mihar-22 Seems like the issue was on my end, trying to call DOM elements the wrong way. Was solved by adding a container within the modal element with id menu-container, and fetching it through

  @query('#menu-container')
  private accessor menuContainerElement: HTMLElement | null = null;

which makes it so this works just fine:

      layout: new VidstackPlayerLayout({
        ...defaultVidstackLayout,
        menuContainer: this.menuContainerElement,
      }),

@taran2k taran2k closed this as completed Jun 20, 2024
@taran2k
Copy link
Author

taran2k commented Jun 20, 2024

A new issue did arrive now though, only the settings and chapters container pop up like they should. But all nested containers don't (playback speed, audio settings, etc.)...

Schermopname.van.2024-06-20.12-17-17.webm

@taran2k taran2k reopened this Jun 20, 2024
@mihar-22 mihar-22 changed the title Settings and library popover always being appended as last div element, because menuContainer is being ignored menu container prop is ignored Jul 5, 2024
@mihar-22
Copy link
Member

mihar-22 commented Jul 5, 2024

You'll need to provide me a reproduction because this is working in my testing. Closing until one is provided, thanks!

@mihar-22 mihar-22 closed this as completed Jul 5, 2024
@taran2k
Copy link
Author

taran2k commented Jul 5, 2024

@mihar-22 Could you reopen the issue?

I've created a simple recreation of the issue by simulating the way components are loaded within the shadow DOM. As can be seen, when a modal is loaded within a shadow DOM, issues arise. Below is the reproduction.

Reproduction

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Video Player Modal Example</title>
    <link rel="icon" type="image/png" sizes="32x32" href="/favicon-32x32.png" />
    <link rel="icon" type="image/png" sizes="16x16" href="/favicon-16x16.png" />
</head>

<body>
    <main>
        <button id="openButton">Open Video Modal</button>

        <foo-element></foo-element>
    </main>

    <script type="module">
        class FooElement extends HTMLElement {
            constructor() {
                super();
                const shadow = this.attachShadow({ mode: "open" });

                // External stylesheets within shadow DOM
                const themeLink = document.createElement("link");
                themeLink.setAttribute("rel", "stylesheet");
                themeLink.setAttribute("href", "https://cdn.vidstack.io/player/theme.css");
                shadow.appendChild(themeLink);

                const videoLink = document.createElement("link");
                videoLink.setAttribute("rel", "stylesheet");
                videoLink.setAttribute("href", "https://cdn.vidstack.io/player/video.css");
                shadow.appendChild(videoLink);

                const dialog = document.createElement("dialog");
                dialog.id = "videoModal";
                const video = document.createElement("div");
                video.id = "videoPlayer";
                const menu = document.createElement("div");
                menu.id = "menuContainer";
                dialog.appendChild(video);
                dialog.appendChild(menu);
                shadow.appendChild(dialog);
            }
        }
        customElements.define("foo-element", FooElement);
    </script>

    <script type="module">
        import { VidstackPlayer, VidstackPlayerLayout } from 'https://cdn.vidstack.io/player';

        document.addEventListener('DOMContentLoaded', () => {
            const openButton = document.getElementById('openButton');
            const fooElement = document.querySelector('foo-element');

            openButton.addEventListener('click', async () => {
                const shadow = fooElement.shadowRoot;
                const videoModal = shadow.getElementById('videoModal');
                const videoPlayerContainer = shadow.getElementById('videoPlayer');
                const menuContainer = shadow.getElementById('menuContainer');

                videoModal.showModal();

                const player = await VidstackPlayer.create({
                    target: videoPlayerContainer,
                    src: 'https://files.vidstack.io/sprite-fight/720p.mp4',
                    viewType: 'video',
                    streamType: 'on-demand',
                    logLevel: 'warn',
                    crossOrigin: true,
                    playsInline: true,
                    title: 'Sprite Fight',
                    poster: 'https://files.vidstack.io/sprite-fight/poster.webp',
                    layout: new VidstackPlayerLayout({
                        thumbnails: 'https://files.vidstack.io/sprite-fight/thumbnails.vtt',
                        menuContainer: menuContainer,
                    }),
                    tracks: [
                        {
                            src: 'https://files.vidstack.io/sprite-fight/subs/english.vtt',
                            label: 'English',
                            language: 'en-US',
                            kind: 'subtitles',
                            type: 'vtt',
                            default: true,
                        },
                        {
                            src: 'https://files.vidstack.io/sprite-fight/subs/spanish.vtt',
                            label: 'Spanish',
                            language: 'es-ES',
                            kind: 'subtitles',
                            type: 'vtt',
                        },
                        {
                            src: 'https://files.vidstack.io/sprite-fight/chapters.vtt',
                            language: 'en-US',
                            kind: 'chapters',
                            type: 'vtt',
                            default: true,
                        },
                    ],
                });

                videoModal.addEventListener('close', () => {
                    videoPlayerContainer.innerHTML = '';
                });
            });
        });
    </script>
</body>

</html>

@taran2k taran2k changed the title menu container prop is ignored Popover Menu Not Loading Correctly Inside Shadow DOM Jul 5, 2024
@taran2k
Copy link
Author

taran2k commented Jul 15, 2024

@mihar-22 Just checking in, any chance the issue could be reopened? Can't do it myself.

@mihar-22
Copy link
Member

Thanks for the repro @taran2k, found the issue and I'll release the fix some time today or tomorrow.

@amureki
Copy link

amureki commented Jul 29, 2024

Thanks for the issue and the solution!

We are seeing the similar thingy with [email protected], so I wonder if this is related or some other issue.
@taran2k was the issue resolved for you?

@taran2k
Copy link
Author

taran2k commented Jul 29, 2024

Thank you @mihar-22! This solved the problems we were having. @amureki, yes, the most recent release resolved our issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants