[stylelint-polaris][v5] Final rule categorization #7828
Conversation
chloerice
requested review from
sam-b-rose,
alex-page,
sophschneider and
aaronccasanova
chloerice
force-pushed
the
final-categorization
branch
3 times, most recently
from
cb13cf3
to
f92afdb
Compare
chloerice
force-pushed
the
final-categorization
branch
from
f92afdb
to
3d04138
Compare
stylelint-polaris/index.js
Outdated
| 'z-index': { | ||
| 'declaration-property-value-allowed-list': [ | ||
| { | ||
| 'z-index': [/--p-z-\b([1-9]|1[0-2])\b/], |
There was a problem hiding this comment.
Can we update this to iterate through the zIndex tokens. e.g.
import {createVar, tokens} from '@shopify/polaris-tokens'
// ...
{
'z-index': Object.keys(tokens.zIndex).map(createVar)
}| }, | ||
| mediaQueries: { | ||
| breakpoints: { |
There was a problem hiding this comment.
mediaQueries may be more appropriate as this category can lint more than Polaris breakpoints e.g. media types, media features, media conditions/operators, etc. Thoughts?
There was a problem hiding this comment.
@aaronccasanova What do you think about re-categorizing the non-breakpoint related options in the breakpoints category media-query-allowed-list over to conventions? It seems like that would be a better fit so the coverage site can map the results related to breakpoints to the breakpoints token coverage.
| @@ -367,7 +383,8 @@ const stylelintPolarisCoverageOptions = { | |||
| ], | |||
| }, | |||
| }, | |||
| legacySass: { | |||
| legacy: { | |||
There was a problem hiding this comment.
Looks good to me. @aaronccasanova gave some great feedback.
stylelint-polaris/index.js
Outdated
| @@ -71,7 +73,7 @@ const stylelintPolarisCoverageOptions = { | |||
| /--p-duration-1-5-0/, | |||
| ], | |||
| }, | |||
| typography: { | |||
| font: { | |||
There was a problem hiding this comment.
font aligns with our token group but I'm not sure it describes the full scope of what we're able to lint regarding typography. That said, I'm not attached to typography as a category so feel free to proceed with this update!
There was a problem hiding this comment.
I agree! I'm gonna leave it as typography, as I also wonder how me might want to iterate on or add to this category to account for the new Text component folks should be leaning on primarily going forward 🤔
stylelint-polaris/index.js
Outdated
| @@ -162,7 +164,6 @@ const stylelintPolarisCoverageOptions = { | |||
| 'grid-template-rows', | |||
| 'grid-template-columns', | |||
| 'grid-area', | |||
| 'display', | |||
There was a problem hiding this comment.
Why are position and display removed? It looks like the replacements don't cover the same scope. I thought having these set to warnings gives us insight on potential/future layout components.
There was a problem hiding this comment.
Good call, I've removed those changes as they were incomplete explorations of addressing some of the things we need to iron out in #7827 💯
chloerice
force-pushed
the
final-categorization
branch
from
6938171
to
f6e750e
Compare
chloerice
force-pushed
the
final-categorization
branch
from
f6e750e
to
515feb4
Compare
### WHY are these changes being introduced? To unblock the release of v5 and enabling the linter in `@shopify/polaris` after running the disable comments codemod. ### WHAT is this pull request doing? This PR fixes: - Miscategorization of a few legacy at-rules grouped into `function-disallowed-list` instead of `stylelint-polaris/at-rule-disallowed-list` - Categories not mapping to token groups: - ~Changed typography to font~ - Added z-index category - Changed mediaQueries to breakpoints - Changed legacySass to legacy (as it encompasses legacy Sass API stuff and legacy CSS custom properties)
### WHY are these changes being introduced? To unblock the release of v5 and enabling the linter in `@shopify/polaris` after running the disable comments codemod. ### WHAT is this pull request doing? This PR fixes: - Miscategorization of a few legacy at-rules grouped into `function-disallowed-list` instead of `stylelint-polaris/at-rule-disallowed-list` - Categories not mapping to token groups: - ~Changed typography to font~ - Added z-index category - Changed mediaQueries to breakpoints - Changed legacySass to legacy (as it encompasses legacy Sass API stuff and legacy CSS custom properties)
WHY are these changes being introduced?
To unblock the release of v5 and enabling the linter in
@shopify/polarisafter running the disable comments codemod.WHAT is this pull request doing?
This PR fixes:
function-disallowed-listinstead ofstylelint-polaris/at-rule-disallowed-listChanged typography to font