oddbird / popover-polyfill

Polyfills the HTML popover attribute and showPopover/hidePopover/togglePopover methods onto HTMLElement, as well as the popovertarget and popovertargetaction attributes on <button> elements.
BSD 3-Clause "New" or "Revised" License
252 stars 14 forks source link

Demo Application Does Not Work in Safari 15 or 16 #205

Open spiralman opened 4 months ago

spiralman commented 4 months ago

I wanted to evaluate the behavior of the polyfill in Safari 15 & 16 before integrating it into my project, so I opened the demo netlify app in each browser (I was using Saucelabs to access the browser), and in both cases, the styling of the demo app had issues:

In Safari 15, the demo application was usable, but completely unstyled.

In Safari 16, the demo application is completely unusable, as all the elements on the screen are invisible (except for the code samples, and, only when hovering, the OddBird logo in the nav bar).

It seems like, if the purpose of the library is to support older browsers, the demo app should probably work in older browsers. It certainly gives me pause as to the extent of the backwards compatible support if the demo app doesn't work in older browsers.

How is the polyfill being tested in these browser environments?

Note: I don't see any code for the demo app in this repo, but couldn't find a repo in your org that does have it. If there's a better place to file the ticket, please let me know.

jgerigmeyer commented 4 months ago

@spiralman Thanks for reporting! I'm able to reproduce what you're seeing in Safari 15, but Safari 16 looks mostly-fine to me, with the exception of some Shadow DOM content.

20240517_16_31_46_safari_16

It seems like, if the purpose of the library is to support older browsers, the demo app should probably work in older browsers. It certainly gives me pause as to the extent of the backwards compatible support if the demo app doesn't work in older browsers.

A fair point! We're working under very limited time/money resources at the moment, and just recently launched even the most basic version of a styled demo page -- whereas the polyfill itself has been used in production heavily over the past year. We'll take a look, but if you're willing to dig in and contribute that would be much appreciated!

How is the polyfill being tested in these browser environments?

In CI at the moment we're testing against Chromium 113, Firefox 112, and Webkit 16.4.

Note: I don't see any code for the demo app in this repo, but couldn't find a repo in your org that does have it. If there's a better place to file the ticket, please let me know.

Everything is here in the repo. The demo is served from index.html, with styles in the css folder.

jgerigmeyer commented 4 months ago

@stacyk or @dvdherron Could one of you look into this?

dvdherron commented 4 months ago

@jgerigmeyer I think there might be two things causing the styles not to show properly here. The oldest version of Safari probably doesn't have support for Cascade Layers. The issue in the later version of Safari is probably caused by lack of CSS support for nesting. I could rework some of the css so that we're not using nesting. As far as Layers support, I could unlayer some of the critical styles or we could use our polyfill?

jgerigmeyer commented 4 months ago

I could rework some of the css so that we're not using nesting. As far as Layers support, I could unlayer some of the critical styles or we could use our polyfill?

Those both sound reasonable to me, I'm fine with whatever you'd propose 👍

dvdherron commented 3 months ago

@stacyk I only got so far as to test whether I was right about what was causing the style issues here. Nesting will obviously be an issue with some styles since that's one of the latest features to be implemented. But I think the main cause of style issues is layers. Like I mentioned above, we could either unlayer the bare minimum styles to have this work okay in older browsers, or make use of our polyfill. Container queries are used for styles here too, but I don't think them not working is having as drastic of an effect on the layout as layers/nesting.

jgerigmeyer commented 3 months ago

Like I mentioned above, we could either unlayer the bare minimum styles to have this work okay in older browsers, or make use of our polyfill.

I think adding the layers polyfill is fine, if that's quickest and works well.