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

Clean up styles for older browsers #236

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

dvdherron
Copy link
Contributor

@dvdherron dvdherron commented Feb 26, 2025

Related Issue(s)

Fixes #205

Steps to test/reproduce

  • On Sauce Labs, view the demo page in Safari 15 and 16 (Ventura OS) or versions of other browsers before they implemented cascade layers.
  • Compare the the deploy preview here to the live site. In the latest versions of browsers, everything should be the same (except for the updated @layers popover example).

image

Please explain how to best reproduce the issue and/or test the changes locally (including the pages/URLs/views/states to review).

@dvdherron dvdherron requested a review from stacyk February 26, 2025 15:12
Copy link

netlify bot commented Feb 26, 2025

Deploy Preview for popover-polyfill ready!

Name Link
🔨 Latest commit 0f301bc
🔍 Latest deploy log https://app.netlify.com/sites/popover-polyfill/deploys/67c629d5ba8230000813db99
😎 Deploy Preview https://deploy-preview-236--popover-polyfill.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dvdherron
Copy link
Contributor Author

@stacyk I've removed the references here to layers (and most nesting). There's something happening that's causing a lot of the layout styles to not be applied. Testing on FF 95, custom properties are valid but not all of them are reflected in the styles of the page. Can you see the same thing in Safari 15?

@dvdherron dvdherron self-assigned this Feb 26, 2025
@dvdherron
Copy link
Contributor Author

@stacyk I figured out why the size variables were not working. It's due to the cq units in the base sizes.

@stacyk
Copy link
Member

stacyk commented Feb 26, 2025

@dvdherron I have Safari 18 and there isn't a way to downgrade that I know of, so using Saucelabs is the way to go for that. Sounds like you are on the right trail so let me know if you want a review when ready.

@dvdherron dvdherron marked this pull request as ready for review February 27, 2025 20:56
@dvdherron
Copy link
Contributor Author

@jgerigmeyer @stacyk I think this is ready for review.

I removed the references to layers (except for the demo example). I changed the example from having a red background to having a red border. With the former, an old browser could not read the text (It was white text on a white background).

I also removed nesting and added some @support for container query related styles. Its not perfect in older browsers, but I'm not sure how far back we want to fully support for this page.

If you test this page on Safari 16, it seems to only work when you choose Ventura as an OS version.

The shadowed popovers in older browsers don't seem to be getting the "UA Stylesheet" styles we are injecting with JS. I think that's because we are using .innerHTML to create those elements @jgerigmeyer? These popovers aren't positioned so they push content around when they are opened.

dvdherron and others added 3 commits February 28, 2025 14:06
* main:
  typo
  pin back ts for now
  chore(deps-dev): Bump the npm-minor-upgrades group with 9 updates
Copy link
Member

@jgerigmeyer jgerigmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

The shadowed popovers in older browsers don't seem to be getting the "UA Stylesheet" styles we are injecting with JS. I think that's because we are using .innerHTML to create those elements @jgerigmeyer? These popovers aren't positioned so they push content around when they are opened.

Yeah, I think this is directly related to this note. I think I fixed it (conditionally re-injecting the styles in shadow roots) in 0f301bc -- take a look and see what you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Demo Application Does Not Work in Safari 15 or 16
3 participants