-
-
Notifications
You must be signed in to change notification settings - Fork 927
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 ignoreLonghands: []
to declaration-block-no-redundant-longhand-properties
#7611
Conversation
🦋 Changeset detectedLatest commit: 4c9912e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
0248a0b
to
2ceade8
Compare
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Show resolved
Hide resolved
ignoreLonghands
option to declaration-block-no-redundant-longhand-properties
ignoreLonghands: []
to declaration-block-no-redundant-longhand-properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request. Can you address my comments?
lib/rules/declaration-block-no-redundant-longhand-properties/__tests__/index.mjs
Show resolved
Hide resolved
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
lib/rules/declaration-block-no-redundant-longhand-properties/index.mjs
Outdated
Show resolved
Hide resolved
e85e464
to
ff29dfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. LGTM 👍🏼
@ybiquitous @jeddy3 I would advise to add |
Hum, this example on MDN makes sense, right? .thick {
text-decoration: solid underline purple 4px; /* includes `text-decoration-thickness: 4px` */
} |
It does if you consider that 95%+ support on caniuse is enough. If not it would be better to ignore |
I think the percentage is enough for now. Let's keep things simple. If people complain, then we can revisit it. |
lib/rules/declaration-block-no-redundant-longhand-properties/README.md
Outdated
Show resolved
Hide resolved
|
Have you merge the commit fca3269 into this branch? |
Closes #7232
To understand why a fix is being accompanied by an option check the discussion starting with #7232 (comment)
tl;dr: CSS Text Decoration Module Level 3 vs CSS Text Decoration Module Level 4 means that a granular option is necessary for the
text-decoration
fix not be considered a downgrade