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

Add support for cascade layers #178

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

Prestaul
Copy link
Contributor

@Prestaul Prestaul commented Feb 7, 2024

Description

User-agent and polyfilled styles should always have lowest priority, but if a consumer uses cascade layers (i.e. @layer) then polyfilled popover styles have highest priority and overwrite other styles unless they are also in a layer. This change puts polyfilled styles into a layer so that consumers can control layer order.

Related Issue(s)

#175 (original issue and discussion)

Steps to test/reproduce

Place a style that sets the background or color of any popover element into any @layer block. That style will only apply correctly after this change.

Copy link

netlify bot commented Feb 7, 2024

Deploy Preview for popover-polyfill ready!

Name Link
🔨 Latest commit 7be6579
🔍 Latest deploy log https://app.netlify.com/sites/popover-polyfill/deploys/65c50779e156030008ed6962
😎 Deploy Preview https://deploy-preview-178--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.

@Prestaul
Copy link
Contributor Author

Prestaul commented Feb 7, 2024

I apologize for the stacked PR. This seemed the best way to avoid conflicts with my other PR, but I'm happy to break this up and just deal with conflicts and rebasing if one lands.

The main issue is in the index.html where each PR is adding a new popover and button for testing. The existing pattern of just incrementing a number in the id (i.e. id="popover12) doesn't lend itself to concurrent changes.

README.md Show resolved Hide resolved
src/popover.ts Outdated Show resolved Hide resolved
tests/edge-cases.spec.ts Show resolved Hide resolved
@jgerigmeyer jgerigmeyer linked an issue Feb 8, 2024 that may be closed by this pull request
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.

Thanks!

The existing pattern of just incrementing a number in the id (i.e. id="popover12) doesn't lend itself to concurrent changes.

Yes -- we're working on creating a better demo page.

@jgerigmeyer jgerigmeyer merged commit 7053284 into oddbird:main Feb 8, 2024
7 checks passed
@Prestaul Prestaul deleted the 175-cascade-layers-support branch February 8, 2024 17:56
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.

Current implementation incompatible with css layers
6 participants