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

Fix rule media-feature-range-notation to use 'prefix' notation #59

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 29, 2023

A previous update to Stylelint included the rule media-feature-range-notation

But similar to other rules it enforces syntax that is incompatible with Internet Explorer

This PR changes the default to context prefix until Media Queries Level 4 range context is better supported

.govuk-footer__copyright-logo {
  background-image: govuk-image-url("govuk-crest.png");

- @media (resolution >= 2dppx) {
+ @media (min-resolution: 2dppx) {
    background-image: govuk-image-url("govuk-crest-2x.png");
  }
}

Note: Autoprefixer only handles prefixes but a PostCSS plugin for @media range context is available

Media Queries Level 4 feature range syntax lacks both Autoprefixer and Internet Explorer support so we should override the rule default `'context'` to `'prefix'`

https://stylelint.io/user-guide/rules/media-feature-range-notation
https://www.w3.org/TR/mediaqueries-4/#mq-range-context
https://caniuse.com/css-media-range-syntax
@colinrotherham colinrotherham added the bug Something isn't working label Nov 29, 2023
@colinrotherham colinrotherham self-assigned this Dec 4, 2023
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Sneaky, thanks for spotting it. Looks like we'll need to keep a keen eye on Stylelint's changelog in regards to IE11 compatibility for now 😔

I'd be keen to make that PR bump the versions to 1.1.1 so a patch release gets published as well 😊

@colinrotherham
Copy link
Contributor Author

Thanks @romaricpascal

Sounds good, I'll update the version number then merge

@colinrotherham colinrotherham merged commit 48c00db into main Dec 15, 2023
2 checks passed
@colinrotherham colinrotherham deleted the media-query-context branch December 15, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

2 participants