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-notation autofix #6742

Conversation

romainmenke
Copy link
Member

Which issue, if any, is this issue related to?

Closes #6501

Is there anything in the PR that needs further explanation?

Some bits are more complex/messy than I would like but I held of on trying to abstract this.
I personally prefer to only abstract away complexity after having found ±3 use cases.

But I don't mind spending time on refactors now if people with more experience in this code base have good ideas on how to improve this :)

@changeset-bot
Copy link

changeset-bot bot commented Mar 28, 2023

🦋 Changeset detected

Latest commit: b854a03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
stylelint Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This was referenced Mar 29, 2023
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

@romainmenke Thanks for the pull request. Great work! 👏🏼

I've left a few suggestions, but this change has no big problems. 👍🏼

.changeset/dull-knives-jam.md Outdated Show resolved Hide resolved
docs/user-guide/rules.md Outdated Show resolved Hide resolved
lib/rules/media-feature-range-notation/index.js Outdated Show resolved Hide resolved
lib/rules/media-feature-range-notation/index.js Outdated Show resolved Hide resolved
romainmenke and others added 4 commits March 29, 2023 11:36
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
@romainmenke
Copy link
Member Author

Thank you for the review and feedback @ybiquitous 🙇
I've applied all the suggestions.

@@ -228,7 +228,7 @@ Enforce one representation of things that have multiple with these `notation` (s
- [`hue-degree-notation`](../../lib/rules/hue-degree-notation/README.md): Specify number or angle notation for degree hues (Autofixable) (Ⓢ).
- [`import-notation`](../../lib/rules/import-notation/README.md): Specify string or URL notation for `@import` rules (Autofixable) (Ⓢ).
- [`keyframe-selector-notation`](../../lib/rules/keyframe-selector-notation/README.md): Specify keyword or percentage notation for keyframe selectors (Autofixable) (Ⓢ).
- [`media-feature-range-notation`](../../lib/rules/media-feature-range-notation/README.md): Specify context or prefix notation for media feature ranges.
- [`media-feature-range-notation`](../../lib/rules/media-feature-range-notation/README.md): Specify context or prefix notation for media feature ranges (Autofixable) (Ⓢ).
Copy link
Member

Choose a reason for hiding this comment

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

[note] Good catch to support the standard config. 👍🏼

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Wow, thanks for your quick fix! LGTM 👍🏼

@ybiquitous ybiquitous merged commit 6675487 into main Mar 29, 2023
14 checks passed
@ybiquitous ybiquitous deleted the add-fix-support-for-media-feature-range-notation--resourceful-harrier-c39c531fed branch March 29, 2023 09:57
@yoyo837
Copy link
Contributor

yoyo837 commented Apr 3, 2023

Unable to parse when :

less file:

@media screen and(width <= @screen-sm-max) {
  .topbox {
    // code
  }
}

Need to add spaces manually:

@media screen and (width <= @screen-sm-max) {
  .topbox {
    // code
  }
}

@ybiquitous
Copy link
Member

ybiquitous commented Apr 3, 2023

@yoyo837 Thanks for the report. Can you open a new issue with reproducible info, please?

@yoyo837
Copy link
Contributor

yoyo837 commented Apr 3, 2023

@yoyo837 Thanks for the report. Can you open a new issue with reproducible info, please?

Yes, here it is #6754.

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

Successfully merging this pull request may close these issues.

Add media-feature-range-notation autofix
3 participants