oddbird/popover-polyfill

Problems with overwriting popover styling for Firefox

anna-lach opened this issue · 16 comments

I’m currently working on a popover component and I’d like to overwrite some styling for the [popover] like overflow to visible etc., but in the Firefox it doesn’t work. Since there are styles added in the popover.ts file in the polyfill it looks like I cannot overwrite those styles in my app. It seems those styles from polyfill's popover.ts file have the highest specificity.

@keithamus or @mirisuzanne I'm curious if you have suggestions for how to approach this? Seems like from an API perspective we could expose an option to omit the polyfill styles, but maybe there's a better CSS approach to allow overrides.

There are CSS-first approaches. The most relevant and simplest would be Cascade Layers. They're fairly new, but have good support for the last couple years. I'm not sure what our support matrix looks like here? The other CSS option is wrapping all provided styles in :where() selectors to remove specificity. That has browser support a bit farther back.

In either case, if there are overrides that should not be allowed, those properties can be marked as !important.

I’ll try a solution using :where tomorrow and see what impacts that has, but it seems like a good solution.

As a workaround for now, @anna-lach, you could increase specificity by, for example, using [popover][popover] (that’s not a typo, use the popover attribute selector twice) in your selectors.

Thank you @keithamus. I tried with [popover][popover] but it's still not working properly. So for now maybe I'll mark some properties as !important, because I only need it for a few like border.

@anna-lach This is released in v0.3.4.

I'm working with Anna on this. I think this has something to do with the shadow DOM: no matter what selectors I add to the scoped styling within the custom element, I cannot get it to "overwrite" the border-* styling of the polyfill.

@jgerigmeyer the :where([popover]) fix also doesn't help.

So afaics the only way to work around this is to use !important atm.

To clarify, we are doing something like this:

// Doesn't matter if how many selectors you add here, the polyfill styles will always have a higher specificity
:host([popover]) {
  border: var(--_border);
}

@jpzwarte are you using constructable stylesheets? If so, does your stylesheet appear before or after the popover stylesheet in the shadow root's adoptedStylesheets?

@keithamus We use Lit, so yes. Afaics the polyfill stylesheet appears before the component stylesheet.
CleanShot 2023-11-29 at 10 53 25@2x

Do all browsers behave the same? Is the variable for sure correct? If you add !important does it fix it? What does devtools look like?

This is how it looks in the dev tools atm:
CleanShot 2023-11-29 at 11 20 25@2x

If I add !important, it looks like this:
CleanShot 2023-11-29 at 11 22 13@2x

The only browser I have that hasn't implemented popover is FF, so I can't speak on "all browsers".

I think I know the issue.

The popover style sheet is appended to the document, so that light DOM popovers work, but :host styles always take lower precedent than the light DOM styles.

If you open this codepen you'll see a pink border. If you comment out line 19 (this.ownerDocument.adoptedStyleSheets.push(styles)) you'll get a green one.

I'm not sure what the resolution here is...

I guess !important for now. Thanks.

Yes, author styles in the light dom always override author styles in the shadow dom. The only way to override that is with !important. Unless we can provide a way for authors to load these styles into a shadow dom context, rather than putting them in the light dom? @keithamus I don't know if that's a reasonable thing to do?

I’m not sure we could do that as the styles are required for popover to work. The only other option I can think of is inline styles but then they’ll take precedence over stylesheets anyway 🤷‍♂️

That makes sense. It's too bad there's no way for polyfills to inject styles at the 'presentational hints' level of the cascade (between browser and author styles)… It would be a difficult pitch to the working group if that's the only use-case, but might have other use-cases, the more authors are relying on shadow DOM. Should a site should be able to apply global defaults that underly custom element defaults? Seems like people would put their resets into a layer like that.

I have the same problem with Firefox 123, even without shadow dom. The only thing that worked was using !important. The :where() or @layer did not help.