added checking color #1
Conversation
add test hsl, hsla, hsv, hsva, cmyk, cmyka color;
index.js
Outdated
| break; | ||
| } | ||
|
|
||
| return cssTree.fromPlainObject(cssTree.clone(ast)); |
There was a problem hiding this comment.
It’s not clear if this return statement works here. I may guess, your modifications are done in ast directly.
index.js
Outdated
| switch (node.name) { | ||
| case 'rgb': | ||
| if(colorGenerate === initialColor.rgb.toString()) { | ||
| ast.children.map((item, index) => { |
There was a problem hiding this comment.
You have many similar block, which can be extracted into one function. For example,
- You can provide
finalColor.rgb,finalColor.rgba… as a parameter. - You can set the number components to process (3 for rgb, 4 for rgba…)
I suggest replace .map() with .forEach() here. You don’t interested in a result, right? You just want to iterate over array, so .map() is irrelevant here.
index.js
Outdated
|
|
||
| switch (node.name) { | ||
| case 'rgb': | ||
| if(colorGenerate === initialColor.rgb.toString()) { |
There was a problem hiding this comment.
Please, format all your sources properly.
index.js
Outdated
|
|
||
| const ast = cssTree.toPlainObject(node); | ||
|
|
||
| const colorGenerate = ast.children.reduce((acc, item) => { |
There was a problem hiding this comment.
It‘s not clear what does this mean. colorGenerate doesn’t make sense. Why are you join these parts again?
Aside… I may guess, you are using cssTree.toPlainObject to access array methods like .map() and .reduce(). Can you confirm that there are no such methods on original object, please?
|
made edits |
test/hsv-color.spec.js
Outdated
| test('should alter hsv colors', () => { | ||
| return postcss() | ||
| .use(alterColorPlugin({from: 'black', to: 'red'})) | ||
| .process(`div{color:hsv(0,0,0)}`, {from: undefined}) |
There was a problem hiding this comment.
There is no hsv() function in CSS. Please, refer https://developer.mozilla.org/ru/docs/Web/CSS/color_value
test/hsva-color.spec.js
Outdated
| test('should alter hsva colors', () => { | ||
| return postcss() | ||
| .use(alterColorPlugin({from: 'black', to: 'red'})) | ||
| .process(`div{color:hsva(0,0,0,1)}`, {from: undefined}) |
There was a problem hiding this comment.
There is no hsva() function in CSS. Please, refer https://developer.mozilla.org/ru/docs/Web/CSS/color_value
test/hsl-color.spec.js
Outdated
| test('should alter hsl colors', () => { | ||
| return postcss() | ||
| .use(alterColorPlugin({from: 'black', to: 'red'})) | ||
| .process(`div{color:hsl(0,0,0)}`, {from: undefined}) |
There was a problem hiding this comment.
hsl(0,0%,0%) — don't mix integers and percentages
test/hsl-color.spec.js
Outdated
| test('should alter hsl color in a complex value', () => { | ||
| return postcss() | ||
| .use(alterColorPlugin({from: 'black', to: 'red'})) | ||
| .process(`div{color:hsl(0,0,0);border:1px solid hsl(255,255,255);outline:1px solid hsl(0,0,0)}`, {from: undefined}) |
There was a problem hiding this comment.
You test against invalid colours. It should be like this:
div{color:hsl(0,0%,0%);border:1px solid hsl(255,100%,100%);outline:1px solid hsl(0,0%,0%)}
test/hsla-color.spec.js
Outdated
| test('should alter hsla colors', () => { | ||
| return postcss() | ||
| .use(alterColorPlugin({from: 'black', to: 'red'})) | ||
| .process(`div{color:hsla(0,0,0,1)}`, {from: undefined}) |
There was a problem hiding this comment.
hsl(0,0%,0%,1) — don't mix integers and percentages
index.js
Outdated
| case 'hsla': | ||
| changingColorFunction(ast, checkedColor, initialColor.hsla, finalColor.hsla); | ||
| break; | ||
| case 'hsv': |
There was a problem hiding this comment.
Please, remove all irrelevant switch cases.
index.js
Outdated
| return `${hexValue[0]}${hexValue[2]}${hexValue[4]}`; | ||
| } | ||
|
|
||
| function changingColorFunction(color, checkedColor, initialColor, finalColor) { |
There was a problem hiding this comment.
Your function is not working properly with "Number" and "Percentage" values.
index.js
Outdated
| visit: 'Function', | ||
| enter(node) { | ||
|
|
||
| const ast = cssTree.toPlainObject(node); |
There was a problem hiding this comment.
You don't need this conversion because node.children has .forEach() and .map() methods. Please, see https://github.com/csstree/csstree/blob/master/lib/utils/list.js
index.js
Outdated
|
|
||
| const ast = cssTree.toPlainObject(node); | ||
|
|
||
| const checkedColor = ast.children.reduce((color, item) => { |
There was a problem hiding this comment.
Could you get rid of such unclear conversions and compare objects by their content directly, please?
index.js
Outdated
| } | ||
|
|
||
| function changingColorFunction(color, checkedColor, initialColor, finalColor) { | ||
| if (checkedColor === initialColor.toString()) { |
There was a problem hiding this comment.
Please, don’t use .toString() to compare objects or arrays!
No description provided.