-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Use parsers.parseColor() in border-color parsing, like background-color #185
base: main
Are you sure you want to change the base?
Conversation
lib/properties/borderColor.js
Outdated
} | ||
if ( | ||
parsers.valueType(v) === parsers.TYPES.KEYWORD && | ||
(v.toLowerCase() === 'transparent' || v.toLowerCase() === 'inherit') |
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.
I think we don't need v.toLowerCase() === 'transparent'
.
It's already resolved by css-color
.
To make sure, add test like below somewhere around test/parsers.js#L64
it('returns color for transparent keyword', () => {
let input = 'transparent';
let output = parsers.valueType(input);
assert.strictEqual(output, parsers.TYPES.COLOR);
});
And before test/parsers.js#L248 i.e. it.todo('Add more tests')
it('should output transparent keyword', () => {
let input = 'transparent';
let output = parsers.parseColor(input);
assert.strictEqual(output, 'transparent');
});
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.
I had effectively copy/pasted this from the background-color
logic. Should I update that too, to keep in sync? (I've proactively updated it, but as a separate commit that should be easy to revert if needed)
thanks for the review!
abcf2a6
to
c23435a
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.
LGTM
My attempt at fixing #184. Please feel free to use this code however ends up most useful.