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 media-feature-range-context-notation #6502

Open
jeddy3 opened this issue Dec 1, 2022 · 5 comments
Open

Add media-feature-range-context-notation #6502

jeddy3 opened this issue Dec 1, 2022 · 5 comments
Labels
status: ask to implement ask before implementing as may no longer be relevant type: new rule an entirely new rule

Comments

@jeddy3
Copy link
Member

jeddy3 commented Dec 1, 2022

What is the problem you're trying to solve?

To write media features in a range context consistently when there are two media features of the same name.

What solution would you like to see?

A new rule to either combine or separate media features.

For example, either:

@media (width >= 500px) and (width <= 1200px) {}

Or:

@media (500px <= width <= 1200px) {}
  • Name: media-feature-range-context-notation
  • Primary option: "basic"|"nested"
  • Secondary options: none
  • Autofixable: Yes
  • Message: Expected "${primary}" media feature range context notation
  • Description: "Specify basic or nested notation for media features in range context (Autofixable)."
  • Section: "Enforce conventions" -> "Notation"

Primary options names from spec:

The basic form, consisting of a feature name, a comparison operator, and a value, returns true if the relationship is true.

And:

The remaining forms, with the feature name nested between two value comparisons, returns true if both comparisons are true.

See #6497 (comment)

@jeddy3 jeddy3 added the status: needs discussion triage needs further discussion label Dec 1, 2022
@ybiquitous
Copy link
Member

Sounds good! 👍🏼

@Mouvedia
Copy link
Contributor

Mouvedia commented Dec 3, 2022

The fix for this one will require advanced conversion of units for proper mergers.

e.g.
@media (width >= 500px) and (width < 50em) and (width <= 1200px) {} (16px font-size)
to
@media (500px <= width <= 1200px) {}

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: new rule an entirely new rule and removed status: needs discussion triage needs further discussion labels Dec 6, 2022
@jeddy3
Copy link
Member Author

jeddy3 commented Dec 6, 2022

I may be wrong, but I don't think we can safely merge ems into px as people can change the initial value of font-size in their browser settings. For example, doubling it to 32px.

Either way, it's likely people will be using the same units across their media features so let's implement that first. This reminds me, I need to create an issue for media-feature-name-unit-allowed-list from #6211 (comment).

I've labelled the issue as ready to implement. Please consider contributing this rule if anyone has time.

There are steps on how to add a new rule in the Developer guide.

@Mouvedia
Copy link
Contributor

Mouvedia commented Dec 6, 2022

so let's implement that first

same unit : the removal of useless conditions while merging is a first step
inconsistent units : the conversion—if possible—can be done later

Copy link
Contributor

This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.

@github-actions github-actions bot added status: ask to implement ask before implementing as may no longer be relevant and removed status: ready to implement is ready to be worked on by someone labels Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ask to implement ask before implementing as may no longer be relevant type: new rule an entirely new rule
Development

No branches or pull requests

3 participants