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 range context support to media-feature-name-* rules #1744

Closed
jeddy3 opened this issue Jul 30, 2016 · 6 comments · Fixed by #4581
Closed

Add range context support to media-feature-name-* rules #1744

jeddy3 opened this issue Jul 30, 2016 · 6 comments · Fixed by #4581
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@jeddy3
Copy link
Member

jeddy3 commented Jul 30, 2016

Ref: #1732 (comment)

@media (10px < width > 50px) {}

Is dependent on the creation of a range context parser.

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules labels Jul 30, 2016
@jeddy3 jeddy3 added this to the future-major milestone Jul 30, 2016
@jeddy3 jeddy3 modified the milestones: 8.x, future-major Jul 11, 2017
vankop added a commit that referenced this issue Aug 19, 2019
@vankop vankop mentioned this issue Aug 19, 2019
@vankop
Copy link
Member

vankop commented Aug 19, 2019

I have look only on tests and did not understand from them that this is not a part of the parser yet 🤦🏼‍♂️

@alexander-akait
Copy link
Member

@vankop because parser is not maintainable 😞

@jdiogopeixoto
Copy link
Member

I am available to work on this.
Should I change media-feature-name-* rules to not ignore range context medias? It would be necessary to make a workaround, since the parser is not being maintained.

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 25, 2020

I am available to work on this.

Fantastic!

Should I change media-feature-name-* rules to not ignore range context medias?

Yes. If you can adapt the rules to support range context media robustly, then we should no longer ignore them.

@jeddy3 jeddy3 added status: wip is being worked on by someone and removed status: ready to implement is ready to be worked on by someone labels Jan 25, 2020
@jdiogopeixoto
Copy link
Member

Nice, I will start work on this.
I noticed that already exists two rules for ranged operators. Because of that wouldn't it be better to create new rules for each media-feature-name-* instead of adapting the rules? Just for consistency.

@jeddy3
Copy link
Member Author

jeddy3 commented Jan 28, 2020

I will start work on this.

Good stuff.

I noticed that already exists two rules for ranged operators. Because of that wouldn't it be better to create new rules for each media-feature-name-* instead of adapting the rules?

The two rules (media-feature-range-operator-space-after and media-feature-range-operator-space-before) are targeting the range operator within media features, which is a specific bit of punctuation.

Whereas the media-feature-name-* rules are targeting media feature names, which are present:

At the moment, the media-feature-name-* rules only check media feature names outside of the range context. It should, however, check all media feature names regardless of the context. To do so would be consistent with our conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
4 participants