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

Too strict approach when hiding host element in Popover component #7979

Open
tomerlichtash opened this issue Oct 16, 2024 · 1 comment
Open
Labels
question Further information is requested vaadin-popover waiting for author Further information is requested

Comments

@tomerlichtash
Copy link

tomerlichtash commented Oct 16, 2024

Description

In a recent commit to the new Popover component the host element was set to hidden (see #7671), so it will be aligned with other modal-related components hidden host (see #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
Copy link
Member

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 rolfsmeds added question Further information is requested waiting for author Further information is requested labels Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested vaadin-popover waiting for author Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants