Make sure it checks for display-scale #619
Conversation
🦋 Changeset detectedLatest commit: 1e3429d 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 |
| @@ -25,8 +25,8 @@ export const messages = ruleMessages(ruleName, { | |||
| let variables = primitivesVariables('colors') | |||
| const validProps = { | |||
| '^color$': ['fgColor', 'iconColor'], | |||
| '^background(-color)?$': ['bgColor'], | |||
| '^border(-top|-right|-bottom|-left|-inline|-block)*(-color)?$': ['borderColor'], | |||
| '^background(-color|-image)?$': ['bgColor'], | |||
There was a problem hiding this comment.
Also test background-image
| '^background(-color)?$': ['bgColor'], | ||
| '^border(-top|-right|-bottom|-left|-inline|-block)*(-color)?$': ['borderColor'], | ||
| '^background(-color|-image)?$': ['bgColor'], | ||
| '^border(?:-top|-right|-bottom|-left|-inline|-block)?(?:-color)?$': ['borderColor'], |
There was a problem hiding this comment.
Enable testing for border
| (/^border(-top|-right|-bottom|-left|-inline|-block)*$/.test(prop) || /^background$/.test(prop)) && | ||
| !valueNode.value.toLowerCase().includes('color') | ||
| (/^border(?:-top|-right|-bottom|-left|-inline|-block)?$/.test(prop) || /^background$/.test(prop)) && | ||
| ['color', 'scale'].every(word => !valueNode.value.toLowerCase().includes(word)) |
There was a problem hiding this comment.
We now only return if neither color nor scale is in the token, because we want --display-blue-scale-0 to fail.
If this is risky we could also consider changing the name of the token to --display-blue-color-0 or --display-blue-colorScale-0 CC:@langermank
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the color validation rule to handle background-image properties, exclude display-scale tokens from shorthand props, and adds corresponding test coverage.
- Broaden regexes to include
-imagefor backgrounds and refine border shorthand matching - Update shorthand logic to skip values containing “scale” as well as “color”
- Add new tests for rejecting
--display-*-scale-*variables in various properties
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugins/colors.js | Extended background regex, refined border shorthand logic, and updated the skip‐value check to also exclude “scale.” |
| tests/colors.js | Added tests to ensure display‐scale tokens are rejected by the rule |
| .changeset/pink-beds-mix.md | Bumped minor version and noted new background/border behavior |
Comments suppressed due to low confidence (3)
plugins/colors.js:139
- Typo in comment:
shortandshould beshorthand.
// Property is shortand and value doesn't include color
plugins/colors.js:29
- The updated border regex only allows one segment (e.g.,
-top) but drops support for multi-part shorthands likeborder-inline-block. Change?back to*on the segment group to restore repeated segments.
'^border(?:-top|-right|-bottom|-left|-inline|-block)?(?:-color)?$': ['borderColor'],
plugins/colors.js:51
- Same issue here: this
propTyperegex restricts to a single segment. Use(?:-top|-right|-bottom|-left|-inline|-block)*instead of?to allow multiple segments in shorthand border props.
} else if (/^border(?:-top|-right|-bottom|-left|-inline|-block)?(?:-color)?$/.test(prop)) {
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
@jonrohan can you do this? I have no idea how to do what you just said. |
This pull request introduces updates to the color validation logic and test cases in the
colorsplugin. The changes primarily enhance the handling ofbackground,border, and related properties, expand the validation for shorthand properties, and add new test cases to cover display scale tokens.@langermank @jonrohan I hope this change make sense. It is the first time for me working in this repo. I'll add some comments in the code to explain