Too strict approach when hiding host element in Popover component

[tomerlichtash]

Description

In a recent commit to the new Popover component the host element was set to hidden (see https://github.com/vaadin/web-components/pull/7671), so it will be aligned with other modal-related components hidden host (see https://github.com/vaadin/web-components/pull/3597).

However, in Popover, you use Lit's static get styles function, while in other components, like Dialog, you use a style tag in the template to achieve the same result (see https://github.com/vaadin/web-components/blob/main/packages/dialog/src/vaadin-dialog.js#L99).

I was wondering if the latter approach, of using Lit to enforce static CSS, is not too harsh, and if it would be possible to maintain the template style approach instead.

I myself have a use case where I internalize the overlay to be within the shadow DOM (so it won't be affected from other styles in the page) and I need to override the hidden root style. While I was able to achieve this result with Dialog, it seems that in Popover there is no way to do it.

Expected outcome

<style>
  :host {
    display: none !important;
  }
 </style>

instead of

static get styles() {
    return css`
      :host {
        display: none !important;
      }
    `;
  }

Minimal reproducible example

...

Steps to reproduce

...

Environment

Vaadin version(s): 24.5.0

Browsers

No response

[web-padawan]

If you want to apply significant changes like moving the overlay to shadow DOM, I'd suggest to do this instead:

class CustomPopover extends Popover {
  static get is() {
    return 'custom-popover';
  }

  static get styles() {
    return css`
      :host {
        display: block !important;
      }
    `
  }
}

Note, the Lit approach will be also used by other components in the future and it's based on constructable stylesheets. So we don't recommend to rely on the presence of <style> tags in Shadow DOM or using some JS to remove them.

[rolfsmeds]

@tomerlichtash, does the above suggestion solve your issue?

[tomerlichtash]

While the above explanation is quite clear, since I'm proxying Vaadin Web Components in different versions it would be difficult for me to go with the above solution, so, TBH – I went with a different approach, which suits my situation better.

Eventually, I went with overriding the constructed stylesheet, like so:

const stylesheet = new CSSStyleSheet();
stylesheet.insertRule(':host{display:block!important;}');
popover.shadowRoot.adoptedStyleSheets.push(stylesheet);

Thanks for the followup, @rolfsmeds.

[web-padawan]

I believe this should be fixed by switching to the native popover API in Vaadin 25 instead of teleportation. Until then, I don't think we should change anything in the web component since it works as intended.