Add new option to allow hex values to be output #11
Conversation
There was a problem hiding this comment.
@andrewbrandwood Thanks for submitting this PR. I think the hex option could be valuable but the implementation needs to be thought out a bit. As I mentioned, what should be returned when a color has a alpha value that's not 1 or 0? Do we want to force all colors to hex with this option?
Aside from that, please be sure to clean up and rebase and commits that are not directly related to the change you're making. It makes the PR easier to read and keeps the commit history clean. This change shouldn't require the package-lock.json file to be updated.
Last, the documentation will need to be updated to include this new option.
| @@ -85,7 +92,11 @@ function transformKey(key, shouldCamelCase) { | |||
| * @return {object} Transformed variables object | |||
| */ | |||
| function transformVars(varsObj, options) { | |||
| const opts = Object.assign({ camelCase: true }, options); | |||
| const defaults = { | |||
There was a problem hiding this comment.
nit: Don't really need to create a const for defaults
| const defaults = { | ||
| camelCase: true, | ||
| hex: false | ||
| } |
|
|
||
| // return the hex value if specified. | ||
| if(options.hex){ | ||
| return hex; |
There was a problem hiding this comment.
Not so sure about this change since it will force all colors to be returned as a hex, even colors with an alpha value. I would think we'd still want to return rgba in those cases.
package.json
Outdated
| @@ -27,8 +27,7 @@ | |||
| "author": "Adam Gruber <[email protected]>", | |||
| "license": "MIT", | |||
| "dependencies": { | |||
| "camel-case": "^3.0.0", | |||
| "jest-cli": "^23.6.0" | |||
| "camel-case": "^3.0.0" | |||
There was a problem hiding this comment.
If you rebase then this fix commit isn't necessary and the package-lock file won't be updated.
|
You're right it does need thinking about a little more. It's a shame sass-extract doesn't just return the colour in it's original form |
@andrewbrandwood |
I needed this to show the hex values for my style guide visual reference.